Show pending edits in history (fixes #109)

2025-01-26 07:15:47 +00:00 · 2025-01-17 10:58:37 +01:00 · 2025-01-17 10:58:37 +01:00 · 0edc863bb7
commit 0edc863bb7
parent b573c92a19
15 changed files with 93 additions and 47 deletions
--- a/migrations/2025-01-17-090151_edit_pending/down.sql
+++ b/migrations/2025-01-17-090151_edit_pending/down.sql
@ -0,0 +1 @@
+alter table edit drop column pending;
--- a/migrations/2025-01-17-090151_edit_pending/up.sql
+++ b/migrations/2025-01-17-090151_edit_pending/up.sql
@ -0,0 +1 @@
+alter table edit add column pending bool not null default false;
--- a/src/backend/api/article.rs
+++ b/src/backend/api/article.rs
@ -237,6 +237,7 @@ pub(in crate::backend::api) async fn fork_article(
            hash: e.hash,
            previous_version_id: e.previous_version_id,
            published: Utc::now(),
+            pending: false,
        };
        DbEdit::create(&form, &data)?;
    }
--- a/src/backend/api/mod.rs
+++ b/src/backend/api/mod.rs
@ -92,6 +92,7 @@ pub(in crate::backend::api) async fn site_view(
 #[debug_handler]
 pub async fn edit_list(
    Query(query): Query<GetEditList>,
+    user: Option<Extension<LocalUserView>>,
    data: Data<IbisData>,
 ) -> MyResult<Json<Vec<EditView>>> {
    let params = if let Some(article_id) = query.article_id {
@ -101,7 +102,7 @@ pub async fn edit_list(
    } else {
        return Err(anyhow!("Must provide article_id or person_id").into());
    };
-    Ok(Json(DbEdit::view(params, &data)?))
+    Ok(Json(DbEdit::view(params, &user.map(|u| u.0), &data)?))
 }

 /// Trims the string param, and converts to None if it is empty
--- a/src/backend/database/conflict.rs
+++ b/src/backend/database/conflict.rs
@ -1,6 +1,9 @@
 use crate::{
    backend::{
-        database::{schema::conflict, IbisData},
+        database::{
+            schema::{conflict, edit},
+            IbisData,
+        },
        federation::activities::submit_article_update,
        utils::{error::MyResult, generate_article_version},
    },
@ -69,14 +72,21 @@ impl DbConflict {
    }

    /// Delete merge conflict which was created by specific user
-    pub fn delete(id: ConflictId, creator_id: PersonId, data: &IbisData) -> MyResult<Self> {
+    pub fn delete(id: ConflictId, creator_id: PersonId, data: &IbisData) -> MyResult<()> {
        let mut conn = data.db_pool.get()?;
-        Ok(delete(
+        let conflict: Self = delete(
            conflict::table
                .filter(conflict::dsl::creator_id.eq(creator_id))
                .find(id),
        )
-        .get_result(conn.deref_mut())?)
+        .get_result(conn.deref_mut())?;
+        delete(
+            edit::table
+                .filter(edit::dsl::creator_id.eq(creator_id))
+                .filter(edit::dsl::hash.eq(conflict.hash)),
+        )
+        .execute(conn.deref_mut())?;
+        Ok(())
    }

    pub async fn to_api_conflict(&self, data: &Data<IbisData>) -> MyResult<Option<ApiConflict>> {
--- a/src/backend/database/edit.rs
+++ b/src/backend/database/edit.rs
@ -7,11 +7,21 @@ use crate::{
    common::{
        article::{DbArticle, DbEdit, EditVersion, EditView},
        newtypes::{ArticleId, PersonId},
+        user::LocalUserView,
    },
 };
 use activitypub_federation::fetch::object_id::ObjectId;
 use chrono::{DateTime, Utc};
-use diesel::{insert_into, AsChangeset, ExpressionMethods, Insertable, QueryDsl, RunQueryDsl};
+use diesel::{
+    dsl::not,
+    insert_into,
+    AsChangeset,
+    BoolExpressionMethods,
+    ExpressionMethods,
+    Insertable,
+    QueryDsl,
+    RunQueryDsl,
+};
 use diffy::create_patch;
 use std::ops::DerefMut;

@ -26,6 +36,7 @@ pub struct DbEditForm {
    pub article_id: ArticleId,
    pub previous_version_id: EditVersion,
    pub published: DateTime<Utc>,
+    pub pending: bool,
 }

 impl DbEditForm {
@ -35,6 +46,7 @@ impl DbEditForm {
        updated_text: &str,
        summary: String,
        previous_version_id: EditVersion,
+        pending: bool,
    ) -> MyResult<Self> {
        let diff = create_patch(&original_article.text, updated_text);
        let version = EditVersion::new(&diff.to_string());
@ -48,6 +60,7 @@ impl DbEditForm {
            previous_version_id,
            summary,
            published: Utc::now(),
+            pending,
        })
    }

@ -96,11 +109,18 @@ impl DbEdit {
            .get_results(conn.deref_mut())?)
    }

-    pub fn view(params: ViewEditParams, data: &IbisData) -> MyResult<Vec<EditView>> {
+    pub fn view(
+        params: ViewEditParams,
+        user: &Option<LocalUserView>,
+        data: &IbisData,
+    ) -> MyResult<Vec<EditView>> {
        let mut conn = data.db_pool.get()?;
+        let person_id = user.as_ref().map(|u| u.person.id).unwrap_or(PersonId(-1));
        let query = edit::table
            .inner_join(article::table)
            .inner_join(person::table)
+            // only the creator can view pending edits
+            .filter(not(edit::pending).or(edit::creator_id.eq(person_id)))
            .into_boxed();

        let query = match params {
--- a/src/backend/database/schema.rs
+++ b/src/backend/database/schema.rs
@ -40,6 +40,7 @@ diesel::table! {
        article_id -> Int4,
        previous_version_id -> Uuid,
        published -> Timestamptz,
+        pending -> Bool,
    }
 }

--- a/src/backend/federation/activities/mod.rs
+++ b/src/backend/federation/activities/mod.rs
@ -10,11 +10,10 @@ use crate::{
    common::{
        article::{DbArticle, DbEdit, EditVersion},
        instance::DbInstance,
-        newtypes::{EditId, PersonId},
+        newtypes::PersonId,
    },
 };
 use activitypub_federation::config::Data;
-use chrono::Utc;

 pub mod accept;
 pub mod create_article;
@ -31,12 +30,13 @@ pub async fn submit_article_update(
    creator_id: PersonId,
    data: &Data<IbisData>,
 ) -> Result<(), Error> {
-    let form = DbEditForm::new(
+    let mut form = DbEditForm::new(
        original_article,
        creator_id,
        &new_text,
        summary,
        previous_version,
+        false,
    )?;
    if original_article.local {
        let edit = DbEdit::create(&form, data)?;
@ -44,18 +44,9 @@ pub async fn submit_article_update(

        UpdateLocalArticle::send(updated_article, vec![], data).await?;
    } else {
-        // dont insert edit into db, might be invalid in case of conflict
-        let edit = DbEdit {
-            id: EditId(-1),
-            creator_id,
-            hash: form.hash,
-            ap_id: form.ap_id,
-            diff: form.diff,
-            summary: form.summary,
-            article_id: form.article_id,
-            previous_version_id: form.previous_version_id,
-            published: Utc::now(),
-        };
+        // insert edit as pending, so only the creator can see it
+        form.pending = true;
+        let edit = DbEdit::create(&form, data)?;
        let instance = DbInstance::read(original_article.instance_id, data)?;
        UpdateRemoteArticle::send(edit, instance, data).await?;
    }
--- a/src/backend/federation/objects/edit.rs
+++ b/src/backend/federation/objects/edit.rs
@ -98,6 +98,7 @@ impl Object for DbEdit {
            hash: json.version,
            previous_version_id: json.previous_version,
            published: json.published,
+            pending: false,
        };
        let edit = DbEdit::create(&form, data)?;
        Ok(edit)
--- a/src/backend/utils/mod.rs
+++ b/src/backend/utils/mod.rs
@ -57,6 +57,18 @@ pub(super) fn generate_article_version(
    Err(anyhow!("failed to generate article version").into())
 }

+/// Use a single static keypair during testing which is signficantly faster than
+/// generating dozens of keys from scratch.
+pub fn generate_keypair() -> MyResult<Keypair> {
+    if cfg!(debug_assertions) {
+        static KEYPAIR: LazyLock<Keypair> =
+            LazyLock::new(|| generate_actor_keypair().expect("generate keypair"));
+        Ok(KEYPAIR.clone())
+    } else {
+        Ok(generate_actor_keypair()?)
+    }
+}
+
 #[cfg(test)]
 mod test {
    use super::*;
@ -81,6 +93,7 @@ mod test {
                article_id: ArticleId(0),
                previous_version_id: Default::default(),
                published: Utc::now(),
+                pending: false,
            })
        };
        Ok([
@ -115,15 +128,3 @@ mod test {
        Ok(())
    }
 }
-
-/// Use a single static keypair during testing which is signficantly faster than
-/// generating dozens of keys from scratch.
-pub fn generate_keypair() -> MyResult<Keypair> {
-    if cfg!(debug_assertions) {
-        static KEYPAIR: LazyLock<Keypair> =
-            LazyLock::new(|| generate_actor_keypair().expect("generate keypair"));
-        Ok(KEYPAIR.clone())
-    } else {
-        Ok(generate_actor_keypair()?)
-    }
-}
--- a/src/common/article.rs
+++ b/src/common/article.rs
@ -123,6 +123,7 @@ pub struct DbEdit {
    /// First edit of an article always has `EditVersion::default()` here
    pub previous_version_id: EditVersion,
    pub published: DateTime<Utc>,
+    pub pending: bool,
 }

 #[derive(Deserialize, Serialize, Clone, Debug, Default)]
--- a/src/frontend/components/edit_list.rs
+++ b/src/frontend/components/edit_list.rs
@ -42,9 +42,16 @@ pub fn EditList(edits: Vec<EditView>, for_article: bool) -> impl IntoView {
                        view! {
                            <li class="m-2 card card-compact bg-base-100 card-bordered rounded-s">
                                <div class="card-body">
-                                    <a class="w-full text-lg link link-primary" href=path>
-                                        {edit.edit.summary}
-                                    </a>
+                                    <div class="flex w-full">
+                                        <a class="text-lg grow link link-primary" href=path>
+                                            {edit.edit.summary}
+                                        </a>
+                                        <Show when=move || edit.edit.pending>
+                                            <span class="p-1 w-min rounded border-2 border-rose-300">
+                                                Pending
+                                            </span>
+                                        </Show>
+                                    </div>
                                    <p>{second_line}</p>
                                </div>
                            </li>
--- a/src/frontend/pages/diff.rs
+++ b/src/frontend/pages/diff.rs
@ -27,8 +27,16 @@ pub fn EditDiff() -> impl IntoView {
                    edit.edit.summary,
                    render_date_time(edit.edit.published),
                );
+                let pending = edit.edit.pending;
                view! {
-                    <h2 class="my-2 font-serif text-xl font-bold">{label}</h2>
+                    <div class="flex w-full">
+                        <h2 class="my-2 font-serif text-xl font-bold grow">{label}</h2>
+                        <Show when=move || pending>
+                            <span class="p-1 w-min rounded border-2 border-rose-300 h-min">
+                                Pending
+                            </span>
+                        </Show>
+                    </div>
                    <p>"by " {user_link(&edit.creator)}</p>
                    <div class="p-2 my-2 bg-gray-200 rounded">
                        <pre class="text-wrap">
--- a/tests/common.rs
+++ b/tests/common.rs
@ -32,8 +32,8 @@ impl TestData {
        INIT.call_once(|| {
            env_logger::builder()
                .filter_level(LevelFilter::Warn)
-                .filter_module("activitypub_federation", LevelFilter::Info)
-                .filter_module("ibis", LevelFilter::Info)
+                //.filter_module("activitypub_federation", LevelFilter::Info)
+                //.filter_module("ibis", LevelFilter::Info)
                .init();
        });

--- a/tests/test.rs
+++ b/tests/test.rs
@ -28,7 +28,7 @@ async fn test_create_read_and_edit_local_article() -> Result<()> {
    let TestData(alpha, beta, gamma) = TestData::start(false).await;

    // create article
-    const TITLE: &'static str = "Manu_Chao";
+    const TITLE: &str = "Manu_Chao";
    let create_form = CreateArticleForm {
        title: "Manu Chao".to_string(),
        text: TEST_ARTICLE_DEFAULT_TEXT.to_string(),
@ -337,8 +337,8 @@ async fn test_edit_remote_article() -> Result<()> {
        .starts_with(&edit_res.article.ap_id.to_string()));

    // edit should be federated to beta and gamma
-    let get_res = alpha.get_article(get_article_data_alpha).await.unwrap();
-    let edits = alpha.get_article_edits(get_res.article.id).await.unwrap();
+    let get_res = beta.get_article(get_article_data_alpha).await.unwrap();
+    let edits = beta.get_article_edits(get_res.article.id).await.unwrap();
    assert_eq!(edit_res.article.title, get_res.article.title);
    assert_eq!(edits.len(), 2);
    assert_eq!(edit_res.article.text, get_res.article.text);
@ -456,7 +456,7 @@ async fn test_federated_edit_conflict() -> Result<()> {
    let edit_form = EditArticleForm {
        article_id: get_res.article.id,
        new_text: "Lorem Ipsum\n".to_string(),
-        summary: "summary".to_string(),
+        summary: "first edit".to_string(),
        previous_version_id: create_res.latest_version.clone(),
        resolve_conflict_id: None,
    };
@ -476,14 +476,15 @@ async fn test_federated_edit_conflict() -> Result<()> {
    let edit_form = EditArticleForm {
        article_id: resolve_res.article.id,
        new_text: "aaaa\n".to_string(),
-        summary: "summary".to_string(),
+        summary: "second edit".to_string(),
        previous_version_id: create_res.latest_version,
        resolve_conflict_id: None,
    };
    let edit_res = gamma.edit_article(&edit_form).await.unwrap();
    let gamma_edits = gamma.get_article_edits(edit_res.article.id).await.unwrap();
    assert_ne!(edit_form.new_text, edit_res.article.text);
-    assert_eq!(1, gamma_edits.len());
+    assert_eq!(2, gamma_edits.len());
+    assert!(gamma_edits[1].edit.pending);
    assert!(!edit_res.article.local);

    assert_eq!(1, gamma.notifications_count().await.unwrap());
@ -497,7 +498,7 @@ async fn test_federated_edit_conflict() -> Result<()> {
    let edit_form = EditArticleForm {
        article_id: resolve_res.article.id,
        new_text: "aaaa\n".to_string(),
-        summary: "summary".to_string(),
+        summary: "resolve conflict".to_string(),
        previous_version_id: conflict.previous_version_id.clone(),
        resolve_conflict_id: Some(conflict.id),
    };
@ -505,6 +506,7 @@ async fn test_federated_edit_conflict() -> Result<()> {
    let gamma_edits = gamma.get_article_edits(edit_res.article.id).await.unwrap();
    assert_eq!(edit_form.new_text, edit_res.article.text);
    assert_eq!(3, gamma_edits.len());
+    assert!(gamma_edits.iter().all(|e| !e.edit.pending));

    assert_eq!(0, gamma.notifications_count().await.unwrap());
    let notifications = gamma.notifications_list().await.unwrap();
				`@ -0,0 +1 @@`
				`alter table edit add column pending bool not null default false;`