Skip to content
zhenyi

gitdata-core

0
Clone

HTTPS

SSH

SSH needs a key in your settings.

Download ZIP

refactor(chat,issues): name the positional uuid arguments

Recording an issue event took four trailing Options in a fixed order and
every call site passed them as `None, None, None, Some(name)` - correct
only as long as nobody reorders the parameters. IssueEventDetail bundles
comment_id / old_title / new_title / content, and the @everyone fan-out
gets the same treatment as EveryoneMention (room, space, privacy, message,
actor and the explicit list are all Uuid-shaped and trivially transposed).
ZhenYi committed
parent 21bacd8 commit db52eb2 Browse files

9 files changed

+150 −86

File tree 9
Modified lib/chat/src/share/notify.rs View lib/chat/src/share/notify.rs
@@ -176,19 +176,31 @@
176176 Ok(NotifyDecision::Notify)
177177}
178178
179/// One @everyone fan-out. Grouped into a struct because the room scope, the
180/// message and the actor are all `Uuid`-shaped and would be easy to transpose.
181pub struct EveryoneMention<'a> {
182 pub room_id: Uuid,
183 pub space_id: Option<Uuid>,
184 pub is_private: bool,
185 pub message_id: Uuid,
186 pub body: String,
187 pub actor: Uuid,
188 pub explicit: &'a [Uuid],
189}
190
179191/// Fan out an @everyone mention to room/space members that were not explicitly
180192/// mentioned. Runs best-effort after commit: writes notifications and recent
181193/// mentions, then pushes a WS user event.
182pub async fn fanout_everyone_mentions(
183 service: &ChatService,
184 room_id: Uuid,
185 space_id: Option<Uuid>,
186 is_private: bool,
187 message_id: Uuid,
188 body: String,
189 actor: Uuid,
190 explicit: &[Uuid],
191) {
194pub async fn fanout_everyone_mentions(service: &ChatService, scope: EveryoneMention<'_>) {
195 let EveryoneMention {
196 room_id,
197 space_id,
198 is_private,
199 message_id,
200 body,
201 actor,
202 explicit,
203 } = scope;
192204 let candidates: Vec<Uuid> = if is_private || space_id.is_none() {
193205 room_members::Entity::find()
194206 .filter(room_members::Column::RoomId.eq(room_id))
Modified lib/chat/src/worker.rs View lib/chat/src/worker.rs
@@ -36,7 +36,7 @@
3636 tracing::warn!(%error, "chat outbound webhook delivery failed");
3737 }
3838 tick = tick.wrapping_add(1);
39 if tick % 120 == 0 {
39 if tick.is_multiple_of(120) {
4040 if let Err(error) = service.prune_old_mentions().await {
4141 tracing::warn!(%error, "chat mention prune failed");
4242 }
Modified lib/issues/src/services/comments.rs View lib/issues/src/services/comments.rs
@@ -15,6 +15,7 @@
1515use uuid::Uuid;
1616
1717use crate::errors::IssueError;
18use crate::services::service::IssueEventDetail;
1819use crate::services::service::IssueService;
1920use crate::share::authz::is_board_manager;
2021use crate::share::lookup::{issue_context, require_issue_access};
@@ -157,10 +158,12 @@
157158 issue_id,
158159 caller,
159160 IssueEventType::CommentCreated,
160 Some(comment_id),
161 None,
162 None,
163 None,
161 IssueEventDetail {
162 comment_id: Some(comment_id),
163 old_title: None,
164 new_title: None,
165 content: None,
166 },
164167 )
165168 .await?;
166169
@@ -252,17 +255,9 @@
252255 let locale = self.recipient_locale(uid).await;
253256 let is_reply = Some(uid) == parent_poster_id;
254257 let subject = if is_reply {
255 i18n::catalog::issue_reply_subject(
256 locale,
257 item.autoincrement_id,
258 &item.title,
259 )
258 i18n::catalog::issue_reply_subject(locale, item.autoincrement_id, &item.title)
260259 } else {
261 i18n::catalog::issue_mention_subject(
262 locale,
263 item.autoincrement_id,
264 &item.title,
265 )
260 i18n::catalog::issue_mention_subject(locale, item.autoincrement_id, &item.title)
266261 };
267262 let link = format!(
268263 "{}/issues/{}/{}/{}",
@@ -506,10 +501,12 @@
506501 issue_id,
507502 caller,
508503 IssueEventType::CommentUpdated,
509 Some(comment_id),
510 None,
511 None,
512 None,
504 IssueEventDetail {
505 comment_id: Some(comment_id),
506 old_title: None,
507 new_title: None,
508 content: None,
509 },
513510 )
514511 .await?;
515512 Ok(u)
@@ -571,10 +568,12 @@
571568 issue_id,
572569 caller,
573570 IssueEventType::CommentDeleted,
574 Some(comment_id),
575 None,
576 None,
577 None,
571 IssueEventDetail {
572 comment_id: Some(comment_id),
573 old_title: None,
574 new_title: None,
575 content: None,
576 },
578577 )
579578 .await?;
580579 Ok(())
Modified lib/issues/src/services/dependencies.rs View lib/issues/src/services/dependencies.rs
@@ -9,6 +9,7 @@
99use uuid::Uuid;
1010
1111use crate::errors::IssueError;
12use crate::services::service::IssueEventDetail;
1213use crate::services::service::IssueService;
1314use crate::share::authz::require_board_manage;
1415use crate::share::lookup::{lookup_board, lookup_issue, require_issue_access, space_context};
@@ -102,10 +103,12 @@
102103 issue_id,
103104 caller,
104105 IssueEventType::DependencyAdded,
105 None,
106 None,
107 None,
108 Some(dep_target.to_string()),
106 IssueEventDetail {
107 comment_id: None,
108 old_title: None,
109 new_title: None,
110 content: Some(dep_target.to_string()),
111 },
109112 )
110113 .await?;
111114
@@ -188,10 +191,12 @@
188191 issue_id,
189192 caller,
190193 IssueEventType::DependencyRemoved,
191 None,
192 None,
193 None,
194 Some(dep_target.to_string()),
194 IssueEventDetail {
195 comment_id: None,
196 old_title: None,
197 new_title: None,
198 content: Some(dep_target.to_string()),
199 },
195200 )
196201 .await?;
197202
Modified lib/issues/src/services/issues.rs View lib/issues/src/services/issues.rs
@@ -31,6 +31,7 @@
3131use uuid::Uuid;
3232
3333use crate::errors::IssueError;
34use crate::services::service::IssueEventDetail;
3435use crate::services::service::IssueService;
3536use crate::share::authz::is_board_manager;
3637use crate::share::lookup::{board_context, issue_context, lookup_issue_by_ref};
@@ -92,7 +93,10 @@
9293 pub milestone_id: Option<Uuid>,
9394 #[serde(default)]
9495 pub priority: IssuePriority,
95 #[serde(default, deserialize_with = "export::serde_datetime::deserialize_opt_datetime")]
96 #[serde(
97 default,
98 deserialize_with = "export::serde_datetime::deserialize_opt_datetime"
99 )]
96100 pub due_at: Option<OffsetDateTime>,
97101 #[serde(default)]
98102 pub assignee_ids: Vec<Uuid>,
@@ -465,10 +469,12 @@
465469 issue_id,
466470 caller,
467471 IssueEventType::Created,
468 None,
469 None,
470 Some(title_clone.clone()),
471 None,
472 IssueEventDetail {
473 comment_id: None,
474 old_title: None,
475 new_title: Some(title_clone.clone()),
476 content: None,
477 },
472478 )
473479 .await?;
474480
@@ -523,8 +529,11 @@
523529 let emails = self.primary_emails(&mentioned_uids).await?;
524530 for (uid, addr) in emails {
525531 let locale = self.recipient_locale(uid).await;
526 let subject =
527 i18n::catalog::issue_mention_subject(locale, inserted.autoincrement_id, &inserted.title);
532 let subject = i18n::catalog::issue_mention_subject(
533 locale,
534 inserted.autoincrement_id,
535 &inserted.title,
536 );
528537 let link = format!(
529538 "{}/issues/{}/{}/{}",
530539 self.config().web.web_base_url,
@@ -874,10 +883,12 @@
874883 issue_id,
875884 caller,
876885 IssueEventType::TitleUpdated,
877 None,
878 Some(old_title.clone()),
879 Some(new_t.clone()),
880 None,
886 IssueEventDetail {
887 comment_id: None,
888 old_title: Some(old_title.clone()),
889 new_title: Some(new_t.clone()),
890 content: None,
891 },
881892 )
882893 .await?;
883894
@@ -944,10 +955,12 @@
944955 issue_id,
945956 caller,
946957 IssueEventType::MilestoneUpdated,
947 None,
948 None,
949 None,
950 Some(content),
958 IssueEventDetail {
959 comment_id: None,
960 old_title: None,
961 new_title: None,
962 content: Some(content),
963 },
951964 )
952965 .await?;
953966 }
@@ -1031,10 +1044,7 @@
10311044 } else {
10321045 IssueEventType::Reopened
10331046 },
1034 None,
1035 None,
1036 None,
1037 None,
1047 IssueEventDetail::default(),
10381048 )
10391049 .await?;
10401050 updated += 1;
@@ -1108,8 +1118,14 @@
11081118 .transaction::<_, issue_item::Model, IssueError>(move |txn| {
11091119 Box::pin(async move {
11101120 let u = am.update(txn).await?;
1111 this.write_event(txn, issue_id, caller, event_type, None, None, None, None)
1112 .await?;
1121 this.write_event(
1122 txn,
1123 issue_id,
1124 caller,
1125 event_type,
1126 IssueEventDetail::default(),
1127 )
1128 .await?;
11131129 Ok(u)
11141130 })
11151131 })
Modified lib/issues/src/services/labels.rs View lib/issues/src/services/labels.rs
@@ -11,6 +11,7 @@
1111use uuid::Uuid;
1212
1313use crate::errors::IssueError;
14use crate::services::service::IssueEventDetail;
1415use crate::services::service::IssueService;
1516use crate::share::authz::require_board_manage;
1617use crate::share::lookup::{
@@ -438,10 +439,12 @@
438439 issue_id,
439440 caller,
440441 IssueEventType::LabelAdded,
441 None,
442 None,
443 None,
444 Some(name_clone),
442 IssueEventDetail {
443 comment_id: None,
444 old_title: None,
445 new_title: None,
446 content: Some(name_clone),
447 },
445448 )
446449 .await?;
447450
@@ -490,10 +493,12 @@
490493 issue_id,
491494 caller,
492495 IssueEventType::LabelRemoved,
493 None,
494 None,
495 None,
496 Some(name_clone),
496 IssueEventDetail {
497 comment_id: None,
498 old_title: None,
499 new_title: None,
500 content: Some(name_clone),
501 },
497502 )
498503 .await?;
499504
Modified lib/issues/src/services/members.rs View lib/issues/src/services/members.rs
@@ -13,6 +13,7 @@
1313use uuid::Uuid;
1414
1515use crate::errors::IssueError;
16use crate::services::service::IssueEventDetail;
1617use crate::services::service::IssueService;
1718use crate::share::authz::require_board_manage;
1819use crate::share::lookup::{lookup_board, lookup_issue, require_issue_access, space_context};
@@ -122,10 +123,12 @@
122123 issue_id,
123124 caller,
124125 IssueEventType::MemberAdded,
125 None,
126 None,
127 None,
128 Some(content_str),
126 IssueEventDetail {
127 comment_id: None,
128 old_title: None,
129 new_title: None,
130 content: Some(content_str),
131 },
129132 )
130133 .await?;
131134
@@ -159,8 +162,11 @@
159162 let emails = self.primary_emails(&[target_uid]).await?;
160163 if let Some(addr) = emails.get(&target_uid) {
161164 let locale = self.recipient_locale(target_uid).await;
162 let subject =
163 i18n::catalog::issue_assigned_subject(locale, item.autoincrement_id, &item.title);
165 let subject = i18n::catalog::issue_assigned_subject(
166 locale,
167 item.autoincrement_id,
168 &item.title,
169 );
164170 let link = format!(
165171 "{}/issues/{}/{}/{}",
166172 self.config().web.web_base_url,
@@ -281,10 +287,12 @@
281287 issue_id,
282288 caller,
283289 IssueEventType::MemberRemoved,
284 None,
285 None,
286 None,
287 Some(content_str),
290 IssueEventDetail {
291 comment_id: None,
292 old_title: None,
293 new_title: None,
294 content: Some(content_str),
295 },
288296 )
289297 .await?;
290298
Modified lib/issues/src/services/milestones.rs View lib/issues/src/services/milestones.rs
@@ -38,7 +38,10 @@
3838 pub name: String,
3939 #[serde(default)]
4040 pub description: Option<String>,
41 #[serde(default, deserialize_with = "export::serde_datetime::deserialize_opt_datetime")]
41 #[serde(
42 default,
43 deserialize_with = "export::serde_datetime::deserialize_opt_datetime"
44 )]
4245 pub due_at: Option<OffsetDateTime>,
4346}
4447
Modified lib/issues/src/services/service.rs View lib/issues/src/services/service.rs
@@ -5,8 +5,8 @@
55use config::AppConfig;
66use email::EmailService;
77use export::AppState;
8use export::database::AppDatabase;
89use i18n;
9use export::database::AppDatabase;
1010use models::issue::issue_event_line::{self, IssueEventType};
1111use models::space::space_members::{self, MemberStatus};
1212use models::space::spaces;
@@ -26,6 +26,17 @@
2626 pub state: AppState,
2727}
2828
29/// The optional "what changed" columns of an issue activity row. Grouped into a
30/// struct so `write_event` does not take four positionally-interchangeable
31/// `Option` arguments.
32#[derive(Default)]
33pub(crate) struct IssueEventDetail {
34 pub comment_id: Option<Uuid>,
35 pub old_title: Option<String>,
36 pub new_title: Option<String>,
37 pub content: Option<String>,
38}
39
2940impl IssueService {
3041 pub fn new(state: AppState) -> Self {
3142 Self { state }
@@ -96,17 +107,22 @@
96107 )
97108 }
98109
110 /// Record one activity row. The optional columns all describe "what changed"
111 /// and are grouped so the call sites stay readable.
99112 pub(crate) async fn write_event(
100113 &self,
101114 txn: &impl ConnectionTrait,
102115 issue_id: Uuid,
103116 user_id: Uuid,
104117 event_type: IssueEventType,
105 comment_id: Option<Uuid>,
106 old_title: Option<String>,
107 new_title: Option<String>,
108 content: Option<String>,
118 detail: IssueEventDetail,
109119 ) -> Result<(), IssueError> {
120 let IssueEventDetail {
121 comment_id,
122 old_title,
123 new_title,
124 content,
125 } = detail;
110126 issue_event_line::ActiveModel {
111127 id: Set(Uuid::now_v7()),
112128 issue_id: Set(Some(issue_id)),