a73x

tests/body_edit_test.rs

Ref:   Size: 29.9 KiB   History

//! Correcting prose that is already in the DAG.
//!
//! Events are append-only and stay that way: an edit is a new event that
//! supersedes an earlier one's body, exactly as `IssueEdit` already
//! supersedes an issue's title and body. The same single mechanism covers
//! thread comments, inline comments, review bodies and revision
//! descriptions, because they are all just a body hanging off an event OID.
//!
//! Three properties are load-bearing and each is tested here:
//!
//! * **Attribution.** An edit signed by a different key must not silently
//!   rewrite someone else's words. The fold ignores it.
//! * **Ordering.** An edited comment stays where it was, so a reply above or
//!   below it still reads in sequence.
//! * **Convergence.** Two clones editing the same comment offline settle on
//!   the same text, decided by `(clock, oid)` like every other conflict here.

mod common;

use common::{alice, bob, init_repo, test_signing_key, TestRepo};

use git2::Repository;
use git_collab::dag;
use git_collab::event::{Action, Author, Event, ReviewVerdict};
use git_collab::state::{IssueState, PatchState};
use tempfile::TempDir;

// ===========================================================================
// Helpers for driving the DAG directly (used by the attribution and
// convergence tests, which need to forge events from a second identity).
// ===========================================================================

fn append(repo: &Repository, ref_name: &str, author: &Author, action: Action) -> git2::Oid {
    let sk = test_signing_key();
    let event = Event {
        timestamp: common::now(),
        author: author.clone(),
        action,
        clock: 0,
    };
    dag::append_event(repo, ref_name, &event, &sk).unwrap()
}

fn open_issue_with_comment(repo: &Repository) -> (String, String, git2::Oid) {
    let (ref_name, id) = common::open_issue(repo, &alice(), "issue under test");
    let comment_oid = append(
        repo,
        &ref_name,
        &alice(),
        Action::IssueComment {
            body: "original text".to_string(),
        },
    );
    (ref_name, id, comment_oid)
}

fn issue_state(repo: &Repository, ref_name: &str, id: &str) -> IssueState {
    IssueState::from_ref_uncached(repo, ref_name, id).unwrap()
}

/// Pull the short comment id `patch show` / `issue show` prints, for the
/// first comment listed.
fn first_comment_id(json: &str, list: &str) -> String {
    let value: serde_json::Value = serde_json::from_str(json).expect("valid JSON");
    value[list][0]["commit_id"]
        .as_str()
        .unwrap_or_else(|| panic!("no commit_id on {}[0] in {}", list, json))
        .to_string()
}

fn body_at(json: &str, pointer: &str) -> String {
    let value: serde_json::Value = serde_json::from_str(json).expect("valid JSON");
    value
        .pointer(pointer)
        .unwrap_or_else(|| panic!("no {} in {}", pointer, json))
        .as_str()
        .expect("string")
        .to_string()
}

// ===========================================================================
// Editing, end to end
// ===========================================================================

#[test]
fn an_issue_comment_can_be_corrected() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.issue_open("typo");
    repo.run_ok(&["issue", "comment", &id, "-b", "teh original"]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");

    repo.run_ok(&[
        "issue",
        "edit-comment",
        &id,
        &comment_id[..8],
        "-b",
        "the original",
    ]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/comments/0/body"), "the original");
    assert_eq!(
        body_at(&json, "/comments/0/author/email"),
        "alice@example.com",
        "an edit must not change who wrote the comment"
    );
}

#[test]
fn a_patch_thread_comment_can_be_corrected() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("thread edit");
    repo.run_ok(&["patch", "comment", &id, "-b", "wrong"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");

    repo.run_ok(&[
        "patch",
        "edit-comment",
        &id,
        &comment_id[..8],
        "-b",
        "right",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/comments/0/body"), "right");
}

#[test]
fn an_inline_comment_can_be_corrected_without_moving() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("inline edit");
    // The anchor has to resolve in the revision: `patch_create` commits
    // `<title>.txt`, so that is the file under review.
    repo.run_ok(&[
        "patch",
        "comment",
        &id,
        "--file",
        "inline-edit.txt",
        "--line",
        "1",
        "-b",
        "wrong",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "inline_comments");

    repo.run_ok(&[
        "patch",
        "edit-comment",
        &id,
        &comment_id[..8],
        "-b",
        "right",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/inline_comments/0/body"), "right");
    assert_eq!(
        body_at(&json, "/inline_comments/0/file"),
        "inline-edit.txt",
        "an edit touches the body and nothing else"
    );
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    assert_eq!(value["inline_comments"][0]["line"], 1);
}

/// The case the issue was actually filed about: a typo in a review body.
#[test]
fn a_review_body_can_be_corrected_without_changing_the_verdict() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("review edit");
    repo.run_ok(&[
        "patch",
        "review",
        &id,
        "-v",
        "request-changes",
        "-b",
        "probe comment, please ignore",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let review_id = first_comment_id(&json, "reviews");

    repo.run_ok(&[
        "patch",
        "edit-comment",
        &id,
        &review_id[..8],
        "-b",
        "the real review",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/reviews/0/body"), "the real review");
    assert_eq!(
        body_at(&json, "/reviews/0/verdict"),
        "request-changes",
        "editing the prose must not disturb the vote"
    );
}

#[test]
fn an_edit_reads_its_body_from_a_file_or_stdin_too() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("edit from stdin");
    repo.run_ok(&["patch", "comment", &id, "-b", "wrong"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");

    let corrected = "corrected `body` with $vars\n\n";
    repo.run_stdin_ok(
        &["patch", "edit-comment", &id, &comment_id[..8], "-F", "-"],
        corrected.as_bytes(),
    );

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/comments/0/body"), corrected);
}

/// Editing with no body given opens the editor seeded with the text as it
/// stands, which is what makes fixing a typo a matter of one keystroke.
#[test]
fn editing_with_no_body_opens_the_editor_seeded_with_the_current_text() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.issue_open("seeded editor");
    repo.run_ok(&["issue", "comment", &id, "-b", "seed me"]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");

    // Echo back what the editor was handed, with a suffix, so the test can
    // prove the buffer arrived pre-filled.
    let editor = repo.write_script(
        "seed-editor.sh",
        "#!/bin/sh\nprintf '%s and more' \"$(cat \"$1\")\" > \"$1\"\n",
    );

    repo.run_in_pty(&["issue", "edit-comment", &id, &comment_id[..8]], &editor);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/comments/0/body"), "seed me and more");
}

#[test]
fn editing_an_unknown_comment_is_an_error() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("unknown target");

    let err = repo.run_err(&["patch", "edit-comment", &id, "deadbeef", "-b", "x"]);
    assert!(
        err.contains("deadbeef"),
        "error should name the id that matched nothing: {}",
        err
    );
}

// ===========================================================================
// Deleting: a tombstone, not a disappearance
// ===========================================================================

/// A deleted comment leaves a tombstone. Others may have replied to it, the
/// event is still in the DAG either way, and a comment that silently
/// vanishes makes every reply to it reference nothing.
#[test]
fn a_deleted_comment_leaves_a_tombstone_in_place() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("tombstone");
    repo.run_ok(&["patch", "comment", &id, "-b", "first"]);
    repo.run_ok(&["patch", "comment", &id, "-b", "regrettable probe"]);
    repo.run_ok(&[
        "patch",
        "comment",
        &id,
        "-b",
        "third, replying to the above",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    let target = value["comments"][1]["commit_id"]
        .as_str()
        .unwrap()
        .to_string();

    repo.run_ok(&["patch", "delete-comment", &id, &target[..8]]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    let comments = value["comments"].as_array().unwrap();

    assert_eq!(
        comments.len(),
        3,
        "the tombstone keeps the comment's slot so replies still line up"
    );
    assert_eq!(comments[0]["body"], "first");
    assert_eq!(comments[2]["body"], "third, replying to the above");
    assert_eq!(
        comments[1]["deleted"], true,
        "the middle comment is marked deleted"
    );
    assert_eq!(
        comments[1]["body"], "",
        "the deleted text must be gone from derived state, not merely hidden"
    );
    assert_eq!(
        comments[1]["author"]["email"], "alice@example.com",
        "a tombstone still says who left the comment"
    );
}

#[test]
fn a_deleted_comment_is_shown_as_deleted_not_as_blank() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("tombstone display");
    repo.run_ok(&["patch", "comment", &id, "-b", "regrettable"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let target = first_comment_id(&json, "comments");
    repo.run_ok(&["patch", "delete-comment", &id, &target[..8]]);

    let out = repo.run_ok(&["patch", "show", &id]);
    assert!(
        out.contains("[deleted]"),
        "a deleted comment should read as deleted: {}",
        out
    );
    assert!(
        !out.contains("regrettable"),
        "the deleted text must not still be rendered: {}",
        out
    );
}

/// Deleted text must not leak out through the side doors either.
#[test]
fn a_deleted_comment_stops_matching_search() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("search leak");
    repo.run_ok(&["patch", "comment", &id, "-b", "chartreuse blunder"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let target = first_comment_id(&json, "comments");

    let out = repo.run_ok(&["search", "chartreuse"]);
    assert!(out.contains("comment match"), "precondition: {}", out);

    repo.run_ok(&["patch", "delete-comment", &id, &target[..8]]);
    let out = repo.run_ok(&["search", "chartreuse"]);
    assert!(
        !out.contains("comment match"),
        "deleted text must not remain searchable: {}",
        out
    );
}

/// A review carries a vote, so removing it would silently drop a verdict.
/// Change the vote with a new review instead.
#[test]
fn a_review_cannot_be_deleted() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("no review delete");
    repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "lgtm"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let review_id = first_comment_id(&json, "reviews");

    let err = repo.run_err(&["patch", "delete-comment", &id, &review_id[..8]]);
    assert!(
        err.contains("review"),
        "the refusal should explain that this is a review: {}",
        err
    );
}

// ===========================================================================
// Attribution: a different key must not rewrite someone else's words
// ===========================================================================

/// The fold is what every reader sees, and anyone can append anything to a
/// DAG they have a copy of. So the rule has to hold in the fold, not only at
/// the CLI: an edit whose author is not the comment's author is ignored.
#[test]
fn an_edit_by_a_different_author_is_ignored_by_the_fold() {
    let dir = TempDir::new().unwrap();
    let repo = init_repo(dir.path(), &alice());
    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);

    append(
        &repo,
        &ref_name,
        &bob(),
        Action::BodyEdit {
            target: comment_oid.to_string(),
            body: "Bob's forgery".to_string(),
        },
    );

    let state = issue_state(&repo, &ref_name, &id);
    assert_eq!(
        state.comments[0].body, "original text",
        "an edit from another key must not rewrite the comment"
    );
    assert!(
        !state.comments[0].edited,
        "and must not even mark it as edited"
    );
}

#[test]
fn a_delete_by_a_different_author_is_ignored_by_the_fold() {
    let dir = TempDir::new().unwrap();
    let repo = init_repo(dir.path(), &alice());
    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);

    append(
        &repo,
        &ref_name,
        &bob(),
        Action::CommentDelete {
            target: comment_oid.to_string(),
        },
    );

    let state = issue_state(&repo, &ref_name, &id);
    assert_eq!(state.comments[0].body, "original text");
    assert!(!state.comments[0].deleted);
}

/// An honest user gets told, rather than silently no-op'd.
#[test]
fn the_cli_refuses_to_edit_someone_elses_comment() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("not yours");
    repo.run_ok(&["patch", "comment", &id, "-b", "Alice wrote this"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");

    // Same repo, different committer identity.
    repo.git(&["config", "user.name", "Bob"]);
    repo.git(&["config", "user.email", "bob@example.com"]);

    let err = repo.run_err(&[
        "patch",
        "edit-comment",
        &id,
        &comment_id[..8],
        "-b",
        "Bob rewrites history",
    ]);
    assert!(
        err.contains("alice@example.com"),
        "the refusal should name the author who owns the comment: {}",
        err
    );

    repo.git(&["config", "user.name", "Alice"]);
    repo.git(&["config", "user.email", "alice@example.com"]);
    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    assert_eq!(body_at(&json, "/comments/0/body"), "Alice wrote this");
}

// ===========================================================================
// Ordering and convergence
// ===========================================================================

#[test]
fn an_edited_comment_keeps_its_position() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.issue_open("ordering");
    repo.run_ok(&["issue", "comment", &id, "-b", "first"]);
    repo.run_ok(&["issue", "comment", &id, "-b", "second"]);
    repo.run_ok(&["issue", "comment", &id, "-b", "third"]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    let target = value["comments"][0]["commit_id"]
        .as_str()
        .unwrap()
        .to_string();

    repo.run_ok(&["issue", "edit-comment", &id, &target[..8], "-b", "FIRST"]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    let bodies: Vec<&str> = value["comments"]
        .as_array()
        .unwrap()
        .iter()
        .map(|c| c["body"].as_str().unwrap())
        .collect();
    assert_eq!(
        bodies,
        vec!["FIRST", "second", "third"],
        "an edited comment stays where it was"
    );
}

/// Two clones edit the same comment while apart. Once they see each other's
/// events, every clone must land on the same text — decided by `(clock, oid)`
/// over the event set, which is content-derived and so client-independent.
#[test]
fn concurrent_edits_of_one_comment_converge() {
    let dir = TempDir::new().unwrap();
    let repo = init_repo(dir.path(), &alice());
    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);
    let fork_point = repo.refname_to_id(&ref_name).unwrap();

    // Alice's clone edits.
    let a_edit = append(
        &repo,
        &ref_name,
        &alice(),
        Action::BodyEdit {
            target: comment_oid.to_string(),
            body: "edit from clone A".to_string(),
        },
    );
    let a_tip = repo.refname_to_id(&ref_name).unwrap();

    // Her other clone, offline, edits the same comment from the fork point.
    let branch_ref = "refs/collab/issues/other-clone";
    repo.reference(branch_ref, fork_point, false, "fork")
        .unwrap();
    let b_edit = append(
        &repo,
        branch_ref,
        &alice(),
        Action::BodyEdit {
            target: comment_oid.to_string(),
            body: "edit from clone B".to_string(),
        },
    );

    // Reconcile in both directions and check the two agree.
    let sk = test_signing_key();
    let merged_ref = "refs/collab/issues/merged";
    repo.reference(merged_ref, a_tip, false, "copy").unwrap();
    dag::reconcile(&repo, merged_ref, branch_ref, &alice(), &sk).unwrap();
    let one = issue_state(&repo, merged_ref, &id);

    let other_ref = "refs/collab/issues/merged-other-way";
    repo.reference(other_ref, b_edit, false, "copy").unwrap();
    dag::reconcile(&repo, other_ref, &ref_name, &alice(), &sk).unwrap();
    let two = issue_state(&repo, other_ref, &id);

    assert_eq!(
        one.comments[0].body, two.comments[0].body,
        "both join orders must fold to the same text"
    );

    // And the winner is the one decided by (clock, oid): equal clocks here,
    // so the lexicographically larger OID wins.
    let expected = if a_edit.to_string() > b_edit.to_string() {
        "edit from clone A"
    } else {
        "edit from clone B"
    };
    assert_eq!(one.comments[0].body, expected);
}

/// A delete racing an edit is the same conflict, resolved the same way.
#[test]
fn a_delete_and_an_edit_racing_converge() {
    let dir = TempDir::new().unwrap();
    let repo = init_repo(dir.path(), &alice());
    let (ref_name, id, comment_oid) = open_issue_with_comment(&repo);
    let fork_point = repo.refname_to_id(&ref_name).unwrap();

    let edit_oid = append(
        &repo,
        &ref_name,
        &alice(),
        Action::BodyEdit {
            target: comment_oid.to_string(),
            body: "kept after all".to_string(),
        },
    );
    let a_tip = repo.refname_to_id(&ref_name).unwrap();

    let branch_ref = "refs/collab/issues/delete-clone";
    repo.reference(branch_ref, fork_point, false, "fork")
        .unwrap();
    let delete_oid = append(
        &repo,
        branch_ref,
        &alice(),
        Action::CommentDelete {
            target: comment_oid.to_string(),
        },
    );

    let sk = test_signing_key();
    let merged_ref = "refs/collab/issues/dm";
    repo.reference(merged_ref, a_tip, false, "copy").unwrap();
    dag::reconcile(&repo, merged_ref, branch_ref, &alice(), &sk).unwrap();
    let one = issue_state(&repo, merged_ref, &id);

    let other_ref = "refs/collab/issues/dm-other";
    repo.reference(other_ref, delete_oid, false, "copy")
        .unwrap();
    dag::reconcile(&repo, other_ref, &ref_name, &alice(), &sk).unwrap();
    let two = issue_state(&repo, other_ref, &id);

    assert_eq!(one.comments[0].deleted, two.comments[0].deleted);
    assert_eq!(one.comments[0].body, two.comments[0].body);

    let delete_wins = delete_oid.to_string() > edit_oid.to_string();
    assert_eq!(one.comments[0].deleted, delete_wins);
}

// ===========================================================================
// Revision bodies (7a299d2c)
// ===========================================================================

#[test]
fn a_revision_body_can_be_corrected() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("revision body");

    repo.git(&["checkout", "test/revision-body"]);
    repo.commit_file("more.txt", "more", "second commit");
    repo.run_ok(&["patch", "revise", &id, "-b", "test"]);
    repo.git(&["checkout", "main"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    let commit_before = value["revisions"][1]["commit"]
        .as_str()
        .unwrap()
        .to_string();
    assert_eq!(value["revisions"][1]["body"], "test");

    repo.run_ok(&[
        "patch",
        "edit-revision",
        &id,
        "2",
        "-b",
        "Rework the trailer scan in response to review",
    ]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let value: serde_json::Value = serde_json::from_str(&json).unwrap();
    assert_eq!(
        value["revisions"][1]["body"],
        "Rework the trailer scan in response to review"
    );
    assert_eq!(
        value["revisions"][1]["commit"], commit_before,
        "the revision's commit is immutable; only its description changes"
    );
    assert_eq!(
        value["revisions"].as_array().unwrap().len(),
        2,
        "correcting a body must not manufacture a revision"
    );
}

#[test]
fn editing_an_unknown_revision_is_an_error() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("no such revision");

    let err = repo.run_err(&["patch", "edit-revision", &id, "9", "-b", "x"]);
    assert!(
        err.contains('9'),
        "error should name the revision that does not exist: {}",
        err
    );
}

#[test]
fn a_revision_body_edit_by_a_different_author_is_ignored() {
    let dir = TempDir::new().unwrap();
    let repo = init_repo(dir.path(), &alice());
    let (ref_name, id) = common::create_patch(&repo, &alice(), "revision attribution");

    let revision_oid = append(
        &repo,
        &ref_name,
        &alice(),
        Action::PatchRevision {
            commit: "a".repeat(40),
            tree: "b".repeat(40),
            body: Some("mine".to_string()),
            base: None,
        },
    );
    append(
        &repo,
        &ref_name,
        &bob(),
        Action::BodyEdit {
            target: revision_oid.to_string(),
            body: "Bob's rewrite".to_string(),
        },
    );

    let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.revisions[1].body.as_deref(), Some("mine"));
}

// ===========================================================================
// Reading must never write
// ===========================================================================

/// This project has shipped a "display appends an event" bug before. Every
/// read path is checked against the events ref, which is the one that carries
/// history: showing, diffing and logging a patch must leave it exactly where
/// it was.
#[test]
fn reading_a_patch_never_moves_its_event_ref() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("read only");
    repo.run_ok(&["patch", "comment", &id, "-b", "a comment"]);
    repo.run_ok(&["patch", "review", &id, "-v", "comment", "-b", "a review"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");
    repo.run_ok(&["patch", "delete-comment", &id, &comment_id[..8]]);

    let events_ref = repo
        .git(&[
            "for-each-ref",
            "--format=%(refname)",
            "refs/collab/patches/",
        ])
        .lines()
        .find(|l| l.ends_with("/events"))
        .expect("patch events ref")
        .to_string();

    let before = repo.git(&["rev-parse", &events_ref]).trim().to_string();

    repo.run_ok(&["patch", "show", &id]);
    repo.run_ok(&["patch", "show", &id, "--json"]);
    repo.run_ok(&["patch", "log", &id]);
    repo.run_ok(&["patch", "log", &id, "--json"]);
    repo.run_ok(&["patch", "diff", &id]);
    repo.run_ok(&["patch", "list"]);
    repo.run_ok(&["search", "comment"]);

    let after = repo.git(&["rev-parse", &events_ref]).trim().to_string();
    assert_eq!(before, after, "reading a patch must not append events");
}

#[test]
fn reading_an_issue_never_moves_its_ref() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.issue_open("read only issue");
    repo.run_ok(&["issue", "comment", &id, "-b", "a comment"]);

    let issue_ref = repo
        .git(&["for-each-ref", "--format=%(refname)", "refs/collab/issues/"])
        .lines()
        .next()
        .expect("issue ref")
        .to_string();
    let before = repo.git(&["rev-parse", &issue_ref]).trim().to_string();

    repo.run_ok(&["issue", "show", &id]);
    repo.run_ok(&["issue", "show", &id, "--json"]);
    repo.run_ok(&["issue", "list"]);
    repo.run_ok(&["search", "comment"]);

    let after = repo.git(&["rev-parse", &issue_ref]).trim().to_string();
    assert_eq!(before, after, "reading an issue must not append events");
}

// ===========================================================================
// The original events stay in the DAG
// ===========================================================================

/// Editing supersedes; it does not rewrite. The original event is still
/// there, which is what makes the log an audit trail rather than a summary.
#[test]
fn an_edit_leaves_the_original_event_in_the_log() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.issue_open("audit trail");
    repo.run_ok(&["issue", "comment", &id, "-b", "the original wording"]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");
    repo.run_ok(&[
        "issue",
        "edit-comment",
        &id,
        &comment_id[..8],
        "-b",
        "the revised wording",
    ]);

    let log = repo.run_ok(&["log"]);
    assert!(
        log.contains("IssueComment") && log.contains("BodyEdit"),
        "both events should appear in the raw event log: {}",
        log
    );
    assert!(
        log.contains("the original wording") && log.contains("the revised wording"),
        "the log should show what was said before and after: {}",
        log
    );
    assert!(
        log.contains(&format!("edit body of {:.8}", comment_id)),
        "the edit should name the event it supersedes: {}",
        log
    );

    // And the original body is still readable from the event object itself.
    let blob = repo.git(&["show", &format!("{}:event.json", comment_id)]);
    assert!(
        blob.contains("the original wording"),
        "the superseded event is unchanged in the DAG"
    );
}

#[test]
fn an_edited_comment_is_marked_as_edited() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.issue_open("edited marker");
    repo.run_ok(&["issue", "comment", &id, "-b", "before"]);

    let json = repo.run_ok(&["issue", "show", &id, "--json"]);
    let comment_id = first_comment_id(&json, "comments");
    repo.run_ok(&[
        "issue",
        "edit-comment",
        &id,
        &comment_id[..8],
        "-b",
        "after",
    ]);

    let out = repo.run_ok(&["issue", "show", &id]);
    assert!(
        out.contains("edited"),
        "a corrected comment should say so: {}",
        out
    );
}

/// Comment ids have to be visible, or nothing above can be addressed.
#[test]
fn show_prints_addressable_comment_ids() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("ids");
    repo.run_ok(&["patch", "comment", &id, "-b", "thread"]);
    repo.run_ok(&[
        "patch", "comment", &id, "--file", "ids.txt", "--line", "1", "-b", "inline",
    ]);
    repo.run_ok(&["patch", "review", &id, "-v", "comment", "-b", "review"]);

    let json = repo.run_ok(&["patch", "show", &id, "--json"]);
    let out = repo.run_ok(&["patch", "show", &id]);

    for list in ["comments", "inline_comments", "reviews"] {
        let full = first_comment_id(&json, list);
        assert!(
            out.contains(&full[..8]),
            "`patch show` should print the short id for {}: {}",
            list,
            out
        );
    }
}

/// A verdict that is not a comment still needs its author checked, so make
/// sure the plumbing for reviews goes through the same rule.
#[test]
fn a_review_edit_by_a_different_author_is_ignored_by_the_fold() {
    let dir = TempDir::new().unwrap();
    let repo = init_repo(dir.path(), &alice());
    let (ref_name, id) = common::create_patch(&repo, &alice(), "review attribution");

    let review_oid = append(
        &repo,
        &ref_name,
        &alice(),
        Action::PatchReview {
            verdict: ReviewVerdict::Comment,
            body: "Alice's review".to_string(),
            revision: Some(1),
        },
    );
    append(
        &repo,
        &ref_name,
        &bob(),
        Action::BodyEdit {
            target: review_oid.to_string(),
            body: "Bob's rewrite".to_string(),
        },
    );

    let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.reviews[0].body, "Alice's review");
}