Consolidated case check for moderate actions. (#7733)

dart-lang · May 17, 2024 · b2af3da · b2af3da
1 parent b12655d
commit b2af3da
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 38 deletions.
diff --git a/app/lib/admin/actions/moderate_package.dart b/app/lib/admin/actions/moderate_package.dart
@@ -26,10 +26,6 @@ Set the moderated flag on a package (updating the flag and the timestamp).
   },
   invoke: (options) async {
     final caseId = options['case'];
-    InvalidInputException.check(
-      caseId != null && caseId.isNotEmpty,
-      'case must be given',
-    );
 
     final package = options['package'];
     InvalidInputException.check(
@@ -50,13 +46,11 @@ Set the moderated flag on a package (updating the flag and the timestamp).
 
     final message = options['message'];
 
-    final refCase = await adminBackend.lookupModerationCase(caseId!);
-    if (refCase == null) {
-      throw NotFoundException.resource(caseId);
-    }
-    if (refCase.status != ModerationStatus.pending) {
-      throw InvalidInputException('ModerationCase is already closed.');
-    }
+    final refCase =
+        await adminBackend.loadAndVerifyModerationCaseForAdminAction(
+      caseId,
+      status: ModerationStatus.pending,
+    );
 
     final p = await packageBackend.lookupPackage(package!);
     if (p == null) {
@@ -70,13 +64,15 @@ Set the moderated flag on a package (updating the flag and the timestamp).
         pkg.updateIsModerated(isModerated: valueToSet!);
         tx.insert(pkg);
 
-        final mc = await tx.lookupValue<ModerationCase>(refCase.key);
-        mc.addActionLogEntry(
-          ModerationSubject.package(package).fqn,
-          valueToSet ? ModerationAction.apply : ModerationAction.revert,
-          message,
-        );
-        tx.insert(mc);
+        if (refCase != null) {
+          final mc = await tx.lookupValue<ModerationCase>(refCase.key);
+          mc.addActionLogEntry(
+            ModerationSubject.package(package).fqn,
+            valueToSet ? ModerationAction.apply : ModerationAction.revert,
+            message,
+          );
+          tx.insert(mc);
+        }
 
         return pkg;
       });

diff --git a/app/lib/admin/actions/moderate_publisher.dart b/app/lib/admin/actions/moderate_publisher.dart
@@ -28,10 +28,6 @@ can't be updated, administrators must not be able to update publisher options.
   },
   invoke: (options) async {
     final caseId = options['case'];
-    InvalidInputException.check(
-      caseId != null && caseId.isNotEmpty,
-      'case must be given',
-    );
 
     final publisherId = options['publisher'];
     InvalidInputException.check(
@@ -56,13 +52,11 @@ can't be updated, administrators must not be able to update publisher options.
 
     final message = options['message'];
 
-    final refCase = await adminBackend.lookupModerationCase(caseId!);
-    if (refCase == null) {
-      throw NotFoundException.resource(caseId);
-    }
-    if (refCase.status != ModerationStatus.pending) {
-      throw InvalidInputException('ModerationCase is already closed.');
-    }
+    final refCase =
+        await adminBackend.loadAndVerifyModerationCaseForAdminAction(
+      caseId,
+      status: ModerationStatus.pending,
+    );
 
     Publisher? publisher2;
     if (valueToSet != null) {
@@ -71,13 +65,15 @@ can't be updated, administrators must not be able to update publisher options.
         p.updateIsModerated(isModerated: valueToSet!);
         tx.insert(p);
 
-        final mc = await tx.lookupValue<ModerationCase>(refCase.key);
-        mc.addActionLogEntry(
-          ModerationSubject.publisher(publisherId).fqn,
-          valueToSet ? ModerationAction.apply : ModerationAction.revert,
-          message,
-        );
-        tx.insert(mc);
+        if (refCase != null) {
+          final mc = await tx.lookupValue<ModerationCase>(refCase.key);
+          mc.addActionLogEntry(
+            ModerationSubject.publisher(publisherId).fqn,
+            valueToSet ? ModerationAction.apply : ModerationAction.revert,
+            message,
+          );
+          tx.insert(mc);
+        }
 
         return p;
       });

diff --git a/app/lib/admin/backend.dart b/app/lib/admin/backend.dart
@@ -722,4 +722,31 @@ class AdminBackend {
     return await dbService.lookupOrNull<ModerationCase>(
         dbService.emptyKey.append(ModerationCase, id: caseId));
   }
+
+  /// Returns a valid [ModerationCase] if it exists and [status] is matching.
+  /// Returns `null` if [caseId] is `none`.
+  ///
+  /// Throws exceptions otherwise.
+  Future<ModerationCase?> loadAndVerifyModerationCaseForAdminAction(
+    String? caseId, {
+    required String? status,
+  }) async {
+    InvalidInputException.check(
+      caseId != null && caseId.isNotEmpty,
+      'case must be given',
+    );
+    if (caseId == 'none') {
+      return null;
+    }
+
+    final refCase = await adminBackend.lookupModerationCase(caseId!);
+    if (refCase == null) {
+      throw NotFoundException.resource(caseId);
+    }
+    if (status != null && refCase.status != status) {
+      throw InvalidInputException(
+          'ModerationCase.status ("${refCase.status}") != "$status".');
+    }
+    return refCase;
+  }
 }
diff --git a/app/test/package/moderate_package_test.dart b/app/test/package/moderate_package_test.dart
@@ -357,7 +357,7 @@ void main() {
         _moderate('oxygen', state: true, caseId: mc.caseId),
         code: 'InvalidInput',
         status: 400,
-        message: 'ModerationCase is already closed',
+        message: 'ModerationCase.status ("no-action") != "pending".',
       );
     });
   });

diff --git a/app/test/publisher/moderate_publisher_test.dart b/app/test/publisher/moderate_publisher_test.dart
@@ -198,7 +198,7 @@ void main() {
         _moderate('example.com', state: true, caseId: mc.caseId),
         code: 'InvalidInput',
         status: 400,
-        message: 'ModerationCase is already closed',
+        message: 'ModerationCase.status ("no-action") != "pending".',
       );
     });
   });