From 347b44aff3fe85e8c53e247095f5c7ad45c995b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Istv=C3=A1n=20So=C3=B3s?= <isoos@users.noreply.github.com>
Date: Wed, 13 Mar 2024 11:32:48 +0100
Subject: [PATCH] Integrity checks for isModerated/moderatedAt fields. (#7554)

---
 app/lib/account/models.dart   |  3 +++
 app/lib/shared/integrity.dart | 51 ++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/app/lib/account/models.dart b/app/lib/account/models.dart
index bc70c1d90c..c7a60c11b5 100644
--- a/app/lib/account/models.dart
+++ b/app/lib/account/models.dart
@@ -69,6 +69,9 @@ class User extends db.ExpandoModel<String> {
     isDeleted = false;
     isModerated = false;
   }
+
+  late final isVisible = !isBlocked && !(isModerated ?? false);
+  late final isNotVisible = !isVisible;
 }
 
 /// Maps Oauth user_id to User.id
diff --git a/app/lib/shared/integrity.dart b/app/lib/shared/integrity.dart
index 1d35bc51cb..0b5f3e1b52 100644
--- a/app/lib/shared/integrity.dart
+++ b/app/lib/shared/integrity.dart
@@ -140,9 +140,16 @@ class IntegrityChecker {
         yield 'User "${user.userId}" is recently created, but has no `oauthUserId`.';
       }
 
-      if (user.isBlocked) {
+      if (user.isNotVisible) {
         _blockedUsers.add(user.userId);
       }
+
+      yield* _checkModeratedFlags(
+        kind: 'User',
+        id: user.userId,
+        isModerated: user.isModerated,
+        moderatedAt: user.moderatedAt,
+      );
     });
   }
 
@@ -197,6 +204,13 @@ class IntegrityChecker {
     _logger.info('Scanning Publishers...');
     yield* _queryWithPool<Publisher>((p) async* {
       publisherAttributes.addPublisher(p);
+
+      yield* _checkModeratedFlags(
+        kind: 'Publisher',
+        id: p.publisherId,
+        isModerated: p.isModerated,
+        moderatedAt: p.moderatedAt,
+      );
     });
   }
 
@@ -286,13 +300,13 @@ class IntegrityChecker {
     // empty uploaders
     if (p.uploaders == null || p.uploaders!.isEmpty) {
       // no publisher
-      if (p.publisherId == null && !p.isBlocked && !p.isDiscontinued) {
+      if (p.publisherId == null && p.isVisible && !p.isDiscontinued) {
         yield 'Package "${p.name}" has no uploaders, must be marked discontinued.';
       }
 
       if (p.publisherId != null &&
           publisherAttributes.isAbandoned(p.publisherId!) &&
-          !p.isBlocked &&
+          p.isVisible &&
           !p.isDiscontinued) {
         yield 'Package "${p.name}" has an abandoned publisher, must be marked discontinued.';
       }
@@ -399,6 +413,12 @@ class IntegrityChecker {
     } else if (!versionKeys.contains(p.latestPreviewVersionKey)) {
       yield 'Package "${p.name}" has missing `latestPreviewVersionKey`: "${p.latestPreviewVersionKey!.id}".';
     }
+    yield* _checkModeratedFlags(
+      kind: 'Package',
+      id: p.name!,
+      isModerated: p.isModerated,
+      moderatedAt: p.moderatedAt,
+    );
 
     // Checking if PackageVersionInfo is referenced by a PackageVersion entity.
     final pviQuery = _db.query<PackageVersionInfo>()
@@ -536,6 +556,13 @@ class IntegrityChecker {
       yield* Stream.fromIterable(tarballItems);
     }
 
+    yield* _checkModeratedFlags(
+      kind: 'PackageVersion',
+      id: pv.qualifiedVersionKey.toString(),
+      isModerated: pv.isModerated,
+      moderatedAt: pv.moderatedAt,
+    );
+
     // TODO: remove null check after the backfill should have filled the property.
     final sha256Hash = pv.sha256;
     if (sha256Hash == null || sha256Hash.length != 32) {
@@ -879,3 +906,21 @@ class _PublisherAttributes {
     _memberCount.clear();
   }
 }
+
+/// Check that `isModerated` and `moderatedAt` are consistent.
+Stream<String> _checkModeratedFlags({
+  required String kind,
+  required String id,
+  required bool? isModerated,
+  required DateTime? moderatedAt,
+}) async* {
+  if (isModerated == null) {
+    yield '$kind "$id" has an `isModerated` property which is null.';
+  }
+  if ((isModerated ?? false) && moderatedAt == null) {
+    yield '$kind "$id" has `isModerated = true` but `moderatedAt` is null.';
+  }
+  if (!(isModerated ?? false) && moderatedAt != null) {
+    yield '$kind "$id" has `isModerated = false` but `moderatedAt` is not null.';
+  }
+}