diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 6636e42c..3645fa38 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -12,7 +12,7 @@ use octorust::{ use partial_struct::partial; use rfd_data::{ content::{RfdContent, RfdDocument, RfdTemplate, TemplateError}, - RfdNumber, + RfdNumber, RfdState, SingleLineString, }; use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ @@ -81,6 +81,14 @@ pub struct ContentContext { pub new_template: RfdTemplate, } +/// A single attribute change to apply to the header of a RFD document +#[derive(Debug)] +pub enum RfdAttrUpdate { + Discussion(SingleLineString), + Labels(SingleLineString), + State(RfdState), +} + #[derive(Debug, Error)] pub enum UpdateRfdContentError { #[error(transparent)] @@ -691,6 +699,50 @@ impl RfdContext { } } + #[allow(clippy::result_large_err)] + #[instrument(skip(self, caller))] + pub async fn update_rfd_attr( + &self, + caller: &Caller, + rfd_number: i32, + update: RfdAttrUpdate, + message: Option<&str>, + ) -> ResourceResult, UpdateRfdContentError> { + if caller.any( + [ + RfdPermission::UpdateRfd(rfd_number), + RfdPermission::UpdateRfdsAll, + ] + .iter(), + ) { + let latest_revision = self + .get_latest_rfd_revision(caller, rfd_number) + .await + .inner_err_into()?; + + let sha = latest_revision.commit.clone(); + let mut content: RfdContent = latest_revision.into(); + + match update { + RfdAttrUpdate::Discussion(value) => content.update_discussion(&value), + RfdAttrUpdate::Labels(value) => content.update_labels(&value), + RfdAttrUpdate::State(state) => content.update_state(&state.into()), + }; + + tracing::info!("Updated attribute in RFD document"); + + // Persist the data back to GitHub. Note that we do not store this back to the database. + // We rely on GitHub as the source of truth and revisions are required to be linked to + // commits + self.commit_rfd_document(caller, rfd_number.into(), content.raw(), message, sha, None) + .await?; + + Ok(content) + } else { + resource_restricted() + } + } + #[allow(clippy::result_large_err)] #[instrument(skip(self, caller, document))] pub async fn update_rfd_document( diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 7072c611..a9e4aac4 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -25,8 +25,8 @@ use v_model::{permissions::Caller, storage::ListPagination}; use crate::{ caller::CallerExt, context::{ - RfdContext, RfdRevisionIdentifier, RfdRevisionMeta, RfdRevisionMetadataChange, RfdWithPdf, - RfdWithRaw, RfdWithoutContent, + RfdAttrUpdate, RfdContext, RfdRevisionIdentifier, RfdRevisionMeta, + RfdRevisionMetadataChange, RfdWithPdf, RfdWithRaw, RfdWithoutContent, }, endpoints::UNLIMITED, permissions::RfdPermission, @@ -749,41 +749,20 @@ async fn set_rfd_attr_op( body: &RfdAttrValue, ) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { - // Get the latest revision - let revision = ctx.view_rfd_revision(caller, rfd_number, None).await?; - - // TODO: Get rid of these clones - let mut content = match revision.content_format { - ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(revision.content)), - ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(revision.content)), - }; - - // Update the requested attribute - match &attr { - RfdAttrName::Discussion => content.update_discussion(&body.value), - RfdAttrName::Labels => content.update_labels(&body.value), + let update = match &attr { + RfdAttrName::Discussion => RfdAttrUpdate::Discussion(body.value.clone()), + RfdAttrName::Labels => RfdAttrUpdate::Labels(body.value.clone()), RfdAttrName::State => { - let state: RfdState = body.value.as_str().try_into().map_err(|err| { + RfdAttrUpdate::State(body.value.as_str().try_into().map_err(|err| { tracing::info!(?err, "Invalid state was supplied"); HttpError::for_bad_request(None, "Invalid RFD state".to_string()) - })?; - content.update_state(&state.into()) + })?) } }; - tracing::info!("Updated attribute in RFD document"); - - // Persist the data back to GitHub. Note that we do not store this back to the database. - // We rely on GitHub as the source of truth and revisions are required to tbe linked to - // commits - ctx.update_rfd_document( - caller, - rfd_number, - content.raw(), - body.message.as_deref(), - None, - ) - .await?; + let content = ctx + .update_rfd_attr(caller, rfd_number, update, body.message.as_deref()) + .await?; extract_attr(&attr, &content).map(HttpResponseAccepted) } else { diff --git a/rfd-data/src/content/asciidoc.rs b/rfd-data/src/content/asciidoc.rs index 65cb6310..f81f3640 100644 --- a/rfd-data/src/content/asciidoc.rs +++ b/rfd-data/src/content/asciidoc.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use regex::{regex, Regex}; +use regex::{regex, NoExpand, Regex}; use std::borrow::Cow; use super::RfdDocument; @@ -28,20 +28,21 @@ impl<'a> RfdAsciidoc<'a> { } /// Resolve attribute references within the document. Currently only `{authors}` is supported. + /// Only the first reference is expanded, which bounds growth to the size of one value. fn resolve_references(content: &str) -> String { let pattern = regex!(r"(?[^\\]?)(\{authors\})"); if let Some(captures) = pattern.captures(content) { let prefix = captures.get(1).map(|c| c.as_str()).unwrap_or_default(); + let replacement = format!( + "{}{}", + prefix, + Self::attr("authors", content).unwrap_or_default() + ); + + // NoExpand: don't treat `$` as regex capture reference ($1, etc) pattern - .replace_all( - content, - format!( - "{}{}", - prefix, - Self::attr("authors", content).unwrap_or_default() - ), - ) + .replacen(content, 1, NoExpand(&replacement)) .to_string() } else { content.to_string() @@ -1052,4 +1053,11 @@ include::sub_doc2.adoc[]"#; include.perform_replacement(original, replacement_content) ); } + + #[test] + fn test_reference_value_inserted_literally() { + let content = ":authors: Cost $1 ${b} $$\n\n= RFD 123 Place\n\nBy {authors}\n"; + let rfd = RfdAsciidoc::new(content); + assert!(rfd.resolved.contains("By Cost $1 ${b} $$\n")); + } }