dart-lang · isoos · Apr 30, 2024 · Apr 29, 2024
diff --git a/app/lib/admin/models.dart b/app/lib/admin/models.dart
@@ -42,8 +42,8 @@ class ModerationCase extends db.ExpandoModel<String> {
   /// - `package:<package>`
   /// - `package-version:<package>/<version>`
   /// - `publisher:<publisherId>`
-  @db.StringProperty()
-  String? subject;
+  @db.StringProperty(required: true)
+  late String subject;
 
   /// The `caseId` of the appeal (or null).
   @db.StringProperty()
@@ -74,7 +74,7 @@ class ModerationCase extends db.ExpandoModel<String> {
     required this.source,
     required this.kind,
     required this.status,
-    this.subject,
+    required this.subject,
   }) {
     id = caseId;
     opened = clock.now().toUtc();
@@ -173,7 +173,6 @@ class ModerationSubject {
 }
 
 class ModerationSubjectKind {
-  static const unspecified = 'unspecified';
   static const package = 'package';
   static const packageVersion = 'package-version';
   static const publisher = 'publisher';

diff --git a/app/lib/frontend/handlers/report.dart b/app/lib/frontend/handlers/report.dart
@@ -29,14 +29,11 @@ Future<shelf.Response> reportPageHandler(shelf.Request request) async {
   }
 
   final subjectParam = request.requestedUri.queryParameters['subject'];
-  ModerationSubject? subject;
-  if (subjectParam != null) {
-    subject = ModerationSubject.tryParse(subjectParam);
-    if (subject == null) {
-      throw InvalidInputException('Invalid "subject" parameter.');
-    }
-    await _verifySubject(subject);
-  }
+
+  InvalidInputException.checkNotNull(subjectParam, 'subject');
+  final subject = ModerationSubject.tryParse(subjectParam!);
+  InvalidInputException.check(subject != null, 'Invalid "subject" parameter.');
+  await _verifySubject(subject!);
 
   return htmlResponse(
     renderReportPage(
@@ -94,12 +91,10 @@ Future<String> processReportPageHandler(
     InvalidInputException.checkNull(form.email, 'email');
   }
 
-  ModerationSubject? subject;
-  if (form.subject != null) {
-    subject = ModerationSubject.tryParse(form.subject!);
-    InvalidInputException.check(subject != null, 'Invalid subject.');
-    await _verifySubject(subject);
-  }
+  InvalidInputException.checkNotNull(form.subject, 'subject');
+  final subject = ModerationSubject.tryParse(form.subject!);
+  InvalidInputException.check(subject != null, 'Invalid subject.');
+  await _verifySubject(subject!);
 
   InvalidInputException.checkStringLength(
     form.message,
@@ -117,14 +112,14 @@ Future<String> processReportPageHandler(
       source: ModerationDetectedBy.externalNotification,
       kind: ModerationKind.notification,
       status: ModerationStatus.pending,
-      subject: subject?.fqn,
+      subject: subject.fqn,
     );
     tx.insert(mc);
   });
 
   final bodyText = <String>[
     'New report recieved on ${now.toIso8601String()}: $caseId',
-    if (subject != null) 'Subject: ${subject.fqn}',
+    'Subject: ${subject.fqn}',
     'Message:\n${form.message}',
   ].join('\n\n');
 

diff --git a/app/test/frontend/handlers/report_test.dart b/app/test/frontend/handlers/report_test.dart
@@ -20,7 +20,7 @@ void main() {
     testWithProfile('page does not require authentication', fn: () async {
       await expectHtmlResponse(
         await issueGet(
-          '/report',
+          '/report?subject=package:oxygen',
           headers: {'cookie': '$experimentalCookieName=report'},
         ),
         present: [
@@ -34,14 +34,30 @@ void main() {
       final cookies = await acquireSessionCookies('user@pub.dev');
       await expectHtmlResponse(
         await issueGet(
-          '/report',
+          '/report?subject=package:oxygen',
           headers: {'cookie': '$experimentalCookieName=report; $cookies'},
         ),
         present: ['Please describe the issue you want to report:'],
         absent: ['Contact information'],
       );
     });
 
+    testWithProfile('page with missing subject', fn: () async {
+      await expectHtmlResponse(
+        await issueGet(
+          '/report',
+          headers: {'cookie': '$experimentalCookieName=report'},
+        ),
+        present: [
+          '&quot;subject&quot; cannot be `null`',
+        ],
+        absent: [
+          'Please describe the issue you want to report:',
+        ],
+        status: 400,
+      );
+    });
+
     testWithProfile('page with bad subject', fn: () async {
       await expectHtmlResponse(
         await issueGet(
@@ -114,6 +130,100 @@ void main() {
       });
     });
 
+    testWithProfile('subject missing', fn: () async {
+      await withHttpPubApiClient(
+        experimental: {'report'},
+        fn: (client) async {
+          await expectApiException(
+            client.postReport(ReportForm(
+              email: 'user@pub.dev',
+              message: 'Huston, we have a problem.',
+            )),
+            status: 400,
+            code: 'InvalidInput',
+            message: '\"subject\" cannot be `null`',
+          );
+          expect(fakeEmailSender.sentMessages, isEmpty);
+        },
+      );
+    });
+
+    testWithProfile('subject is invalid', fn: () async {
+      await withHttpPubApiClient(
+        experimental: {'report'},
+        fn: (client) async {
+          await expectApiException(
+            client.postReport(ReportForm(
+              email: 'user@pub.dev',
+              subject: 'x',
+              message: 'Huston, we have a problem.',
+            )),
+            status: 400,
+            code: 'InvalidInput',
+            message: 'Invalid subject.',
+          );
+          expect(fakeEmailSender.sentMessages, isEmpty);
+        },
+      );
+    });
+
+    testWithProfile('package missing', fn: () async {
+      await withHttpPubApiClient(
+        experimental: {'report'},
+        fn: (client) async {
+          await expectApiException(
+            client.postReport(ReportForm(
+              email: 'user@pub.dev',
+              subject: 'package:x',
+              message: 'Huston, we have a problem.',
+            )),
+            status: 404,
+            code: 'NotFound',
+            message: 'Package \"x\" does not exist.',
+          );
+          expect(fakeEmailSender.sentMessages, isEmpty);
+        },
+      );
+    });
+
+    testWithProfile('version missing', fn: () async {
+      await withHttpPubApiClient(
+        experimental: {'report'},
+        fn: (client) async {
+          await expectApiException(
+            client.postReport(ReportForm(
+              email: 'user@pub.dev',
+              subject: 'package-version:oxygen/4.0.0',
+              message: 'Huston, we have a problem.',
+            )),
+            status: 404,
+            code: 'NotFound',
+            message: 'Package version \"oxygen/4.0.0\" does not exist.',
+          );
+          expect(fakeEmailSender.sentMessages, isEmpty);
+        },
+      );
+    });
+
+    testWithProfile('publisher missing', fn: () async {
+      await withHttpPubApiClient(
+        experimental: {'report'},
+        fn: (client) async {
+          await expectApiException(
+            client.postReport(ReportForm(
+              email: 'user@pub.dev',
+              subject: 'publisher:unknown-domain.com',
+              message: 'Huston, we have a problem.',
+            )),
+            status: 404,
+            code: 'NotFound',
+            message: 'Publisher \"unknown-domain.com\" does not exist.',
+          );
+          expect(fakeEmailSender.sentMessages, isEmpty);
+        },
+      );
+    });
+
     testWithProfile('too short message', fn: () async {
       await withFakeAuthRequestContext('user@pub.dev', () async {
         final sessionId = requestContext.sessionData?.sessionId;
@@ -125,6 +235,7 @@ void main() {
           fn: (client) async {
             await expectApiException(
               client.postReport(ReportForm(
+                subject: 'package:oxygen',
                 message: 'Problem.',
               )),
               status: 400,
@@ -143,6 +254,7 @@ void main() {
         fn: (client) async {
           final msg = await client.postReport(ReportForm(
             email: 'user2@pub.dev',
+            subject: 'package:oxygen',
             message: 'Huston, we have a problem.',
           ));
 
@@ -154,7 +266,7 @@ void main() {
           expect(email.ccRecipients.single.email, 'user2@pub.dev');
 
           final mc = await dbService.query<ModerationCase>().run().single;
-          expect(mc.subject, isNull);
+          expect(mc.subject, 'package:oxygen');
           expect(mc.reporterEmail, 'user2@pub.dev');
         },
       );

diff --git a/pkg/pub_integration/test/report_test.dart b/pkg/pub_integration/test/report_test.dart
@@ -49,7 +49,7 @@ void main() {
           await page.gotoOrigin('/experimental?report=1');
           await Future.delayed(Duration(seconds: 1));
 
-          await page.gotoOrigin('/report');
+          await page.gotoOrigin('/report?subject=package:oxygen');
           await page.waitFocusAndType('#report-email', 'user@pub.dev');
           await page.waitFocusAndType(
               '#report-message', 'Huston, we have a problem.');