fix(backend): moderation locking logic fix (#5979)

* fix(backend): moderation locking logic fix

* fix: clippy

apps/labrinth/src/routes/v3/projects.rs

@@ -424,28 +424,22 @@ pub async fn project_edit_internal(
             ));
         }
 
-        // If a moderator (non-admin) is completing a review (changing from Processing to another
+        // If a moderator (non-admin) is completing a review while another moderator holds an
-        // status), they must hold an active non-expired lock on this project.
+        // active checklist lock, block them from changing the project status.
         if user.role.is_mod()
             && !user.role.is_admin()
             && project_item.inner.status == ProjectStatus::Processing
             && status != &ProjectStatus::Processing
-        {
+            && let Some(lock) =
-            let lock =
                 DBModerationLock::get_with_user(project_item.inner.id, &pool)
-                    .await?;
+                    .await?
-            let owns = lock.as_ref().is_some_and(|l| {
+            && lock.moderator_id != db_ids::DBUserId::from(user.id)
-                l.moderator_id == db_ids::DBUserId::from(user.id) && !l.expired
+            && !lock.expired
-            });
+        {
-            if !owns {
+            return Err(ApiError::CustomAuthentication(format!(
-                return Err(ApiError::CustomAuthentication(match lock {
+                "This project is currently being moderated by @{}. Please wait for them to finish or for the lock to expire.",
-                    Some(l) => format!(
+                lock.moderator_username
-                        "This project is currently being moderated by @{}. Please wait for them to finish or for the lock to expire.",
+            )));
-                        l.moderator_username
-                    ),
-                    None => "You must hold an active moderation lock to complete this review. Open the project in the moderation checklist to acquire one.".to_string(),
-                }));
-            }
         }
 
         if status == &ProjectStatus::Processing {

apps/labrinth/src/test/api_v2/project.rs

@@ -95,7 +95,7 @@ impl ApiProject for ApiV2 {
         let resp = self.create_project(creation_data, pat).await;
         assert_status!(&resp, StatusCode::OK);
 
-        // Approve as admin so fixture setup is not blocked by the moderation-lock guard.
+        // Approve as admin so fixture setup is not affected by moderation-lock contention.
         let req = TestRequest::patch()
             .uri(&format!("/v2/project/{slug}"))
             .append_pat(ADMIN_USER_PAT)

apps/labrinth/src/test/api_v3/project.rs

@@ -49,8 +49,7 @@ impl ApiProject for ApiV3 {
         let resp = self.create_project(creation_data, pat).await;
         assert_status!(&resp, StatusCode::OK);
 
-        // Approve as admin so fixture setup is not blocked by the moderation-lock guard
+        // Approve as admin so fixture setup is not affected by moderation-lock contention.
-        // (non-admin moderators must hold a lock to move a project out of processing).
         let req = TestRequest::patch()
             .uri(&format!("/v3/project/{slug}"))
             .append_pat(ADMIN_USER_PAT)

apps/labrinth/tests/moderation_lock.rs

@@ -769,9 +769,9 @@ async fn test_http_delete_all_locks_admin() {
     .await;
 }
 
-/// Moderator without a lock cannot move a project out of processing (401 + lock message).
+/// Moderator without a lock can move a project out of processing when nobody else has a lock.
 #[actix_rt::test]
-async fn test_complete_review_requires_lock() {
+async fn test_complete_review_without_lock_succeeds() {
     with_test_environment(None, |test_env: TestEnvironment<common::api_v3::ApiV3>| async move {
         let api = &test_env.api;
         let alpha_id = &test_env.dummy.project_alpha.project_id;
@@ -793,8 +793,8 @@ async fn test_complete_review_requires_lock() {
         let resp = api.call(req).await;
         assert_eq!(
             resp.status(),
-            StatusCode::UNAUTHORIZED,
+            StatusCode::NO_CONTENT,
-            "moderator without lock should not be able to complete review: {}",
+            "moderator without lock should be able to complete review when no active lock exists: {}",
             String::from_utf8_lossy(test::read_body(resp).await.as_ref())
         );
     })
@@ -849,7 +849,7 @@ async fn test_complete_review_blocked_when_other_holds_lock() {
 }
 
 #[actix_rt::test]
-async fn test_complete_review_fails_when_lock_expired() {
+async fn test_complete_review_succeeds_when_other_lock_expired() {
     with_test_environment(
         None,
         |test_env: TestEnvironment<common::api_v3::ApiV3>| async move {
@@ -858,7 +858,6 @@ async fn test_complete_review_fails_when_lock_expired() {
             let project_id = DBProjectId(
                 test_env.dummy.project_alpha.project_id_parsed.0 as i64,
             );
-            let mod_id = DBUserId(MOD_USER_ID_PARSED);
             let pool = &test_env.db.pool;
 
             set_project_processing(
@@ -868,7 +867,11 @@ async fn test_complete_review_fails_when_lock_expired() {
                 &test_env.db.redis_pool,
             )
             .await;
-            DBModerationLock::acquire(project_id, mod_id, pool)
+            DBModerationLock::acquire(
+                project_id,
+                DBUserId(ADMIN_USER_ID_PARSED),
+                pool,
+            )
                 .await
                 .unwrap()
                 .unwrap();
@@ -881,7 +884,12 @@ async fn test_complete_review_fails_when_lock_expired() {
                 .to_request();
 
             let resp = api.call(req).await;
-            assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
+            assert_eq!(
+                resp.status(),
+                StatusCode::NO_CONTENT,
+                "moderator should be able to complete review when another moderator's lock expired: {}",
+                String::from_utf8_lossy(test::read_body(resp).await.as_ref())
+            );
         },
     )
     .await;