radicle: improve reviews
The `Review::summary` was limited in behaviour, since it was only an `Option<String>`. The field is now changed to `NonEmpty<Edit>` so that it can enable an edit history, as well as being able to supply embeds as part of the text. `Reactions` are also introduced to `Review` and, necessarily, introducing an `Action::ReviewReact`. Note that some machinery was added under `actions` and `encoding` to enable backwards-compatibility. For `actions`, a new `ReviewEdit` is introduced to keep track of the different versions of editing actions – supporting the old actions while also introducing the new variant. For `encoding`, the machinery exploits the fact that an `Option<String>` can be converted into a `NonEmpty<Edit>` given that an `ActorId` and `Timestamp` are available, and that `None` can be converted into an empty string. The struct `patch::Review` now deserializes via this `encoding::review::Review` type and, importantly, preserves backwards-compatibility.
This commit is contained in:
parent
c30298fb8a
commit
34939253f7
|
|
@ -802,6 +802,16 @@ version = "1.0.1"
|
|||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5"
|
||||
|
||||
[[package]]
|
||||
name = "erased-serde"
|
||||
version = "0.4.6"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e004d887f51fcb9fef17317a2f3525c887d8aa3f4f50fed920816a688284a5b7"
|
||||
dependencies = [
|
||||
"serde",
|
||||
"typeid",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "errno"
|
||||
version = "0.3.13"
|
||||
|
|
@ -2471,6 +2481,7 @@ dependencies = [
|
|||
"radicle-ssh",
|
||||
"schemars",
|
||||
"serde",
|
||||
"serde-untagged",
|
||||
"serde_json",
|
||||
"siphasher 1.0.1",
|
||||
"sqlite",
|
||||
|
|
@ -3012,6 +3023,17 @@ dependencies = [
|
|||
"serde_derive",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "serde-untagged"
|
||||
version = "0.1.7"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "299d9c19d7d466db4ab10addd5703e4c615dec2a5a16dbbafe191045e87ee66e"
|
||||
dependencies = [
|
||||
"erased-serde",
|
||||
"serde",
|
||||
"typeid",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "serde_derive"
|
||||
version = "1.0.219"
|
||||
|
|
@ -3603,6 +3625,12 @@ dependencies = [
|
|||
"tree-sitter-language",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "typeid"
|
||||
version = "1.0.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c"
|
||||
|
||||
[[package]]
|
||||
name = "typenum"
|
||||
version = "1.17.0"
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ radicle-ssh = { workspace = true }
|
|||
schemars = { workspace = true, optional = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true, features = ["preserve_order"] }
|
||||
serde-untagged = "0.1.7"
|
||||
siphasher = "1.0.0"
|
||||
sqlite = { workspace = true, features = ["bundled"] }
|
||||
tempfile = { workspace = true }
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
pub mod cache;
|
||||
|
||||
mod actions;
|
||||
pub use actions::ReviewEdit;
|
||||
|
||||
mod encoding;
|
||||
|
||||
use std::collections::btree_map;
|
||||
use std::collections::{BTreeMap, BTreeSet, HashMap};
|
||||
use std::fmt;
|
||||
|
|
@ -187,16 +192,6 @@ pub enum Action {
|
|||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
labels: Vec<Label>,
|
||||
},
|
||||
#[serde(rename = "review.edit")]
|
||||
ReviewEdit {
|
||||
review: ReviewId,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
summary: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
verdict: Option<Verdict>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
labels: Vec<Label>,
|
||||
},
|
||||
#[serde(rename = "review.redact")]
|
||||
ReviewRedact { review: ReviewId },
|
||||
#[serde(rename = "review.comment")]
|
||||
|
|
@ -234,6 +229,13 @@ pub enum Action {
|
|||
ReviewCommentResolve { review: ReviewId, comment: EntryId },
|
||||
#[serde(rename = "review.comment.unresolve")]
|
||||
ReviewCommentUnresolve { review: ReviewId, comment: EntryId },
|
||||
/// React to the review.
|
||||
#[serde(rename = "review.react")]
|
||||
ReviewReact {
|
||||
review: ReviewId,
|
||||
reaction: Reaction,
|
||||
active: bool,
|
||||
},
|
||||
|
||||
//
|
||||
// Revision actions
|
||||
|
|
@ -307,6 +309,12 @@ pub enum Action {
|
|||
reaction: Reaction,
|
||||
active: bool,
|
||||
},
|
||||
/// Edit a review's summary, verdict, labels, and embeds.
|
||||
// Note that the tags live on `actions::ReviewEdit`, and according to the
|
||||
// serde.rs docs, it must come after the other variants due to the
|
||||
// `untagged` declaration.
|
||||
#[serde(untagged)]
|
||||
ReviewEdit(actions::ReviewEdit),
|
||||
}
|
||||
|
||||
impl CobAction for Action {
|
||||
|
|
@ -674,7 +682,7 @@ impl Patch {
|
|||
},
|
||||
// Anyone can submit a review.
|
||||
Action::Review { .. } => Authorization::Allow,
|
||||
Action::ReviewRedact { review, .. } | Action::ReviewEdit { review, .. } => {
|
||||
Action::ReviewRedact { review, .. } => {
|
||||
if let Some((_, review)) = lookup::review(self, review)? {
|
||||
Authorization::from(actor == review.author.public_key())
|
||||
} else {
|
||||
|
|
@ -682,6 +690,14 @@ impl Patch {
|
|||
Authorization::Unknown
|
||||
}
|
||||
}
|
||||
Action::ReviewEdit(edit) => {
|
||||
if let Some((_, review)) = lookup::review(self, edit.review_id())? {
|
||||
Authorization::from(actor == review.author.public_key())
|
||||
} else {
|
||||
// Redacted.
|
||||
Authorization::Unknown
|
||||
}
|
||||
}
|
||||
// Anyone can comment on a review.
|
||||
Action::ReviewComment { .. } => Authorization::Allow,
|
||||
// The comment author can edit and redact their own comment.
|
||||
|
|
@ -714,6 +730,7 @@ impl Patch {
|
|||
// Redacted.
|
||||
Authorization::Unknown
|
||||
}
|
||||
Action::ReviewReact { .. } => Authorization::Allow,
|
||||
// Anyone can propose revisions.
|
||||
Action::Revision { .. } => Authorization::Allow,
|
||||
// Only the revision author can edit or redact their revision.
|
||||
|
|
@ -896,7 +913,7 @@ impl Patch {
|
|||
}
|
||||
Action::Review {
|
||||
revision,
|
||||
ref summary,
|
||||
summary,
|
||||
verdict,
|
||||
labels,
|
||||
} => {
|
||||
|
|
@ -914,8 +931,9 @@ impl Patch {
|
|||
id,
|
||||
Author::new(author),
|
||||
verdict,
|
||||
summary.to_owned(),
|
||||
summary.unwrap_or_default(),
|
||||
labels,
|
||||
vec![],
|
||||
timestamp,
|
||||
));
|
||||
// Update reviews index.
|
||||
|
|
@ -928,22 +946,7 @@ impl Patch {
|
|||
}
|
||||
}
|
||||
}
|
||||
Action::ReviewEdit {
|
||||
review,
|
||||
summary,
|
||||
verdict,
|
||||
labels,
|
||||
} => {
|
||||
if summary.is_none() && verdict.is_none() {
|
||||
return Err(Error::EmptyReview);
|
||||
}
|
||||
let Some(review) = lookup::review_mut(self, &review)? else {
|
||||
return Ok(());
|
||||
};
|
||||
review.verdict = verdict;
|
||||
review.summary = summary;
|
||||
review.labels = labels;
|
||||
}
|
||||
Action::ReviewEdit(edit) => edit.run(author, timestamp, self)?,
|
||||
Action::ReviewCommentReact {
|
||||
review,
|
||||
comment,
|
||||
|
|
@ -1042,6 +1045,19 @@ impl Patch {
|
|||
// Set the review locator in the review index to redacted.
|
||||
*locator = None;
|
||||
}
|
||||
Action::ReviewReact {
|
||||
review,
|
||||
reaction,
|
||||
active,
|
||||
} => {
|
||||
if let Some(review) = lookup::review_mut(self, &review)? {
|
||||
if active {
|
||||
review.reactions.insert((author, reaction));
|
||||
} else {
|
||||
review.reactions.remove(&(author, reaction));
|
||||
}
|
||||
}
|
||||
}
|
||||
Action::Merge { revision, commit } => {
|
||||
// If the revision was redacted before the merge, ignore the merge.
|
||||
if lookup::revision_mut(self, &revision)?.is_none() {
|
||||
|
|
@ -1613,6 +1629,7 @@ impl fmt::Display for Verdict {
|
|||
/// A patch review on a revision.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[serde(from = "encoding::review::Review")]
|
||||
pub struct Review {
|
||||
/// Review identifier.
|
||||
pub(super) id: ReviewId,
|
||||
|
|
@ -1624,13 +1641,19 @@ pub struct Review {
|
|||
pub(super) verdict: Option<Verdict>,
|
||||
/// Review summary.
|
||||
///
|
||||
/// Can be edited or set to `None`.
|
||||
pub(super) summary: Option<String>,
|
||||
/// Can be empty, given there is a [`Verdict`].
|
||||
///
|
||||
/// If not empty, then the last [`Edit`] in the `Vec` will be the latest
|
||||
/// edit of the summary.
|
||||
pub(super) summary: NonEmpty<Edit>,
|
||||
/// Review comments.
|
||||
pub(super) comments: Thread<Comment<CodeLocation>>,
|
||||
/// Labels qualifying the review. For example if this review only looks at the
|
||||
/// concept or intention of the patch, it could have a "concept" label.
|
||||
pub(super) labels: Vec<Label>,
|
||||
#[serde(skip_serializing_if = "BTreeSet::is_empty")]
|
||||
/// Reactions to the review.
|
||||
pub(super) reactions: Reactions,
|
||||
/// Review timestamp.
|
||||
pub(super) timestamp: Timestamp,
|
||||
}
|
||||
|
|
@ -1640,16 +1663,19 @@ impl Review {
|
|||
id: ReviewId,
|
||||
author: Author,
|
||||
verdict: Option<Verdict>,
|
||||
summary: Option<String>,
|
||||
summary: String,
|
||||
labels: Vec<Label>,
|
||||
embeds: Vec<Embed<Uri>>,
|
||||
timestamp: Timestamp,
|
||||
) -> Self {
|
||||
let summary = NonEmpty::new(Edit::new(*author.public_key(), summary, timestamp, embeds));
|
||||
Self {
|
||||
id,
|
||||
author,
|
||||
verdict,
|
||||
summary,
|
||||
comments: Thread::default(),
|
||||
reactions: BTreeSet::new(),
|
||||
labels,
|
||||
timestamp,
|
||||
}
|
||||
|
|
@ -1681,8 +1707,23 @@ impl Review {
|
|||
}
|
||||
|
||||
/// Review general comment.
|
||||
pub fn summary(&self) -> Option<&str> {
|
||||
self.summary.as_deref()
|
||||
pub fn summary(&self) -> &str {
|
||||
self.summary.last().body.as_str()
|
||||
}
|
||||
|
||||
/// Review embeds.
|
||||
pub fn embeds(&self) -> &[Embed<Uri>] {
|
||||
&self.summary.last().embeds
|
||||
}
|
||||
|
||||
/// Review reactions.
|
||||
pub fn reactions(&self) -> &Reactions {
|
||||
&self.reactions
|
||||
}
|
||||
|
||||
/// Get the review summary edits.
|
||||
pub fn edits(&self) -> impl Iterator<Item = &Edit> {
|
||||
self.summary.iter()
|
||||
}
|
||||
|
||||
/// Review timestamp.
|
||||
|
|
@ -1911,15 +1952,17 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {
|
|||
&mut self,
|
||||
review: ReviewId,
|
||||
verdict: Option<Verdict>,
|
||||
summary: Option<String>,
|
||||
summary: String,
|
||||
labels: Vec<Label>,
|
||||
embeds: impl IntoIterator<Item = Embed<Uri>>,
|
||||
) -> Result<(), store::Error> {
|
||||
self.push(Action::ReviewEdit {
|
||||
self.push(Action::ReviewEdit(actions::ReviewEdit::new(
|
||||
review,
|
||||
summary,
|
||||
verdict,
|
||||
labels,
|
||||
})
|
||||
embeds.into_iter().collect(),
|
||||
)))
|
||||
}
|
||||
|
||||
/// Redact a patch review.
|
||||
|
|
@ -2272,15 +2315,16 @@ where
|
|||
&mut self,
|
||||
review: ReviewId,
|
||||
verdict: Option<Verdict>,
|
||||
summary: Option<String>,
|
||||
summary: String,
|
||||
labels: Vec<Label>,
|
||||
embeds: impl IntoIterator<Item = Embed<Uri>>,
|
||||
signer: &Device<G>,
|
||||
) -> Result<EntryId, Error>
|
||||
where
|
||||
G: crypto::signature::Signer<crypto::Signature>,
|
||||
{
|
||||
self.transaction("Edit review", signer, |tx| {
|
||||
tx.review_edit(review, verdict, summary, labels)
|
||||
tx.review_edit(review, verdict, summary, labels, embeds)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -3109,7 +3153,7 @@ mod test {
|
|||
|
||||
let review = revision.review_by(alice.signer.public_key()).unwrap();
|
||||
assert_eq!(review.verdict(), Some(Verdict::Accept));
|
||||
assert_eq!(review.summary(), Some("LGTM"));
|
||||
assert_eq!(review.summary(), "LGTM");
|
||||
|
||||
patch.redact_review(review_id, &alice.signer).unwrap();
|
||||
patch.reload().unwrap();
|
||||
|
|
@ -3345,7 +3389,8 @@ mod test {
|
|||
.review_edit(
|
||||
review,
|
||||
Some(Verdict::Reject),
|
||||
Some("Whoops!".to_owned()),
|
||||
"Whoops!".to_owned(),
|
||||
vec![],
|
||||
vec![],
|
||||
&alice.signer,
|
||||
)
|
||||
|
|
@ -3354,7 +3399,7 @@ mod test {
|
|||
let (_, revision) = patch.latest();
|
||||
let review = revision.review_by(alice.signer.public_key()).unwrap();
|
||||
assert_eq!(review.verdict(), Some(Verdict::Reject));
|
||||
assert_eq!(review.summary(), Some("Whoops!"));
|
||||
assert_eq!(review.summary(), "Whoops!");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -3415,7 +3460,14 @@ mod test {
|
|||
.unwrap();
|
||||
|
||||
let _review = patch
|
||||
.review_edit(review, Some(Verdict::Reject), None, vec![], &alice.signer)
|
||||
.review_edit(
|
||||
review,
|
||||
Some(Verdict::Reject),
|
||||
"".to_string(),
|
||||
vec![],
|
||||
vec![],
|
||||
&alice.signer,
|
||||
)
|
||||
.unwrap();
|
||||
patch
|
||||
.review_comment(review, "Second comment!", None, None, [], &alice.signer)
|
||||
|
|
@ -3508,7 +3560,14 @@ mod test {
|
|||
)
|
||||
.unwrap();
|
||||
patch
|
||||
.review_edit(review, Some(Verdict::Accept), None, vec![], &alice.signer)
|
||||
.review_edit(
|
||||
review,
|
||||
Some(Verdict::Accept),
|
||||
"".to_string(),
|
||||
vec![],
|
||||
vec![],
|
||||
&alice.signer,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let id = patch.id;
|
||||
|
|
@ -3517,7 +3576,7 @@ mod test {
|
|||
let review = revision.review_by(alice.signer.public_key()).unwrap();
|
||||
|
||||
assert_eq!(review.verdict(), Some(Verdict::Accept));
|
||||
assert_eq!(review.summary(), None);
|
||||
assert_eq!(review.summary(), "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,180 @@
|
|||
//! Keep track of the patch [`Action`] versions, to ensure compatibility where
|
||||
//! possible.
|
||||
//!
|
||||
//! [`Action`]: super::Action
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::cob::{thread::Edit, ActorId, Embed, Label, Timestamp, Uri};
|
||||
|
||||
use super::{lookup, Error, Patch, ReviewId, Verdict};
|
||||
|
||||
/// A review edit that keeps track of the different versions of actions.
|
||||
///
|
||||
/// [`ReviewEdit::new`] will create the latest version of the action.
|
||||
///
|
||||
/// [`ReviewEdit::run`] will apply the action to the given [`Patch`].
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
|
||||
#[serde(tag = "type", rename_all = "camelCase")]
|
||||
pub enum ReviewEdit {
|
||||
/// The initial version of editing a review.
|
||||
///
|
||||
/// This allowed editing the `summary`, `verdict`, and `labels` of a
|
||||
/// [`Patch`], where the `summary` value was optional.
|
||||
#[serde(rename = "review.edit")]
|
||||
V1(ReviewEditV1),
|
||||
/// The latest version of editing a review.
|
||||
///
|
||||
/// This allows editing the `summary`, `verdict`, `labels` of [`Patch`], and
|
||||
/// introduces `embeds` to the review [`summary`].
|
||||
///
|
||||
/// The `summary` of a [`Review`] is now an edit-history.
|
||||
#[serde(rename = "review.edit.v2")]
|
||||
V2(ReviewEditV2),
|
||||
}
|
||||
|
||||
impl ReviewEdit {
|
||||
/// Create the latest version of [`ReviewEdit`].
|
||||
pub fn new(
|
||||
review: ReviewId,
|
||||
summary: String,
|
||||
verdict: Option<Verdict>,
|
||||
labels: Vec<Label>,
|
||||
embeds: Vec<Embed<Uri>>,
|
||||
) -> Self {
|
||||
Self::V2(ReviewEditV2 {
|
||||
review,
|
||||
summary,
|
||||
verdict,
|
||||
labels,
|
||||
embeds,
|
||||
})
|
||||
}
|
||||
|
||||
/// Get the [`ReviewId`] that this edit is applying to.
|
||||
pub fn review_id(&self) -> &ReviewId {
|
||||
match self {
|
||||
ReviewEdit::V1(ReviewEditV1 { review, .. }) => review,
|
||||
ReviewEdit::V2(ReviewEditV2 { review, .. }) => review,
|
||||
}
|
||||
}
|
||||
|
||||
/// Apply the action to the given [`Patch`].
|
||||
pub fn run(
|
||||
self,
|
||||
author: ActorId,
|
||||
timestamp: Timestamp,
|
||||
patch: &mut Patch,
|
||||
) -> Result<(), Error> {
|
||||
match self {
|
||||
ReviewEdit::V1(ReviewEditV1 {
|
||||
review,
|
||||
summary,
|
||||
verdict,
|
||||
labels,
|
||||
}) => {
|
||||
if summary.is_none() && verdict.is_none() {
|
||||
return Err(Error::EmptyReview);
|
||||
}
|
||||
let Some(review) = lookup::review_mut(patch, &review)? else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
if let Some(body) = summary {
|
||||
review
|
||||
.summary
|
||||
.push(Edit::new(author, body, timestamp, vec![]));
|
||||
}
|
||||
review.verdict = verdict;
|
||||
review.labels = labels;
|
||||
Ok(())
|
||||
}
|
||||
ReviewEdit::V2(ReviewEditV2 {
|
||||
review,
|
||||
summary,
|
||||
verdict,
|
||||
labels,
|
||||
embeds,
|
||||
}) => {
|
||||
if summary.is_empty() && verdict.is_none() {
|
||||
return Err(Error::EmptyReview);
|
||||
}
|
||||
let Some(review) = lookup::review_mut(patch, &review)? else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
review
|
||||
.summary
|
||||
.push(Edit::new(author, summary, timestamp, embeds));
|
||||
review.verdict = verdict;
|
||||
review.labels = labels;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ReviewEditV2 {
|
||||
review: ReviewId,
|
||||
#[serde(default, skip_serializing_if = "String::is_empty")]
|
||||
summary: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
verdict: Option<Verdict>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
labels: Vec<Label>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
embeds: Vec<Embed<Uri>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ReviewEditV1 {
|
||||
review: ReviewId,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
summary: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
verdict: Option<Verdict>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
labels: Vec<Label>,
|
||||
}
|
||||
|
||||
#[allow(clippy::unwrap_used)]
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use serde_json::json;
|
||||
|
||||
use crate::patch;
|
||||
|
||||
use super::ReviewEdit;
|
||||
|
||||
#[test]
|
||||
fn test_review_edit() {
|
||||
let v1 = json!({
|
||||
"type": "review.edit",
|
||||
"review": "89d45fb371eb2622ba88188d474347cc526d80bb",
|
||||
"summary": "lgtm",
|
||||
"verdict": "accept",
|
||||
"labels": [],
|
||||
});
|
||||
let v2 = json!({
|
||||
"type": "review.edit.v2",
|
||||
"review": "89d45fb371eb2622ba88188d474347cc526d80bb",
|
||||
"summary": "lgtm",
|
||||
"verdict": "accept",
|
||||
"labels": [],
|
||||
"embeds": [],
|
||||
});
|
||||
serde_json::from_value::<ReviewEdit>(v1.clone()).unwrap();
|
||||
serde_json::from_value::<ReviewEdit>(v2.clone()).unwrap();
|
||||
assert!(matches!(
|
||||
serde_json::from_value::<patch::Action>(v1).unwrap(),
|
||||
patch::Action::ReviewEdit { .. }
|
||||
));
|
||||
assert!(matches!(
|
||||
serde_json::from_value::<patch::Action>(v2).unwrap(),
|
||||
patch::Action::ReviewEdit { .. }
|
||||
));
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
//! Home for separating serialization and deserialization of [`Patch`] data from
|
||||
//! the final [`Patch`] state.
|
||||
//!
|
||||
//! [`Patch`]: super::Patch
|
||||
|
||||
pub mod review;
|
||||
|
|
@ -0,0 +1,254 @@
|
|||
//! Representation of a [`patch::Review`] for deserializing.
|
||||
//!
|
||||
//! The [`Review`] type contains fields that are:
|
||||
//!
|
||||
//! - Shared within previous versions, e.g. V1 fields that are shared with V2
|
||||
//! - Introduced at a later version, but can use a [`Default`] instance
|
||||
//! - Can be migrated from one version to another
|
||||
//!
|
||||
//! [`patch::Review`]: crate::cob::patch::Review
|
||||
|
||||
use nonempty::NonEmpty;
|
||||
use serde::Deserialize;
|
||||
use serde_untagged::UntaggedEnumVisitor;
|
||||
|
||||
use crate::cob::patch;
|
||||
use crate::cob::patch::{
|
||||
Author, CodeLocation, Comment, Edit, Label, Reactions, ReviewId, Thread, Timestamp, Verdict,
|
||||
};
|
||||
|
||||
/// The encoding for a `patch::Review` that can be deserialized and migrated.
|
||||
///
|
||||
/// To maintain backwards-compatibility, [`Review`] must implement:
|
||||
/// ```rust, no_run
|
||||
/// From<Review> for patch::Review
|
||||
/// ```
|
||||
#[derive(Deserialize)]
|
||||
pub(in crate::cob::patch) struct Review {
|
||||
// V1 fields
|
||||
id: ReviewId,
|
||||
author: Author,
|
||||
verdict: Option<Verdict>,
|
||||
comments: Thread<Comment<CodeLocation>>,
|
||||
labels: Vec<Label>,
|
||||
timestamp: Timestamp,
|
||||
|
||||
// V2 fields
|
||||
#[serde(default)]
|
||||
reactions: Reactions,
|
||||
|
||||
// V1 -> V2 conversion
|
||||
#[serde(default)]
|
||||
summary: Summary,
|
||||
}
|
||||
|
||||
/// The [`Summary`] type represents the different versions of the `summary`
|
||||
/// field of a [`Review`].
|
||||
///
|
||||
/// The `V1` variant holds an `Option<String>`, which can be converted into an
|
||||
/// `Edit` given the `ActorId` and `Timestamp` – supplying an empty `Vec` of
|
||||
/// `Embed`s.
|
||||
///
|
||||
/// The `V2` variant holds the current type `NonEmpty<Edit>`.
|
||||
///
|
||||
/// Using [`Summary::into_edits`], we can get the latest representation of a
|
||||
/// [`patch::Review::summary`].
|
||||
///
|
||||
/// [`patch::Review::summary`]: crate::cob::patch::Review::summary
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(in crate::cob::patch) enum Summary {
|
||||
V2(NonEmpty<Edit>),
|
||||
V1(Option<String>),
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for Summary {
|
||||
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
UntaggedEnumVisitor::new()
|
||||
.unit(|| Ok(Self::V1(None)))
|
||||
.string(|body| Ok(Self::V1(Some(body.to_owned()))))
|
||||
.seq(|edits| edits.deserialize().map(Self::V2))
|
||||
.deserialize(deserializer)
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for Summary {
|
||||
fn default() -> Self {
|
||||
Self::V1(None)
|
||||
}
|
||||
}
|
||||
|
||||
impl Summary {
|
||||
fn into_edits(self, author: &Author, timestamp: &Timestamp) -> NonEmpty<Edit> {
|
||||
match self {
|
||||
Summary::V1(summary) => NonEmpty::new(Edit::new(
|
||||
*author.public_key(),
|
||||
summary.unwrap_or_default(),
|
||||
*timestamp,
|
||||
vec![],
|
||||
)),
|
||||
Summary::V2(edits) => edits,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<Review> for patch::Review {
|
||||
fn from(review: Review) -> Self {
|
||||
let Review {
|
||||
id,
|
||||
author,
|
||||
verdict,
|
||||
comments,
|
||||
labels,
|
||||
timestamp,
|
||||
reactions,
|
||||
summary,
|
||||
} = review;
|
||||
let summary = summary.into_edits(&author, ×tamp);
|
||||
Self {
|
||||
id,
|
||||
author,
|
||||
verdict,
|
||||
summary,
|
||||
comments,
|
||||
labels,
|
||||
reactions,
|
||||
timestamp,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::unwrap_used)]
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use nonempty::nonempty;
|
||||
use serde_json::json;
|
||||
|
||||
use crate::{
|
||||
cob::{thread::Edit, Timestamp},
|
||||
patch,
|
||||
};
|
||||
|
||||
use super::{Review, Summary};
|
||||
|
||||
#[test]
|
||||
fn test_review_summary() {
|
||||
let summary_null = json!(null);
|
||||
let summary_string = json!("lgtm");
|
||||
let summary_edits = json!([{
|
||||
"author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
|
||||
"timestamp": 1710947885000_i64,
|
||||
"body": "lgtm",
|
||||
"embeds": [],
|
||||
}]);
|
||||
assert_eq!(
|
||||
serde_json::from_value::<Summary>(summary_null).unwrap(),
|
||||
Summary::V1(None)
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::from_value::<Summary>(summary_string).unwrap(),
|
||||
Summary::V1(Some("lgtm".to_string()))
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::from_value::<Summary>(summary_edits).unwrap(),
|
||||
Summary::V2(nonempty![Edit::new(
|
||||
"z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx"
|
||||
.parse()
|
||||
.unwrap(),
|
||||
"lgtm".to_string(),
|
||||
Timestamp::from_secs(1710947885),
|
||||
vec![]
|
||||
)])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_review_deserialize_summary_migration_null_summary() {
|
||||
let review = json!({
|
||||
"id": "89d45fb371eb2622ba88188d474347cc526d80bb",
|
||||
"author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
|
||||
"verdict": "accept",
|
||||
"summary": null,
|
||||
"comments": {
|
||||
"comments": {},
|
||||
"timeline": []
|
||||
},
|
||||
"labels": [],
|
||||
"timestamp": 1710947885000_i64
|
||||
});
|
||||
let v1 = serde_json::from_value::<Review>(review.clone()).unwrap();
|
||||
assert_eq!(
|
||||
serde_json::from_value::<patch::Review>(review).unwrap(),
|
||||
v1.into()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_review_deserialize_summary_migration_without_summary() {
|
||||
let review = json!({
|
||||
"id": "89d45fb371eb2622ba88188d474347cc526d80bb",
|
||||
"author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
|
||||
"verdict": "accept",
|
||||
"comments": {
|
||||
"comments": {},
|
||||
"timeline": []
|
||||
},
|
||||
"labels": [],
|
||||
"timestamp": 1710947885000_i64
|
||||
});
|
||||
let v1 = serde_json::from_value::<Review>(review.clone()).unwrap();
|
||||
assert_eq!(
|
||||
serde_json::from_value::<patch::Review>(review).unwrap(),
|
||||
v1.into()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_review_deserialize_summary_migration_with_summary() {
|
||||
let review = json!({
|
||||
"id": "89d45fb371eb2622ba88188d474347cc526d80bb",
|
||||
"author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
|
||||
"verdict": "accept",
|
||||
"summary": "lgtm",
|
||||
"comments": {
|
||||
"comments": {},
|
||||
"timeline": []
|
||||
},
|
||||
"labels": [],
|
||||
"timestamp": 1710947885000_i64
|
||||
});
|
||||
let v1 = serde_json::from_value::<Review>(review.clone()).unwrap();
|
||||
assert_eq!(
|
||||
serde_json::from_value::<patch::Review>(review).unwrap(),
|
||||
v1.into()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_review_deserialize_summary_v2() {
|
||||
let review = json!({
|
||||
"id": "89d45fb371eb2622ba88188d474347cc526d80bb",
|
||||
"author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
|
||||
"verdict": "accept",
|
||||
"summary": [{
|
||||
"author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
|
||||
"timestamp": 1710947885000_i64,
|
||||
"body": "lgtm",
|
||||
"embeds": [],
|
||||
}],
|
||||
"comments": {
|
||||
"comments": {},
|
||||
"timeline": []
|
||||
},
|
||||
"labels": [],
|
||||
"timestamp": 1710947885000_i64
|
||||
});
|
||||
let v2 = serde_json::from_value::<Review>(review.clone()).unwrap();
|
||||
assert_eq!(
|
||||
serde_json::from_value::<patch::Review>(review).unwrap(),
|
||||
v2.into()
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue