Integrity checks + admin action updates for ModerationCase.

app/lib/admin/actions/create_moderation_case.dart

@@ -23,7 +23,6 @@ Returns the fields on the newly created moderation case.
     'kind': 'The kind of the moderation case. (default value: `notification`)',
     'source':
         'The source of the moderation case. (default value: `internal-notification`)',
-    'status': 'The status of the moderation case. (default value: `pending`)',
     'subject': 'The subject of the moderation case.',
     'url': 'The url of the moderation case (optional).'
   },
@@ -42,10 +41,6 @@ Returns the fields on the newly created moderation case.
     InvalidInputException.check(
         ModerationSource.isValidSource(source), 'invalid source');
 
-    final status = options['status'] ?? ModerationStatus.pending;
-    InvalidInputException.check(
-        ModerationStatus.isValidStatus(status), 'invalid status');
-
     final subject = options['subject'];
     InvalidInputException.check(
       subject != null && subject.isNotEmpty,
@@ -63,7 +58,7 @@ Returns the fields on the newly created moderation case.
         reporterEmail: reporterEmail,
         source: source,
         kind: kind,
-        status: status,
+        status: ModerationStatus.pending,
         subject: subject,
         isSubjectOwner: false,
         url: url,

app/lib/admin/actions/resolve_moderation_case.dart

@@ -16,6 +16,8 @@ Closes the moderation case and updates the status based on the actions logged on
 ''',
   options: {
     'case': 'The caseId to be closed.',
+    'status':
+        'The resolved status of the case. (optional, will be automatically inferred if absent)'
   },
   invoke: (options) async {
     final caseId = options['case'];
@@ -24,6 +26,14 @@ Closes the moderation case and updates the status based on the actions logged on
       'case must be given',
     );
 
+    var status = options['status'];
+    InvalidInputException.check(
+      status == null ||
+          (status != ModerationStatus.pending &&
+              ModerationStatus.isValidStatus(status)),
+      'invalid status',
+    );
+
     final mc = await withRetryTransaction(dbService, (tx) async {
       final mc = await tx.lookupOrNull<ModerationCase>(
           dbService.emptyKey.append(ModerationCase, id: caseId!));
@@ -38,28 +48,32 @@ Closes the moderation case and updates the status based on the actions logged on
 
       final hasModeratedAction = mc.getActionLog().hasModeratedAction();
 
-      if (mc.kind == ModerationKind.notification) {
-        mc.status = hasModeratedAction
-            ? ModerationStatus.moderationApplied
-            : ModerationStatus.noAction;
-      } else if (mc.kind == ModerationKind.appeal) {
-        final appealedCase = await tx.lookupValue<ModerationCase>(
-            dbService.emptyKey.append(ModerationCase, id: mc.appealedCaseId!));
-        final appealHadModeratedAction =
-            appealedCase.getActionLog().hasModeratedAction();
-        if (appealHadModeratedAction) {
-          mc.status = hasModeratedAction
-              ? ModerationStatus.moderationReverted
-              : ModerationStatus.moderationUpheld;
+      if (status == null) {
+        if (mc.kind == ModerationKind.notification) {
+          status = hasModeratedAction
+              ? ModerationStatus.moderationApplied
+              : ModerationStatus.noAction;
+        } else if (mc.kind == ModerationKind.appeal) {
+          final appealedCase = await tx.lookupValue<ModerationCase>(dbService
+              .emptyKey
+              .append(ModerationCase, id: mc.appealedCaseId!));
+          final appealHadModeratedAction =
+              appealedCase.getActionLog().hasModeratedAction();
+          if (appealHadModeratedAction) {
+            status = hasModeratedAction
+                ? ModerationStatus.moderationReverted
+                : ModerationStatus.moderationUpheld;
+          } else {
+            status = hasModeratedAction
+                ? ModerationStatus.noActionReverted
+                : ModerationStatus.noActionUpheld;
+          }
         } else {
-          mc.status = hasModeratedAction
-              ? ModerationStatus.noActionReverted
-              : ModerationStatus.noActionUpheld;
+          throw UnimplementedError('Kind "${mc.kind}" is not implemented.');
         }
-      } else {
-        throw UnimplementedError('Kind "${mc.kind}" is not implemented.');
       }
 
+      mc.status = status!;
       tx.insert(mc);
       return mc;
     });

app/lib/admin/actions/update_moderation_case.dart

@@ -67,6 +67,12 @@ Returns the fields of the updated moderation case.
       mc.source = source ?? mc.source;
       mc.status = status ?? mc.status;
 
+      InvalidInputException.check(
+        (mc.resolved != null && mc.status != ModerationStatus.pending) ||
+            (mc.resolved == null && mc.status == ModerationStatus.pending),
+        'resolved timestamp in conflict with status',
+      );
+
       tx.insert(mc);
     });
     final mc = await adminBackend.lookupModerationCase(caseId);

app/lib/shared/integrity.dart

@@ -13,21 +13,22 @@ import 'package:gcloud/storage.dart';
 import 'package:http/http.dart' as http;
 import 'package:logging/logging.dart';
 import 'package:pool/pool.dart';
-import 'package:pub_dev/publisher/backend.dart';
-import 'package:pub_dev/shared/env_config.dart';
 import 'package:retry/retry.dart';
 
 import '../account/agent.dart';
 import '../account/models.dart';
+import '../admin/models.dart';
 import '../audit/models.dart';
 import '../package/backend.dart';
 import '../package/model_properties.dart';
 import '../package/models.dart';
+import '../publisher/backend.dart';
 import '../publisher/models.dart';
+import '../shared/env_config.dart';
 
 import 'configuration.dart';
 import 'datastore.dart';
-import 'email.dart' show looksLikeEmail;
+import 'email.dart' show isValidEmail, looksLikeEmail;
 import 'storage.dart';
 import 'urls.dart' as urls;
 import 'utils.dart' show canonicalizeVersion, ByteArrayEqualsExt;
@@ -91,6 +92,7 @@ class IntegrityChecker {
       yield* _checkLikes();
       yield* _checkModeratedPackages();
       yield* _checkAuditLogs();
+      yield* _checkModerationCases();
       yield* _reportPubspecVersionIssues();
       // TODO: report unmapped properties
     } finally {
@@ -851,6 +853,50 @@ class IntegrityChecker {
   Future<bool> _packageMissing(String packageName) async =>
       !(await _packageExists(packageName));
 
+  Stream<String> _checkModerationCases() async* {
+    _logger.info('Scanning ModerationCases...');
+    yield* _queryWithPool<ModerationCase>(_checkModerationCase);
+  }
+
+  Stream<String> _checkModerationCase(ModerationCase mc) async* {
+    if (!isValidEmail(mc.reporterEmail)) {
+      yield 'ModerationCase "${mc.caseId}" has invalid `reporterEmail`.';
+    }
+    if (mc.resolved != null && mc.status == ModerationStatus.pending) {
+      yield 'ModerationCase "${mc.caseId}" is resolved but `status` is "pending".';
+    }
+    if (mc.status != ModerationStatus.pending && mc.resolved == null) {
+      yield 'ModerationCase "${mc.caseId}" has non-pending `status` but `resolved` is null.';
+    }
+    final subject = ModerationSubject.tryParse(mc.subject);
+    if (subject == null) {
+      yield 'ModerationCase "${mc.caseId}" has invalid `subject`.';
+    }
+    if (mc.url != null && Uri.tryParse(mc.url!) == null) {
+      yield 'ModerationCase "${mc.caseId}" has invalid `url`.';
+    }
+    for (final entry in mc.getActionLog().entries) {
+      if (entry.timestamp.isBefore(mc.opened)) {
+        yield 'ModerationCase "${mc.caseId}" has action logged before it was opened.';
+      }
+      if (mc.resolved != null && entry.timestamp.isAfter(mc.resolved!)) {
+        yield 'ModerationCase "${mc.caseId}" has action logged after it was resolved.';
+      }
+      if (ModerationSubject.tryParse(entry.subject) == null) {
+        yield 'ModerationCase "${mc.caseId}" has action logged with invalid `subject`.';
+      }
+    }
+    // TODO: verify fields once the other PR lands
+
+    if (mc.appealedCaseId != null) {
+      final appealed = await dbService.lookupOrNull<ModerationCase>(
+          dbService.emptyKey.append(ModerationCase, id: mc.appealedCaseId!));
+      if (appealed == null) {
+        yield 'ModerationCase "${mc.caseId}" references an appealed case that does not exists.';
+      }
+    }
+  }
+
   Stream<String> _reportPubspecVersionIssues() async* {
     for (final String package in _badVersionInPubspec.keys) {
       final values = _badVersionInPubspec[package]!.toList()..sort();

app/test/admin/moderate_package_test.dart

@@ -354,7 +354,11 @@ void main() {
 
     testWithProfile('status already closed', fn: () async {
       final mc = await _report('oxygen');
-      await dbService.commit(inserts: [mc..status = ModerationStatus.noAction]);
+      await dbService.commit(inserts: [
+        mc
+          ..resolved = clock.now()
+          ..status = ModerationStatus.noAction
+      ]);
 
       await expectApiException(
         _moderate('oxygen', state: true, caseId: mc.caseId),

app/test/admin/moderate_publisher_test.dart

@@ -192,7 +192,11 @@ void main() {
 
     testWithProfile('status already closed', fn: () async {
       final mc = await _report('example.com');
-      await dbService.commit(inserts: [mc..status = ModerationStatus.noAction]);
+      await dbService.commit(inserts: [
+        mc
+          ..resolved = clock.now()
+          ..status = ModerationStatus.noAction
+      ]);
 
       await expectApiException(
         _moderate('example.com', state: true, caseId: mc.caseId),

app/test/admin/moderation_case_test.dart

@@ -45,7 +45,6 @@ void main() {
           'subject': 'package:oxygen',
           'kind': 'notification',
           'source': 'external-notification',
-          'status': 'no-action',
         }),
       );
       expect(rs.output, {
@@ -55,7 +54,7 @@ void main() {
         'opened': isNotEmpty,
         'resolved': null,
         'source': 'external-notification',
-        'status': 'no-action',
+        'status': 'pending',
         'subject': 'package:oxygen',
         'isSubjectOwner': false,
         'url': null,
@@ -101,7 +100,6 @@ void main() {
       final values = {
         'kind': 'kind',
         'source': 'source',
-        'status': 'status',
         'reporter-email': 'non-email',
       };
 
@@ -135,6 +133,14 @@ void main() {
     testWithProfile('success', fn: () async {
       final caseId = await _create();
       final api = createPubApiClient(authToken: siteAdminToken);
+      // close case first to change status
+      await api.adminInvokeAction(
+        'resolve-moderation-case',
+        AdminInvokeActionArguments(arguments: {
+          'case': caseId,
+        }),
+      );
+
       final rs = await api.adminInvokeAction(
         'update-moderation-case',
         AdminInvokeActionArguments(arguments: {
@@ -150,7 +156,7 @@ void main() {
         'reporterEmail': 'user@pub.dev',
         'kind': 'notification',
         'opened': isNotEmpty,
-        'resolved': null,
+        'resolved': isNotEmpty,
         'source': 'external-notification',
         'status': 'no-action',
         'subject': 'package:oxygen',

app/test/frontend/handlers/report_test.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:_pub_shared/data/account_api.dart';
+import 'package:clock/clock.dart';
 import 'package:pub_dev/admin/models.dart';
 import 'package:pub_dev/fake/backend/fake_auth_provider.dart';
 import 'package:pub_dev/fake/backend/fake_email_sender.dart';
@@ -322,6 +323,9 @@ void main() {
       if (logSubject != null) {
         mc.addActionLogEntry(logSubject, ModerationAction.apply, null);
       }
+      if (mc.status != ModerationStatus.pending) {
+        mc.resolved = clock.now();
+      }
       await dbService.commit(inserts: [mc]);
     }