michal/tit
Browse tree · Show commit · Download archive
Diff
ff182b9ee47d → d2a3a4bfd3a5
README.md
Mode 100644 → 100644; object e7c73d347841 → 511acedc90aa
@@ -404,3 +404,18 @@ work limits, and Web output. Read the [pull-request comparison architectural decision record](docs/adr/0023-pull-request-comparison.md) for the computation, limit, and cache contracts. + +## Milestone 5.3 gate + +Run the pull-request review gate: + +```text +./scripts/check-m5-3 +``` + +This command tests general comments, line comments, approvals, change requests, +immutable byte-path anchors, outdated display, repository permission, atomic +events, the chronological timeline, no-JavaScript Web forms, and historical +schema migration. Read the +[pull-request review architectural decision record](docs/adr/0024-pull-request-review.md) +for the anchor, permission, event, and outdated-state contracts.
docs/adr/0024-pull-request-review.md
Mode → 100644; object → 4f9b5e24ba5b
@@ -1,0 +1,85 @@ +# Architectural decision record 0024: Pull-request review + +Status: Accepted + +Date: 2026-07-23 + +## Context + +A review must keep its original meaning after a branch update. General comments +need Markdown content. A line comment also needs an exact Git anchor. Approvals +and change requests must remain in the history. Each metadata mutation must +have one repository event in the same SQLite transaction. + +## Decision + +Store each review action as an append-only `pull_request_review` record with a +random permanent ID. Every action refers to one immutable pull-request +revision and one active account. Use these action kinds: + +- `comment` stores a general comment. +- `line-comment` stores a line comment. +- `approved` stores an approval. +- `changes-requested` stores a change request. + +A line comment stores the exact commit ID, path bytes, side, and one-based line +number. The side is `base` or `head`. The commit ID must equal that side of the +selected revision. The path must be a changed, non-binary file in that +revision. The selected side must contain the path and line. Perform the Git +checks with the bounded comparison and blob reader. Recheck the revision and +commit relation in the write transaction. + +Store raw Markdown with a maximum size of 256 KiB. Use the common safe Markdown +renderer for Web output. A comment and a change request require nonempty text. +An approval can have empty text. + +An active account that can read the repository can review. Thus, a repository +reader can comment, approve, or request changes. Apply the current repository +policy again in the write transaction. Do not accept review actions on a pull +request that is not open. + +Append one specific repository event for each review action. The event payload +contains the review ID, pull-request number, revision number, body, and optional +line anchor. Repository event sequence gives one chronological timeline for +creation, revisions, and review actions. + +## Outdated comments + +A line comment is current only while its revision is the current pull-request +revision. Show `Outdated` after the pull request gets a later revision. Keep the +original commit, path, side, line, and Markdown visible. General comments, +approvals, and change requests stay in the timeline and do not get the +`Outdated` label. + +This rule is conservative. It does not move an anchor to a similar line in a +later commit. Automatic line mapping can change the meaning of a comment, so it +needs a separate design and tests before use. + +## Failure and threat cases + +Reject an unknown revision, invalid kind, invalid or oversized body, missing +path, binary file, unchanged path, absent side, absent line, line outside the +file, and commit mismatch. Store path data as bytes so a Git path does not need +UTF-8. The no-JavaScript Web form sends the path as hexadecimal text. + +If review insertion or event insertion fails, roll back both records. A later +permission change can hide existing review data because reads use the current +repository policy. + +## Evidence + +The integration test uses SHA-1 and SHA-256 repositories. A reader adds a +general comment, approval, and line comment. An owner requests changes. The +test checks the exact line anchor, rejects a missing line, records a later +revision, verifies event order, and denies the reader after private access is +removed. + +The Web integration test submits review actions without JavaScript, renders +safe Markdown, shows the review events, and marks a line comment as outdated +after a later revision. The migration test upgrades every committed schema. + +## Consequences + +Review history is durable and reproducible from SQLite and immutable Git IDs. +The service does not silently move line comments. A later branch-rule milestone +can consume approval and change-request records without changing their history.
scripts/check-m5-3
Mode → 100755; object → 305f497f83a6
@@ -1,0 +1,7 @@ +#!/bin/sh +set -eu + +./scripts/check +cargo test --locked --release --test pull_requests +cargo test --locked --release --test public_routes runs_the_complete_issue_workflow_without_javascript +cargo test --locked --release --test sqlite migrates_each_committed_historical_fixture
src/feed.rs
Mode 100644 → 100644; object fd8f4f6926b4 → 6083ae4415b4
@@ -296,6 +296,10 @@
"issue-unassigned" => issue_value_title(event, "unassigned", "assignee"),
"pull-request-created" => pull_request_title(event, "opened"),
"pull-request-revised" => pull_request_title(event, "revised"),
+ "pull-request-commented" => pull_request_title(event, "commented on"),
+ "pull-request-line-commented" => pull_request_title(event, "commented on a line in"),
+ "pull-request-approved" => pull_request_title(event, "approved"),
+ "pull-request-changes-requested" => pull_request_title(event, "requested changes on"),
_ => "Repository event".to_owned(),
}
}
src/http/pull_requests.rs
Mode 100644 → 100644; object acaacc3275be → 7caddf3b99d2
@@ -32,6 +32,10 @@
"/{owner}/{repository}/pulls/{number}/revisions",
post(revise_pull_request),
)
+ .route(
+ "/{owner}/{repository}/pulls/{number}/reviews",
+ post(create_review),
+ )
.layer(DefaultBodyLimit::max(MAX_PULL_REQUEST_BYTES))
}
@@ -116,14 +120,122 @@
pull_request,
body_html: markdown::render(&pull_request.body),
revisions: &detail.revisions,
+ reviews: detail
+ .reviews
+ .iter()
+ .map(|review| ReviewView {
+ id: &review.id,
+ revision: review.revision,
+ author: &review.author,
+ kind: &review.kind,
+ body_html: markdown::render(&review.body),
+ has_body: !review.body.is_empty(),
+ commit_object_id: review.commit_object_id.as_deref().unwrap_or(""),
+ path: review
+ .path
+ .as_deref()
+ .map(|path| String::from_utf8_lossy(path).into_owned())
+ .unwrap_or_default(),
+ side: review.side.as_deref().unwrap_or(""),
+ line: review
+ .line
+ .map_or_else(String::new, |line| line.to_string()),
+ outdated: review.kind == "line-comment"
+ && review.revision != pull_request_revision(detail),
+ created_at: review.created_at,
+ })
+ .collect(),
+ timeline: detail
+ .timeline
+ .iter()
+ .map(|event| TimelineView {
+ sequence: event.sequence,
+ kind: &event.kind,
+ actor: &event.actor,
+ created_at: event.created_at,
+ })
+ .collect(),
selected_revision: result.revision.number,
comparison: ComparisonView::from(&result.comparison),
csrf: &csrf,
can_revise: detail.can_revise && !csrf.is_empty(),
+ can_review: detail.can_review
+ && !csrf.is_empty()
+ && result.revision.number == pull_request_revision(detail),
},
)
}
Err(error) => read_error(error, &request_id.0),
+ }
+}
+
+async fn create_review(
+ State(state): State<WebState>,
+ Extension(request_id): Extension<RequestId>,
+ Path(path): Path<PullRequestPath>,
+ headers: HeaderMap,
+ body: Bytes,
+) -> Response {
+ let fields = match parse_named_form(
+ &headers,
+ &body,
+ &[
+ "csrf", "revision", "kind", "body", "path-hex", "side", "line",
+ ],
+ ) {
+ Ok(fields) => fields,
+ Err(()) => return bad_request(&request_id.0),
+ };
+ let actor =
+ match authenticate_mutation(state.clone(), &headers, &fields[0], &request_id.0).await {
+ Ok(actor) => actor,
+ Err(response) => return response,
+ };
+ let revision = match fields[1].parse::<i64>() {
+ Ok(revision) => revision,
+ Err(_) => return bad_request(&request_id.0),
+ };
+ let path_bytes = if fields[4].is_empty() {
+ None
+ } else {
+ match decode_hex(&fields[4]) {
+ Some(path) => Some(path),
+ None => return bad_request(&request_id.0),
+ }
+ };
+ let side = (!fields[5].is_empty()).then(|| fields[5].clone());
+ let line = if fields[6].is_empty() {
+ None
+ } else {
+ match fields[6].parse::<i64>() {
+ Ok(line) => Some(line),
+ Err(_) => return bad_request(&request_id.0),
+ }
+ };
+ let Some(service) = state.pull_requests.clone() else {
+ return internal(&request_id.0);
+ };
+ let owner = path.owner.clone();
+ let repository = path.repository.clone();
+ let number = path.number;
+ let result = job(state, move || {
+ service.review(
+ &owner,
+ &repository,
+ number,
+ revision,
+ &actor,
+ &fields[2],
+ &fields[3],
+ path_bytes.as_deref(),
+ side.as_deref(),
+ line,
+ )
+ })
+ .await;
+ match result {
+ Ok(_) => redirect(&path.owner, &path.repository, number),
+ Err(error) => mutation_error(error, &request_id.0),
}
}
@@ -245,6 +357,12 @@
"Pull-request error",
"You cannot change pull requests in this repository.",
),
+ PullRequestError::Store(StoreError::PullRequestState) => render_error(
+ StatusCode::CONFLICT,
+ request_id,
+ "Pull-request conflict",
+ "The pull request is not open.",
+ ),
PullRequestError::Store(
StoreError::RepositoryNotFound(_, _)
| StoreError::PullRequestNotFound(_, _, _)
@@ -255,11 +373,38 @@
| PullRequestError::Branch
| PullRequestError::Number
| PullRequestError::Unchanged
+ | PullRequestError::Revision
+ | PullRequestError::ReviewKind
+ | PullRequestError::ReviewBody
+ | PullRequestError::ReviewAnchor
+ | PullRequestError::Store(StoreError::PullRequestRevisionNotFound)
+ | PullRequestError::Store(StoreError::PullRequestReviewAnchor)
| PullRequestError::Git(crate::git::repository::GitRepositoryError::MissingReference(_)) => {
bad_request(request_id)
}
_ => internal(request_id),
}
+}
+
+fn pull_request_revision(detail: &crate::store::PullRequestDetail) -> i64 {
+ detail
+ .revisions
+ .last()
+ .map_or(0, |revision| revision.number)
+}
+
+fn decode_hex(value: &str) -> Option<Vec<u8>> {
+ if value.is_empty() || !value.len().is_multiple_of(2) {
+ return None;
+ }
+ value
+ .as_bytes()
+ .chunks_exact(2)
+ .map(|pair| {
+ let pair = std::str::from_utf8(pair).ok()?;
+ u8::from_str_radix(pair, 16).ok()
+ })
+ .collect()
}
fn bad_request(request_id: &str) -> Response {
@@ -338,10 +483,35 @@
pull_request: &'a crate::store::PullRequestRecord,
body_html: RenderedMarkdown,
revisions: &'a [crate::store::PullRequestRevisionRecord],
+ reviews: Vec<ReviewView<'a>>,
+ timeline: Vec<TimelineView<'a>>,
selected_revision: i64,
comparison: ComparisonView,
csrf: &'a str,
can_revise: bool,
+ can_review: bool,
+}
+
+struct ReviewView<'a> {
+ id: &'a str,
+ revision: i64,
+ author: &'a str,
+ kind: &'a str,
+ body_html: RenderedMarkdown,
+ has_body: bool,
+ commit_object_id: &'a str,
+ path: String,
+ side: &'a str,
+ line: String,
+ outdated: bool,
+ created_at: i64,
+}
+
+struct TimelineView<'a> {
+ sequence: i64,
+ kind: &'a str,
+ actor: &'a str,
+ created_at: i64,
}
struct ComparisonView {
@@ -359,7 +529,10 @@
struct DiffView {
path: String,
+ path_hex: String,
binary: bool,
+ has_base: bool,
+ has_head: bool,
hunks: String,
}
@@ -396,10 +569,22 @@
.iter()
.map(|file| DiffView {
path: String::from_utf8_lossy(&file.path).into_owned(),
+ path_hex: encode_hex(&file.path),
binary: file.binary,
+ has_base: file.old_id.is_some(),
+ has_head: file.new_id.is_some(),
hunks: String::from_utf8_lossy(&file.hunks).into_owned(),
})
.collect(),
}
}
+}
+
+fn encode_hex(bytes: &[u8]) -> String {
+ let mut encoded = String::with_capacity(bytes.len().saturating_mul(2));
+ for byte in bytes {
+ use std::fmt::Write;
+ write!(encoded, "{byte:02x}").expect("a string write cannot fail");
+ }
+ encoded
}
src/pull_request.rs
Mode 100644 → 100644; object 103ba52412fc → e1fb41428065
@@ -14,12 +14,13 @@
};
use crate::git::repository::{GitRepository, GitRepositoryError};
use crate::store::{
- NewPullRequestRefIntent, PullRequestDetail, PullRequestRecord, PullRequestRefIntentRecord,
- PullRequestRevisionRecord, Store, StoreError,
+ NewPullRequestRefIntent, NewPullRequestReview, PullRequestDetail, PullRequestRecord,
+ PullRequestRefIntentRecord, PullRequestRevisionRecord, Store, StoreError,
};
pub(crate) const MAX_TITLE_BYTES: usize = 200;
pub(crate) const MAX_BODY_BYTES: usize = 256 * 1024;
+pub(crate) const MAX_REVIEW_BODY_BYTES: usize = 256 * 1024;
#[derive(Clone)]
pub(crate) struct PullRequestService {
@@ -237,6 +238,96 @@
}
#[allow(
+ clippy::too_many_arguments,
+ reason = "a review action includes its repository, revision, content, and optional line anchor"
+ )]
+ pub(crate) fn review(
+ &self,
+ owner: &str,
+ repository: &str,
+ number: i64,
+ revision_number: i64,
+ actor: &str,
+ kind: &str,
+ body: &str,
+ path: Option<&[u8]>,
+ side: Option<&str>,
+ line: Option<i64>,
+ ) -> Result<String, PullRequestError> {
+ validate_context(owner, repository, actor)?;
+ if number < 1 || revision_number < 1 {
+ return Err(PullRequestError::Number);
+ }
+ validate_review(kind, body, path, side, line)?;
+ let _operation = self
+ .operations
+ .lock()
+ .unwrap_or_else(std::sync::PoisonError::into_inner);
+ self.recover_inner()?;
+ let detail =
+ Store::open(&self.database)?.pull_request(owner, repository, number, Some(actor))?;
+ let revision = detail
+ .revisions
+ .iter()
+ .find(|revision| revision.number == revision_number)
+ .ok_or(PullRequestError::Revision)?;
+ let commit_object_id = if kind == "line-comment" {
+ let path = path.ok_or(PullRequestError::ReviewAnchor)?;
+ let side = side.ok_or(PullRequestError::ReviewAnchor)?;
+ let line = line.ok_or(PullRequestError::ReviewAnchor)?;
+ let path_to_repository = self.repository_path(&detail.repository.id)?;
+ let reader = RepositoryReadService::open(&path_to_repository, ReadLimits::default())?;
+ let base = parse_id(&revision.base_object_id)?;
+ let head = parse_id(&revision.head_object_id)?;
+ let comparison = reader.comparison(base, head, &ReadCancellation::default())?;
+ let file = comparison
+ .files
+ .iter()
+ .find(|file| file.path == path)
+ .filter(|file| !file.binary)
+ .ok_or(PullRequestError::ReviewAnchor)?;
+ let (commit, exists) = match side {
+ "base" => (base, file.old_id.is_some()),
+ "head" => (head, file.new_id.is_some()),
+ _ => return Err(PullRequestError::ReviewAnchor),
+ };
+ if !exists {
+ return Err(PullRequestError::ReviewAnchor);
+ }
+ let blob = reader.blob(commit, path, &ReadCancellation::default())?;
+ let line_count = if blob.data.is_empty() {
+ 0
+ } else {
+ blob.data.split(|byte| *byte == b'\n').count()
+ - usize::from(blob.data.ends_with(b"\n"))
+ };
+ let line = usize::try_from(line).map_err(|_| PullRequestError::ReviewAnchor)?;
+ if line == 0 || line > line_count {
+ return Err(PullRequestError::ReviewAnchor);
+ }
+ Some(commit.to_string())
+ } else {
+ None
+ };
+ Store::open(&self.database)?
+ .create_pull_request_review(&NewPullRequestReview {
+ owner,
+ repository,
+ number,
+ revision: revision_number,
+ actor,
+ kind,
+ body,
+ commit_object_id: commit_object_id.as_deref(),
+ path,
+ side,
+ line,
+ created_at: timestamp()?,
+ })
+ .map_err(Into::into)
+ }
+
+ #[allow(
dead_code,
reason = "some integration tests compile the service without the Web list route"
)]
@@ -361,6 +452,48 @@
Ok(())
}
+fn validate_review(
+ kind: &str,
+ body: &str,
+ path: Option<&[u8]>,
+ side: Option<&str>,
+ line: Option<i64>,
+) -> Result<(), PullRequestError> {
+ if body.len() > MAX_REVIEW_BODY_BYTES
+ || body
+ .chars()
+ .any(|character| character.is_control() && !matches!(character, '\n' | '\r' | '\t'))
+ {
+ return Err(PullRequestError::ReviewBody);
+ }
+ match kind {
+ "comment" | "changes-requested" if !body.trim().is_empty() => {
+ if path.is_none() && side.is_none() && line.is_none() {
+ Ok(())
+ } else {
+ Err(PullRequestError::ReviewAnchor)
+ }
+ }
+ "approved" if path.is_none() && side.is_none() && line.is_none() => Ok(()),
+ "line-comment" if !body.trim().is_empty() => {
+ let path = path.ok_or(PullRequestError::ReviewAnchor)?;
+ if path.is_empty()
+ || path.len() > ReadLimits::default().max_path_bytes
+ || path.contains(&0)
+ || !matches!(side, Some("base" | "head"))
+ || line.is_none_or(|line| line < 1)
+ {
+ return Err(PullRequestError::ReviewAnchor);
+ }
+ Ok(())
+ }
+ "comment" | "line-comment" | "approved" | "changes-requested" => {
+ Err(PullRequestError::ReviewBody)
+ }
+ _ => Err(PullRequestError::ReviewKind),
+ }
+}
+
fn pull_request_ref(number: i64) -> String {
format!("refs/pull/{number}/head")
}
@@ -425,6 +558,12 @@
Number,
#[error("pull-request revision does not exist")]
Revision,
+ #[error("pull-request review kind is not valid")]
+ ReviewKind,
+ #[error("pull-request review body is not valid")]
+ ReviewBody,
+ #[error("pull-request review line anchor is not valid")]
+ ReviewAnchor,
#[error("pull-request refs have not changed")]
Unchanged,
#[error("stored pull-request object ID is not valid")]
src/store/event.rs
Mode 100644 → 100644; object 2fd644f2bd56 → 98afed30bd53
@@ -24,6 +24,10 @@
IssueUnassigned,
PullRequestCreated,
PullRequestRevised,
+ PullRequestCommented,
+ PullRequestLineCommented,
+ PullRequestApproved,
+ PullRequestChangesRequested,
}
impl EventKind {
@@ -49,7 +53,52 @@
Self::IssueUnassigned => "issue-unassigned",
Self::PullRequestCreated => "pull-request-created",
Self::PullRequestRevised => "pull-request-revised",
+ Self::PullRequestCommented => "pull-request-commented",
+ Self::PullRequestLineCommented => "pull-request-line-commented",
+ Self::PullRequestApproved => "pull-request-approved",
+ Self::PullRequestChangesRequested => "pull-request-changes-requested",
}
+ }
+}
+
+#[allow(
+ clippy::too_many_arguments,
+ reason = "the event payload stores the complete immutable review anchor"
+)]
+pub(super) fn pull_request_review(
+ kind: EventKind,
+ pull_request_id: &str,
+ number: i64,
+ review_id: &str,
+ revision: i64,
+ body: &str,
+ commit_object_id: Option<&str>,
+ path: Option<&[u8]>,
+ side: Option<&str>,
+ line: Option<i64>,
+) -> VersionedEvent {
+ debug_assert!(matches!(
+ kind,
+ EventKind::PullRequestCommented
+ | EventKind::PullRequestLineCommented
+ | EventKind::PullRequestApproved
+ | EventKind::PullRequestChangesRequested
+ ));
+ VersionedEvent {
+ kind,
+ payload: json!({
+ "version": PAYLOAD_VERSION,
+ "pull_request_id": pull_request_id,
+ "number": number,
+ "review_id": review_id,
+ "revision": revision,
+ "body": body,
+ "commit_object_id": commit_object_id,
+ "path_hex": path.map(encode_hex),
+ "side": side,
+ "line": line,
+ })
+ .to_string(),
}
}
src/store/migrations/016_pull_request_reviews.sql
Mode → 100644; object → ae9219c9bd48
@@ -1,0 +1,150 @@
+CREATE TABLE pull_request_review (
+ id TEXT PRIMARY KEY
+ CHECK (
+ length(id) = 32
+ AND id = lower(id)
+ AND id NOT GLOB '*[^0-9a-f]*'
+ ),
+ pull_request_id TEXT NOT NULL
+ REFERENCES pull_request (id) ON DELETE RESTRICT,
+ revision_id TEXT NOT NULL
+ REFERENCES pull_request_revision (id) ON DELETE RESTRICT,
+ author_account_id INTEGER NOT NULL
+ REFERENCES account (id) ON DELETE RESTRICT,
+ kind TEXT NOT NULL
+ CHECK (kind IN ('comment', 'line-comment', 'approved', 'changes-requested')),
+ body TEXT NOT NULL CHECK (length(CAST(body AS BLOB)) <= 262144),
+ commit_object_id TEXT
+ CHECK (
+ commit_object_id IS NULL
+ OR (length(commit_object_id) IN (40, 64)
+ AND commit_object_id = lower(commit_object_id)
+ AND commit_object_id NOT GLOB '*[^0-9a-f]*')
+ ),
+ path BLOB CHECK (path IS NULL OR (length(path) BETWEEN 1 AND 4096 AND instr(path, X'00') = 0)),
+ side TEXT CHECK (side IS NULL OR side IN ('base', 'head')),
+ line INTEGER CHECK (line IS NULL OR line >= 1),
+ created_at INTEGER NOT NULL CHECK (created_at >= 0),
+ CHECK (
+ (kind = 'line-comment'
+ AND length(CAST(body AS BLOB)) >= 1
+ AND commit_object_id IS NOT NULL
+ AND path IS NOT NULL
+ AND side IS NOT NULL
+ AND line IS NOT NULL)
+ OR
+ (kind != 'line-comment'
+ AND commit_object_id IS NULL
+ AND path IS NULL
+ AND side IS NULL
+ AND line IS NULL)
+ ),
+ CHECK (kind NOT IN ('comment', 'changes-requested') OR length(CAST(body AS BLOB)) >= 1)
+) STRICT;
+
+CREATE INDEX pull_request_review_timeline
+ON pull_request_review (pull_request_id, created_at, id);
+
+CREATE INDEX pull_request_review_status
+ON pull_request_review (pull_request_id, author_account_id, created_at DESC, id DESC)
+WHERE kind IN ('approved', 'changes-requested');
+
+DROP INDEX repository_event_feed;
+DROP INDEX repository_event_issue_timeline;
+DROP INDEX repository_event_pull_request_timeline;
+ALTER TABLE repository_event RENAME TO repository_event_v15;
+
+CREATE TABLE repository_event (
+ id INTEGER PRIMARY KEY,
+ event_id TEXT NOT NULL UNIQUE
+ CHECK (
+ length(event_id) = 32
+ AND event_id = lower(event_id)
+ AND event_id NOT GLOB '*[^0-9a-f]*'
+ ),
+ repository_id TEXT NOT NULL
+ REFERENCES repository (id) ON DELETE RESTRICT,
+ sequence INTEGER NOT NULL CHECK (sequence >= 1),
+ source_intent_id TEXT
+ REFERENCES git_operation_intent (id) ON DELETE RESTRICT,
+ source_ordinal INTEGER CHECK (source_ordinal IS NULL OR source_ordinal >= 0),
+ issue_id TEXT REFERENCES issue (id) ON DELETE RESTRICT,
+ pull_request_id TEXT REFERENCES pull_request (id) ON DELETE RESTRICT,
+ kind TEXT NOT NULL
+ CHECK (kind IN (
+ 'repository-created', 'repository-imported', 'push',
+ 'ref-created', 'ref-updated', 'ref-deleted',
+ 'tag-created', 'tag-updated', 'tag-deleted',
+ 'issue-created', 'issue-edited', 'issue-commented',
+ 'issue-closed', 'issue-reopened',
+ 'issue-labeled', 'issue-unlabeled',
+ 'issue-assigned', 'issue-unassigned',
+ 'pull-request-created', 'pull-request-revised',
+ 'pull-request-commented', 'pull-request-line-commented',
+ 'pull-request-approved', 'pull-request-changes-requested'
+ )),
+ actor TEXT NOT NULL CHECK (length(actor) BETWEEN 1 AND 256),
+ ref_name BLOB,
+ old_target TEXT,
+ new_target TEXT,
+ payload_version INTEGER NOT NULL CHECK (payload_version = 1),
+ payload TEXT NOT NULL
+ CHECK (
+ length(payload) BETWEEN 1 AND 1048576
+ AND CASE WHEN json_valid(payload) THEN
+ json_type(payload) = 'object'
+ AND coalesce(json_extract(payload, '$.version') = payload_version, 0)
+ ELSE 0 END
+ ),
+ created_at INTEGER NOT NULL CHECK (created_at >= 0),
+ UNIQUE (repository_id, sequence),
+ UNIQUE (source_intent_id, source_ordinal),
+ CHECK (
+ (source_intent_id IS NULL AND source_ordinal IS NULL)
+ OR (source_intent_id IS NOT NULL AND source_ordinal IS NOT NULL)
+ ),
+ CHECK (
+ (kind IN ('repository-created', 'repository-imported', 'push')
+ AND issue_id IS NULL AND pull_request_id IS NULL
+ AND ref_name IS NULL AND old_target IS NULL AND new_target IS NULL)
+ OR
+ ((kind LIKE 'ref-%' OR kind LIKE 'tag-%')
+ AND issue_id IS NULL AND pull_request_id IS NULL
+ AND ref_name IS NOT NULL
+ AND (old_target IS NOT NULL OR new_target IS NOT NULL))
+ OR
+ (kind LIKE 'issue-%'
+ AND issue_id IS NOT NULL AND pull_request_id IS NULL
+ AND source_intent_id IS NULL AND source_ordinal IS NULL
+ AND ref_name IS NULL AND old_target IS NULL AND new_target IS NULL)
+ OR
+ (kind LIKE 'pull-request-%'
+ AND issue_id IS NULL AND pull_request_id IS NOT NULL
+ AND source_intent_id IS NULL AND source_ordinal IS NULL
+ AND ref_name IS NULL AND old_target IS NULL AND new_target IS NULL)
+ )
+) STRICT;
+
+INSERT INTO repository_event
+ (id, event_id, repository_id, sequence, source_intent_id, source_ordinal,
+ issue_id, pull_request_id, kind, actor, ref_name, old_target, new_target,
+ payload_version, payload, created_at)
+SELECT
+ id, event_id, repository_id, sequence, source_intent_id, source_ordinal,
+ issue_id, pull_request_id, kind, actor, ref_name, old_target, new_target,
+ payload_version, payload, created_at
+FROM repository_event_v15
+ORDER BY id;
+
+DROP TABLE repository_event_v15;
+
+CREATE INDEX repository_event_feed
+ON repository_event (repository_id, sequence DESC);
+
+CREATE INDEX repository_event_issue_timeline
+ON repository_event (issue_id, sequence)
+WHERE issue_id IS NOT NULL;
+
+CREATE INDEX repository_event_pull_request_timeline
+ON repository_event (pull_request_id, sequence)
+WHERE pull_request_id IS NOT NULL;
src/store/mod.rs
Mode 100644 → 100644; object 78abc9d772a5 → 988f9fcd3f7c
@@ -12,7 +12,7 @@
const BUSY_TIMEOUT: Duration = Duration::from_secs(5);
const BUSY_TIMEOUT_MILLISECONDS: i64 = 5_000;
const MAX_ACTIVE_FEED_TOKENS: i64 = 32;
-const SCHEMA_VERSION: i64 = 15;
+const SCHEMA_VERSION: i64 = 16;
#[allow(
dead_code,
reason = "the integration test imports this module without the CLI operation"
@@ -22,7 +22,7 @@
dead_code,
reason = "M1A proves migrations before the M2 server calls them"
)]
-const MIGRATIONS: [&str; 15] = [
+const MIGRATIONS: [&str; 16] = [
include_str!("migrations/001_initial.sql"),
include_str!("migrations/002_state.sql"),
include_str!("migrations/003_git_intents.sql"),
@@ -38,6 +38,7 @@
include_str!("migrations/013_watches.sql"),
include_str!("migrations/014_feed_tokens.sql"),
include_str!("migrations/015_pull_requests.sql"),
+ include_str!("migrations/016_pull_request_reviews.sql"),
];
#[allow(
@@ -142,6 +143,10 @@
PullRequestHidden,
#[error("pull request is not open")]
PullRequestState,
+ #[error("pull-request revision does not exist")]
+ PullRequestRevisionNotFound,
+ #[error("pull-request review anchor does not match its revision")]
+ PullRequestReviewAnchor,
#[error("pull-request ref intent {0} is not in the required state")]
PullRequestIntentState(String),
#[error("repository watch access is not authorized")]
@@ -1802,13 +1807,172 @@
})
})?
.collect::<Result<Vec<_>, _>>()?;
+ let reviews = {
+ let mut statement = self.connection.prepare(
+ "SELECT pull_request_review.id, pull_request_revision.number,
+ author.username, pull_request_review.kind,
+ pull_request_review.body, pull_request_review.commit_object_id,
+ pull_request_review.path, pull_request_review.side,
+ pull_request_review.line, pull_request_review.created_at
+ FROM pull_request_review
+ JOIN pull_request_revision
+ ON pull_request_revision.id = pull_request_review.revision_id
+ JOIN account AS author
+ ON author.id = pull_request_review.author_account_id
+ WHERE pull_request_review.pull_request_id = ?1
+ ORDER BY pull_request_review.created_at, pull_request_review.id",
+ )?;
+ statement
+ .query_map([&pull_request.id], pull_request_review_from_row)?
+ .collect::<Result<Vec<_>, _>>()?
+ };
+ let timeline = {
+ let mut statement = self.connection.prepare(
+ "SELECT sequence, kind, actor, payload, created_at
+ FROM repository_event
+ WHERE pull_request_id = ?1 ORDER BY sequence",
+ )?;
+ statement
+ .query_map([&pull_request.id], |row| {
+ Ok(PullRequestTimelineRecord {
+ sequence: row.get(0)?,
+ kind: row.get(1)?,
+ actor: row.get(2)?,
+ payload: row.get(3)?,
+ created_at: row.get(4)?,
+ })
+ })?
+ .collect::<Result<Vec<_>, _>>()?
+ };
let can_revise = access.can_write_repository();
Ok(PullRequestDetail {
+ can_review: access.active_actor_id.is_some() && access.can_read(),
repository: access.repository,
pull_request,
revisions,
+ reviews,
+ timeline,
can_revise,
})
+ }
+
+ pub(crate) fn create_pull_request_review(
+ &mut self,
+ review: &NewPullRequestReview<'_>,
+ ) -> Result<String, StoreError> {
+ let transaction = self
+ .connection
+ .transaction_with_behavior(TransactionBehavior::Immediate)?;
+ let access = repository_issue_access(
+ &transaction,
+ review.owner,
+ review.repository,
+ Some(review.actor),
+ )?;
+ if !access.can_read() {
+ return Err(StoreError::PullRequestHidden);
+ }
+ let actor_id = access
+ .active_actor_id
+ .ok_or(StoreError::PullRequestDenied)?;
+ let pull_request = transaction
+ .query_row(
+ "SELECT id, state FROM pull_request
+ WHERE repository_id = ?1 AND number = ?2",
+ rusqlite::params![access.repository.id, review.number],
+ |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)),
+ )
+ .optional()?
+ .ok_or_else(|| {
+ StoreError::PullRequestNotFound(
+ review.owner.to_owned(),
+ review.repository.to_owned(),
+ review.number,
+ )
+ })?;
+ if pull_request.1 != "open" {
+ return Err(StoreError::PullRequestState);
+ }
+ let revision = transaction
+ .query_row(
+ "SELECT id, base_object_id, head_object_id
+ FROM pull_request_revision
+ WHERE pull_request_id = ?1 AND number = ?2",
+ rusqlite::params![pull_request.0, review.revision],
+ |row| {
+ Ok((
+ row.get::<_, String>(0)?,
+ row.get::<_, String>(1)?,
+ row.get::<_, String>(2)?,
+ ))
+ },
+ )
+ .optional()?
+ .ok_or(StoreError::PullRequestRevisionNotFound)?;
+ if review.kind == "line-comment" {
+ let expected = match review.side {
+ Some("base") => revision.1.as_str(),
+ Some("head") => revision.2.as_str(),
+ _ => return Err(StoreError::PullRequestReviewAnchor),
+ };
+ if review.commit_object_id != Some(expected) {
+ return Err(StoreError::PullRequestReviewAnchor);
+ }
+ }
+ let id: String =
+ transaction.query_row("SELECT lower(hex(randomblob(16)))", [], |row| row.get(0))?;
+ transaction.execute(
+ "INSERT INTO pull_request_review
+ (id, pull_request_id, revision_id, author_account_id, kind, body,
+ commit_object_id, path, side, line, created_at)
+ VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11)",
+ rusqlite::params![
+ id,
+ pull_request.0,
+ revision.0,
+ actor_id,
+ review.kind,
+ review.body,
+ review.commit_object_id,
+ review.path,
+ review.side,
+ review.line,
+ review.created_at,
+ ],
+ )?;
+ transaction.execute(
+ "UPDATE pull_request SET updated_at = ?2 WHERE id = ?1",
+ rusqlite::params![pull_request.0, review.created_at],
+ )?;
+ let kind = match review.kind {
+ "comment" => event::EventKind::PullRequestCommented,
+ "line-comment" => event::EventKind::PullRequestLineCommented,
+ "approved" => event::EventKind::PullRequestApproved,
+ "changes-requested" => event::EventKind::PullRequestChangesRequested,
+ _ => return Err(StoreError::PullRequestReviewAnchor),
+ };
+ let event = event::pull_request_review(
+ kind,
+ &pull_request.0,
+ review.number,
+ &id,
+ review.revision,
+ review.body,
+ review.commit_object_id,
+ review.path,
+ review.side,
+ review.line,
+ );
+ insert_pull_request_event(
+ &transaction,
+ &access.repository.id,
+ &pull_request.0,
+ review.actor,
+ review.created_at,
+ &event,
+ )?;
+ transaction.commit()?;
+ Ok(id)
}
pub(crate) fn pull_requests(
@@ -3043,11 +3207,52 @@
pub(crate) created_at: i64,
}
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(crate) struct PullRequestReviewRecord {
+ pub(crate) id: String,
+ pub(crate) revision: i64,
+ pub(crate) author: String,
+ pub(crate) kind: String,
+ pub(crate) body: String,
+ pub(crate) commit_object_id: Option<String>,
+ pub(crate) path: Option<Vec<u8>>,
+ pub(crate) side: Option<String>,
+ pub(crate) line: Option<i64>,
+ pub(crate) created_at: i64,
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(crate) struct PullRequestTimelineRecord {
+ pub(crate) sequence: i64,
+ pub(crate) kind: String,
+ pub(crate) actor: String,
+ pub(crate) payload: String,
+ pub(crate) created_at: i64,
+}
+
pub(crate) struct PullRequestDetail {
pub(crate) repository: RepositoryRecord,
pub(crate) pull_request: PullRequestRecord,
pub(crate) revisions: Vec<PullRequestRevisionRecord>,
+ pub(crate) reviews: Vec<PullRequestReviewRecord>,
+ pub(crate) timeline: Vec<PullRequestTimelineRecord>,
pub(crate) can_revise: bool,
+ pub(crate) can_review: bool,
+}
+
+pub(crate) struct NewPullRequestReview<'a> {
+ pub(crate) owner: &'a str,
+ pub(crate) repository: &'a str,
+ pub(crate) number: i64,
+ pub(crate) revision: i64,
+ pub(crate) actor: &'a str,
+ pub(crate) kind: &'a str,
+ pub(crate) body: &'a str,
+ pub(crate) commit_object_id: Option<&'a str>,
+ pub(crate) path: Option<&'a [u8]>,
+ pub(crate) side: Option<&'a str>,
+ pub(crate) line: Option<i64>,
+ pub(crate) created_at: i64,
}
pub(crate) struct NewPullRequestRefIntent<'a> {
@@ -3461,6 +3666,23 @@
})
}
+fn pull_request_review_from_row(
+ row: &rusqlite::Row<'_>,
+) -> rusqlite::Result<PullRequestReviewRecord> {
+ Ok(PullRequestReviewRecord {
+ id: row.get(0)?,
+ revision: row.get(1)?,
+ author: row.get(2)?,
+ kind: row.get(3)?,
+ body: row.get(4)?,
+ commit_object_id: row.get(5)?,
+ path: row.get(6)?,
+ side: row.get(7)?,
+ line: row.get(8)?,
+ created_at: row.get(9)?,
+ })
+}
+
fn validate_feed_scope(scope: &str, repository: Option<(&str, &str)>) -> Result<(), StoreError> {
match (scope, repository) {
("repository", Some(_)) | ("watched" | "assignments" | "mentions", None) => Ok(()),
@@ -3759,6 +3981,32 @@
source_ordinal: None,
issue_id: Some(issue_id),
pull_request_id: None,
+ event,
+ actor,
+ ref_name: None,
+ old_target: None,
+ new_target: None,
+ created_at,
+ },
+ )
+}
+
+fn insert_pull_request_event(
+ transaction: &rusqlite::Transaction<'_>,
+ repository_id: &str,
+ pull_request_id: &str,
+ actor: &str,
+ created_at: i64,
+ event: &event::VersionedEvent,
+) -> Result<(), StoreError> {
+ insert_domain_event(
+ transaction,
+ &NewDomainEvent {
+ repository_id,
+ source_intent_id: None,
+ source_ordinal: None,
+ issue_id: None,
+ pull_request_id: Some(pull_request_id),
event,
actor,
ref_name: None,
templates/pull_request.html
Mode 100644 → 100644; object 523008b402f9 → 039e4b5a5ac6
@@ -37,6 +37,39 @@
</ol>
</section>
+ <section aria-labelledby="reviews-heading">
+ <h2 id="reviews-heading">Reviews</h2>
+{% if reviews.is_empty() %}
+ <p>This pull request has no review activity.</p>
+{% else %}
+{% for review in reviews %}
+ <article id="review-{{ review.id }}">
+ <h3>{{ review.author }}: {{ review.kind }} at <time>{{ review.created_at }}</time></h3>
+ <p>Revision {{ review.revision }}
+{% if review.kind == "line-comment" %}
+ · <code>{{ review.commit_object_id }}</code> · <code>{{ review.path }}</code> · {{ review.side }} line {{ review.line }}
+{% if review.outdated %}
+ · <strong>Outdated</strong>
+{% endif %}
+{% endif %}
+ </p>
+{% if review.has_body %}
+ <div class="markdown">{{ review.body_html|safe }}</div>
+{% endif %}
+ </article>
+{% endfor %}
+{% endif %}
+ </section>
+
+ <section aria-labelledby="timeline-heading">
+ <h2 id="timeline-heading">Timeline</h2>
+ <ol>
+{% for event in timeline %}
+ <li value="{{ event.sequence }}">{{ event.actor }}: {{ event.kind }} at <time>{{ event.created_at }}</time></li>
+{% endfor %}
+ </ol>
+ </section>
+
<section aria-labelledby="comparison-heading">
<h2 id="comparison-heading">Comparison for revision {{ selected_revision }}</h2>
<p>Merge base: <code>{{ comparison.merge_base }}</code></p>
@@ -72,10 +105,58 @@
<p>This file has binary content.</p>
{% else %}
<pre>{{ file.hunks }}</pre>
+{% if can_review %}
+ <form method="post" action="/{{ owner }}/{{ repository }}/pulls/{{ pull_request.number }}/reviews">
+ <input type="hidden" name="csrf" value="{{ csrf }}">
+ <input type="hidden" name="revision" value="{{ selected_revision }}">
+ <input type="hidden" name="kind" value="line-comment">
+ <input type="hidden" name="path-hex" value="{{ file.path_hex }}">
+ <div class="field">
+ <label for="review-side-{{ file.path_hex }}">Side</label>
+ <select id="review-side-{{ file.path_hex }}" name="side">
+{% if file.has_base %}
+ <option value="base">Base</option>
+{% endif %}
+{% if file.has_head %}
+ <option value="head">Head</option>
+{% endif %}
+ </select>
+ </div>
+ <div class="field">
+ <label for="review-line-{{ file.path_hex }}">Line</label>
+ <input id="review-line-{{ file.path_hex }}" name="line" type="number" min="1" required>
+ </div>
+ <div class="field">
+ <label for="review-body-{{ file.path_hex }}">Line comment (Markdown)</label>
+ <textarea id="review-body-{{ file.path_hex }}" name="body" maxlength="262144" rows="4" required></textarea>
+ </div>
+ <button type="submit">Add line comment</button>
+ </form>
+{% endif %}
{% endif %}
</article>
{% endfor %}
</section>
+
+{% if can_review %}
+ <section aria-labelledby="review-heading">
+ <h2 id="review-heading">Review this revision</h2>
+ <form method="post" action="/{{ owner }}/{{ repository }}/pulls/{{ pull_request.number }}/reviews">
+ <input type="hidden" name="csrf" value="{{ csrf }}">
+ <input type="hidden" name="revision" value="{{ selected_revision }}">
+ <input type="hidden" name="path-hex" value="">
+ <input type="hidden" name="side" value="">
+ <input type="hidden" name="line" value="">
+ <div class="field">
+ <label for="review-body">Review comment (Markdown)</label>
+ <textarea id="review-body" name="body" maxlength="262144" rows="8"></textarea>
+ </div>
+ <button name="kind" value="comment" type="submit">Add comment</button>
+ <button name="kind" value="approved" type="submit">Approve</button>
+ <button name="kind" value="changes-requested" type="submit">Request changes</button>
+ </form>
+ </section>
+{% endif %}
{% if can_revise %}
<form method="post" action="/{{ owner }}/{{ repository }}/pulls/{{ pull_request.number }}/revisions">
tests/cli.rs
Mode 100644 → 100644; object fd0b76536be0 → 32e32f6467b4
@@ -24,7 +24,8 @@
include_str!("../src/store/migrations/013_watches.sql"),
include_str!("../src/store/migrations/014_feed_tokens.sql"),
include_str!("../src/store/migrations/015_pull_requests.sql"),
- "PRAGMA user_version = 15;\n",
+ include_str!("../src/store/migrations/016_pull_request_reviews.sql"),
+ "PRAGMA user_version = 16;\n",
);
#[test]
tests/public_routes.rs
Mode 100644 → 100644; object feb01e7cf40c → d146e2787ded
@@ -848,6 +848,69 @@
assert_eq!(first_revision.status, 200);
assert!(first_revision.text().contains("Comparison for revision 1"));
assert!(!first_revision.text().contains("pull-request.txt"));
+ for fields in [
+ vec![
+ ("csrf", csrf.as_str()),
+ ("revision", "2"),
+ ("kind", "comment"),
+ ("body", "A **general review**."),
+ ("path-hex", ""),
+ ("side", ""),
+ ("line", ""),
+ ],
+ vec![
+ ("csrf", csrf.as_str()),
+ ("revision", "2"),
+ ("kind", "approved"),
+ ("body", ""),
+ ("path-hex", ""),
+ ("side", ""),
+ ("line", ""),
+ ],
+ vec![
+ ("csrf", csrf.as_str()),
+ ("revision", "2"),
+ ("kind", "line-comment"),
+ ("body", "Review this line."),
+ ("path-hex", "70756c6c2d726571756573742e747874"),
+ ("side", "head"),
+ ("line", "1"),
+ ],
+ ] {
+ let response = request(
+ server.address(),
+ "POST",
+ "/alice/example/pulls/1/reviews",
+ &headers,
+ form(&fields).as_bytes(),
+ );
+ assert_eq!(response.status, 303);
+ }
+ let reviewed = request(server.address(), "GET", "/alice/example/pulls/1", &[], &[]);
+ assert!(reviewed.text().contains("<strong>general review</strong>"));
+ assert!(reviewed.text().contains("pull-request-approved"));
+ assert!(reviewed.text().contains("head line 1"));
+ assert!(!reviewed.text().contains("<strong>Outdated</strong>"));
+
+ fs::write(worktree.join("pull-request.txt"), b"later revision\n")
+ .expect("write a later pull-request revision");
+ commit_all(&worktree, "later pull-request revision");
+ run(Command::new("git")
+ .arg("-C")
+ .arg(&worktree)
+ .args(["push", "-q"])
+ .arg(&bare)
+ .arg("feature"));
+ let revised_again = request(
+ server.address(),
+ "POST",
+ "/alice/example/pulls/1/revisions",
+ &headers,
+ revision.as_bytes(),
+ );
+ assert_eq!(revised_again.status, 303);
+ let outdated = request(server.address(), "GET", "/alice/example/pulls/1", &[], &[]);
+ assert!(outdated.text().contains("<strong>Outdated</strong>"));
let search_page = request(server.address(), "GET", "/search", &[], &[]);
assert_eq!(search_page.status, 200);
assert!(
tests/pull_requests.rs
Mode 100644 → 100644; object 62d23979f9ee → 0a643fa75935
@@ -25,6 +25,7 @@
mod store;
use std::fs;
+use std::os::unix::ffi::OsStringExt;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::sync::Arc;
@@ -386,6 +387,158 @@
}
}
+#[test]
+fn records_review_actions_and_immutable_line_anchors_for_both_hashes() {
+ for (index, object_format) in ["sha1", "sha256"].into_iter().enumerate() {
+ let fixture = Fixture::new(object_format, index + 20);
+ let byte_path = "review-å.txt".as_bytes();
+ fixture.commit_bytes_on("feature", byte_path, b"byte path\n", "add a byte path");
+ let service = PullRequestService::new(&fixture.database, &fixture.repositories);
+ service
+ .open(
+ "alice",
+ "project",
+ "alice",
+ "Review anchors",
+ "Keep each review action.",
+ "refs/heads/main",
+ "refs/heads/feature",
+ )
+ .expect("open a reviewed pull request");
+ service
+ .review(
+ "alice",
+ "project",
+ 1,
+ 1,
+ "bob",
+ "comment",
+ "A **general** comment.",
+ None,
+ None,
+ None,
+ )
+ .expect("add a reader comment");
+ service
+ .review(
+ "alice", "project", 1, 1, "bob", "approved", "", None, None, None,
+ )
+ .expect("approve the revision");
+ service
+ .review(
+ "alice",
+ "project",
+ 1,
+ 1,
+ "alice",
+ "changes-requested",
+ "Change this line.",
+ None,
+ None,
+ None,
+ )
+ .expect("request changes");
+ let line_id = service
+ .review(
+ "alice",
+ "project",
+ 1,
+ 1,
+ "bob",
+ "line-comment",
+ "Use a clearer value.",
+ Some(byte_path),
+ Some("head"),
+ Some(1),
+ )
+ .expect("add a line comment");
+ assert!(matches!(
+ service.review(
+ "alice",
+ "project",
+ 1,
+ 1,
+ "bob",
+ "line-comment",
+ "This line does not exist.",
+ Some(byte_path),
+ Some("head"),
+ Some(2),
+ ),
+ Err(PullRequestError::ReviewAnchor)
+ ));
+
+ fixture.commit_feature("make the line comment outdated");
+ service
+ .revise("alice", "project", 1, "alice")
+ .expect("record a new revision");
+ let detail = service
+ .get("alice", "project", 1, Some("bob"))
+ .expect("read review activity");
+ assert_eq!(detail.revisions.len(), 2);
+ assert_eq!(detail.reviews.len(), 4);
+ let line = detail
+ .reviews
+ .iter()
+ .find(|review| review.id == line_id)
+ .expect("find the line review");
+ assert_eq!(line.revision, 1);
+ assert_eq!(line.path.as_deref(), Some(byte_path));
+ assert_eq!(line.side.as_deref(), Some("head"));
+ assert_eq!(line.line, Some(1));
+ assert_eq!(
+ line.commit_object_id.as_deref(),
+ Some(detail.revisions[0].head_object_id.as_str())
+ );
+ assert_eq!(
+ detail
+ .timeline
+ .iter()
+ .map(|event| event.kind.as_str())
+ .collect::<Vec<_>>(),
+ [
+ "pull-request-created",
+ "pull-request-commented",
+ "pull-request-approved",
+ "pull-request-changes-requested",
+ "pull-request-line-commented",
+ "pull-request-revised",
+ ]
+ );
+
+ let store = Store::open(&fixture.database).expect("open the review policy store");
+ store
+ .connection()
+ .execute(
+ "UPDATE repository SET visibility = 'private' WHERE slug = 'project'",
+ [],
+ )
+ .expect("make the reviewed repository private");
+ store
+ .connection()
+ .execute(
+ "DELETE FROM repository_collaborator WHERE account_id = 2",
+ [],
+ )
+ .expect("remove the reader");
+ assert!(matches!(
+ service.review(
+ "alice",
+ "project",
+ 1,
+ 2,
+ "bob",
+ "comment",
+ "This must stay hidden.",
+ None,
+ None,
+ None,
+ ),
+ Err(PullRequestError::Store(StoreError::PullRequestHidden))
+ ));
+ }
+}
+
struct Fixture {
_directory: TempDir,
database: PathBuf,
@@ -500,6 +653,27 @@
Command::new("git").args(["switch", "-q", branch]),
);
fs::write(self.worktree.join(path), content).expect("write branch content");
+ git_commit(&self.worktree, message);
+ run(
+ &self.worktree,
+ Command::new("git")
+ .args(["push", "-q"])
+ .arg(&self.bare)
+ .arg(branch),
+ );
+ }
+
+ fn commit_bytes_on(&self, branch: &str, path: &[u8], content: &[u8], message: &str) {
+ run(
+ &self.worktree,
+ Command::new("git").args(["switch", "-q", branch]),
+ );
+ fs::write(
+ self.worktree
+ .join(std::ffi::OsString::from_vec(path.to_vec())),
+ content,
+ )
+ .expect("write byte-path content");
git_commit(&self.worktree, message);
run(
&self.worktree,
tests/sqlite.rs
Mode 100644 → 100644; object f93ba12b080d → 9a654b0702c3
@@ -79,6 +79,18 @@
include_str!("../src/store/migrations/014_feed_tokens.sql"),
"PRAGMA user_version = 14;\n",
);
+const V15_FIXTURE: &str = concat!(
+ include_str!("fixtures/sqlite/v7.sql"),
+ include_str!("../src/store/migrations/008_web_sessions.sql"),
+ include_str!("../src/store/migrations/009_repository_authorization.sql"),
+ include_str!("../src/store/migrations/010_audit_history.sql"),
+ include_str!("../src/store/migrations/011_domain_events.sql"),
+ include_str!("../src/store/migrations/012_issues.sql"),
+ include_str!("../src/store/migrations/013_watches.sql"),
+ include_str!("../src/store/migrations/014_feed_tokens.sql"),
+ include_str!("../src/store/migrations/015_pull_requests.sql"),
+ "PRAGMA user_version = 15;\n",
+);
fn database(directory: &TempDir, name: &str) -> std::path::PathBuf {
directory.path().join(name)
@@ -196,7 +208,7 @@
let directory = TempDir::new().expect("create a temporary directory");
let store = Store::open(&database(&directory, "store.sqlite")).expect("open the store");
- assert_eq!(store.schema_version().expect("read the schema version"), 15);
+ assert_eq!(store.schema_version().expect("read the schema version"), 16);
assert_eq!(
store
.connection()
@@ -1559,13 +1571,14 @@
(V12_FIXTURE, 12),
(V13_FIXTURE, 13),
(V14_FIXTURE, 14),
+ (V15_FIXTURE, 15),
] {
let directory = TempDir::new().expect("create a temporary directory");
let path = database(&directory, "tit.sqlite3");
create_fixture(&path, fixture);
let store = Store::open(&path).expect("migrate the fixture");
- assert_eq!(store.schema_version().expect("read the schema version"), 15);
+ assert_eq!(store.schema_version().expect("read the schema version"), 16);
store.integrity_check().expect("check migrated integrity");
let state: String = store
.connection()
@@ -1638,7 +1651,7 @@
#[test]
fn recovers_complete_schema_versions_after_a_process_kill_during_migration() {
- for (mode, expected_version) in [("migration-uncommitted", 1), ("migration-committed", 15)] {
+ for (mode, expected_version) in [("migration-uncommitted", 1), ("migration-committed", 16)] {
let directory = TempDir::new().expect("create a temporary directory");
let path = database(&directory, "fixture.sqlite");
create_fixture(&path, V1_FIXTURE);