Write to mod_change_community_visibility table (fixes #5704) (#5706)

* Write to mod_change_community_visibility table (fixes #5704)

* cleanup duplicate code

* remove todo

* ci

* fix down migration

crates/api_crud/src/community/update.rs

@@ -19,6 +19,7 @@ use lemmy_db_schema::{
   source::{
     actor_language::{CommunityLanguage, SiteLanguage},
     community::{Community, CommunityUpdateForm},
+    mod_log::moderator::{ModChangeCommunityVisibility, ModChangeCommunityVisibilityForm},
   },
   traits::Crud,
   utils::diesel_string_update,
@@ -86,6 +87,15 @@ pub async fn update_community(
   let community_id = data.community_id;
   let community = Community::update(&mut context.pool(), community_id, &community_form).await?;
 
+  if old_community.visibility != community.visibility {
+    let form = ModChangeCommunityVisibilityForm {
+      mod_person_id: local_user_view.person.id,
+      community_id: community.id,
+      visibility: community.visibility,
+    };
+    ModChangeCommunityVisibility::create(&mut context.pool(), &form).await?;
+  }
+
   ActivityChannel::submit_activity(
     SendActivityData::UpdateCommunity(local_user_view.person.clone(), community),
     &context,

crates/apub/src/activities/community/update.rs

@@ -9,24 +9,19 @@ use activitypub_federation::{
   kinds::activity::UpdateType,
   traits::{ActivityHandler, Actor, Object},
 };
-use chrono::Utc;
 use lemmy_api_common::context::LemmyContext;
 use lemmy_apub_objects::{
   objects::{community::ApubCommunity, person::ApubPerson},
   utils::{
-    functions::{
+    functions::{generate_to, verify_person_in_community, verify_visibility},
-      generate_to,
+    protocol::InCommunity,
-      read_from_string_or_source_opt,
-      verify_person_in_community,
-      verify_visibility,
-    },
-    protocol::{AttributedTo, InCommunity},
   },
 };
 use lemmy_db_schema::{
   source::{
     activity::ActivitySendTargets,
-    community::{Community, CommunityUpdateForm},
+    community::Community,
+    mod_log::moderator::{ModChangeCommunityVisibility, ModChangeCommunityVisibilityForm},
     person::Person,
   },
   traits::Crud,
@@ -90,39 +85,19 @@ impl ActivityHandler for UpdateCommunity {
 
   async fn receive(self, context: &Data<Self::DataType>) -> LemmyResult<()> {
     insert_received_activity(&self.id, context).await?;
-    let community = self.community(context).await?;
+    let old_community = self.community(context).await?;
 
-    let community_update_form = CommunityUpdateForm {
+    let community = ApubCommunity::from_json(*self.object, context).await?;
-      title: Some(self.object.name.unwrap_or(self.object.preferred_username)),
-      description: Some(read_from_string_or_source_opt(
-        &self.object.summary,
-        &None,
-        &self.object.source,
-      )),
-      published: self.object.published,
-      updated: Some(self.object.updated),
-      nsfw: Some(self.object.sensitive.unwrap_or(false)),
-      ap_id: Some(self.object.id.into()),
-      public_key: Some(self.object.public_key.public_key_pem),
-      last_refreshed_at: Some(Utc::now()),
-      icon: Some(self.object.icon.map(|i| i.url.into())),
-      banner: Some(self.object.image.map(|i| i.url.into())),
-      followers_url: self.object.followers.map(Into::into),
-      inbox_url: Some(
-        self
-          .object
-          .endpoints
-          .map(|e| e.shared_inbox)
-          .unwrap_or(self.object.inbox)
-          .into(),
-      ),
-      moderators_url: Some(self.object.attributed_to.and_then(AttributedTo::url)),
-      posting_restricted_to_mods: self.object.posting_restricted_to_mods,
-      featured_url: Some(self.object.featured.map(Into::into)),
-      ..Default::default()
-    };
 
-    Community::update(&mut context.pool(), community.id, &community_update_form).await?;
+    if old_community.visibility != community.visibility {
+      let actor = self.actor.dereference(context).await?;
+      let form = ModChangeCommunityVisibilityForm {
+        mod_person_id: actor.id,
+        community_id: old_community.id,
+        visibility: old_community.visibility,
+      };
+      ModChangeCommunityVisibility::create(&mut context.pool(), &form).await?;
+    }
     Ok(())
   }
 }

crates/apub_objects/src/objects/community.rs

@@ -2,7 +2,12 @@ use crate::{
   objects::instance::fetch_instance_actor_for_object,
   protocol::group::Group,
   utils::{
-    functions::{read_from_string_or_source_opt, GetActorType},
+    functions::{
+      check_apub_id_valid_with_strictness,
+      community_visibility,
+      read_from_string_or_source_opt,
+      GetActorType,
+    },
     markdown_links::markdown_rewrite_remote_links_opt,
     protocol::{AttributedTo, ImageObject, LanguageTag, Source},
   },
@@ -10,7 +15,7 @@ use crate::{
 use activitypub_federation::{
   config::Data,
   kinds::actor::GroupType,
-  protocol::values::MediaTypeHtml,
+  protocol::{values::MediaTypeHtml, verification::verify_domains_match},
   traits::{Actor, Object},
 };
 use chrono::{DateTime, Utc};
@@ -39,7 +44,11 @@ use lemmy_db_schema_file::enums::{ActorType, CommunityVisibility};
 use lemmy_db_views_site::SiteView;
 use lemmy_utils::{
   error::{LemmyError, LemmyResult},
-  utils::{markdown::markdown_to_html, validation::truncate_description},
+  utils::{
+    markdown::markdown_to_html,
+    slurs::{check_slurs, check_slurs_opt},
+    validation::truncate_description,
+  },
 };
 use once_cell::sync::OnceCell;
 use std::ops::Deref;
@@ -137,7 +146,15 @@ impl Object for ApubCommunity {
     expected_domain: &Url,
     context: &Data<Self::DataType>,
   ) -> LemmyResult<()> {
-    group.verify(expected_domain, context).await
+    check_apub_id_valid_with_strictness(group.id.inner(), true, context).await?;
+    verify_domains_match(expected_domain, group.id.inner())?;
+
+    let slur_regex = slur_regex(context).await?;
+
+    check_slurs(&group.preferred_username, &slur_regex)?;
+    check_slurs_opt(&group.name, &slur_regex)?;
+    check_slurs_opt(&group.summary, &slur_regex)?;
+    Ok(())
   }
 
   /// Converts a `Group` to `Community`, inserts it into the database and updates moderators.
@@ -155,13 +172,7 @@ impl Object for ApubCommunity {
     let sidebar = markdown_rewrite_remote_links_opt(sidebar, context).await;
     let icon = proxy_image_link_opt_apub(group.icon.clone().map(|i| i.url), context).await?;
     let banner = proxy_image_link_opt_apub(group.image.clone().map(|i| i.url), context).await?;
-    let visibility = Some(if group.manually_approves_followers.unwrap_or_default() {
+    let visibility = Some(community_visibility(&group));
-      CommunityVisibility::Private
-    } else if !group.discoverable.unwrap_or(true) {
-      CommunityVisibility::Unlisted
-    } else {
-      CommunityVisibility::Public
-    });
 
     // If NSFW is not allowed, then remove NSFW communities
     let removed = check_nsfw_allowed(group.sensitive, local_site.as_ref())

crates/apub_objects/src/protocol/group.rs

@@ -1,27 +1,13 @@
 use crate::{
   objects::community::ApubCommunity,
-  utils::{
+  utils::protocol::{AttributedTo, Endpoints, ImageObject, LanguageTag, Source},
-    functions::check_apub_id_valid_with_strictness,
-    protocol::{AttributedTo, Endpoints, ImageObject, LanguageTag, Source},
-  },
 };
 use activitypub_federation::{
-  config::Data,
   fetch::object_id::ObjectId,
   kinds::actor::GroupType,
-  protocol::{
+  protocol::{helpers::deserialize_skip_error, public_key::PublicKey, values::MediaTypeHtml},
-    helpers::deserialize_skip_error,
-    public_key::PublicKey,
-    values::MediaTypeHtml,
-    verification::verify_domains_match,
-  },
 };
 use chrono::{DateTime, Utc};
-use lemmy_api_common::{context::LemmyContext, utils::slur_regex};
-use lemmy_utils::{
-  error::LemmyResult,
-  utils::slurs::{check_slurs, check_slurs_opt},
-};
 use serde::{Deserialize, Serialize};
 use serde_with::skip_serializing_none;
 use std::fmt::Debug;
@@ -72,21 +58,3 @@ pub struct Group {
   /// https://docs.joinmastodon.org/spec/activitypub/#discoverable
   pub(crate) discoverable: Option<bool>,
 }
-
-impl Group {
-  pub(crate) async fn verify(
-    &self,
-    expected_domain: &Url,
-    context: &Data<LemmyContext>,
-  ) -> LemmyResult<()> {
-    check_apub_id_valid_with_strictness(self.id.inner(), true, context).await?;
-    verify_domains_match(expected_domain, self.id.inner())?;
-
-    let slur_regex = slur_regex(context).await?;
-
-    check_slurs(&self.preferred_username, &slur_regex)?;
-    check_slurs_opt(&self.name, &slur_regex)?;
-    check_slurs_opt(&self.summary, &slur_regex)?;
-    Ok(())
-  }
-}

crates/apub_objects/src/utils/functions.rs

@@ -1,7 +1,7 @@
 use super::protocol::Source;
 use crate::{
   objects::{community::ApubCommunity, instance::ApubSite, person::ApubPerson},
-  protocol::page::Attachment,
+  protocol::{group::Group, page::Attachment},
 };
 use activitypub_federation::{
   config::Data,
@@ -304,3 +304,13 @@ pub async fn append_attachments_to_comment(
 
   Ok(content)
 }
+
+pub fn community_visibility(group: &Group) -> CommunityVisibility {
+  if group.manually_approves_followers.unwrap_or_default() {
+    CommunityVisibility::Private
+  } else if !group.discoverable.unwrap_or(true) {
+    CommunityVisibility::Unlisted
+  } else {
+    CommunityVisibility::Public
+  }
+}

crates/db_schema/src/source/mod_log/moderator.rs

@@ -220,7 +220,6 @@ pub struct ModBan {
 pub struct ModChangeCommunityVisibilityForm {
   pub community_id: CommunityId,
   pub mod_person_id: PersonId,
-  pub reason: Option<String>,
   pub visibility: CommunityVisibility,
 }
 
@@ -235,8 +234,6 @@ pub struct ModChangeCommunityVisibility {
   pub community_id: CommunityId,
   pub mod_person_id: PersonId,
   pub published: DateTime<Utc>,
-  #[cfg_attr(feature = "full", ts(optional))]
-  pub reason: Option<String>,
   pub visibility: CommunityVisibility,
 }
 

crates/db_schema_file/src/schema.rs

@@ -614,7 +614,6 @@ diesel::table! {
         community_id -> Int4,
         mod_person_id -> Int4,
         published -> Timestamptz,
-        reason -> Nullable<Text>,
         visibility -> CommunityVisibility,
     }
 }

crates/db_views/modlog_combined/src/impls.rs

@@ -808,7 +808,6 @@ mod tests {
       mod_person_id: data.timmy.id,
       community_id: data.community.id,
       visibility: CommunityVisibility::Unlisted,
-      reason: None,
     };
     ModChangeCommunityVisibility::create(pool, &form).await?;
 
@@ -817,7 +816,6 @@ mod tests {
       mod_person_id: data.jessica.id,
       community_id: data.community_2.id,
       visibility: CommunityVisibility::Unlisted,
-      reason: None,
     };
     ModChangeCommunityVisibility::create(pool, &form).await?;
 

migrations/2025-05-26-092442_mod-change-community-vis/down.sql

@@ -0,0 +1,17 @@
+ALTER TABLE mod_change_community_visibility
+    ADD COLUMN reason text,
+    ADD COLUMN visibility_new community_visibility;
+
+UPDATE
+    mod_change_community_visibility
+SET
+    visibility_new = visibility;
+
+ALTER TABLE mod_change_community_visibility
+    DROP COLUMN visibility;
+
+ALTER TABLE mod_change_community_visibility RENAME COLUMN visibility_new TO visibility;
+
+ALTER TABLE mod_change_community_visibility
+    ALTER COLUMN visibility SET NOT NULL;
+

migrations/2025-05-26-092442_mod-change-community-vis/up.sql

@@ -0,0 +1,3 @@
+ALTER TABLE mod_change_community_visibility
+    DROP COLUMN reason;
+