Migrate delete-publisher to action framework (dart-lang#7360)

app/lib/admin/actions/actions.dart

@@ -3,6 +3,8 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:pub_dev/admin/actions/create_publisher.dart';
+import 'package:pub_dev/admin/actions/delete_publisher.dart';
+
 import 'package:pub_dev/admin/actions/merge_moderated_package_into_existing.dart';
 import 'package:pub_dev/admin/actions/publisher_block.dart';
 import 'package:pub_dev/admin/actions/publisher_members_list.dart';
@@ -67,6 +69,7 @@ final class AdminAction {
 
   static List<AdminAction> actions = [
     createPublisher,
+    deletePublisher,
     mergeModeratedPackageIntoExisting,
     publisherBlock,
     publisherMembersList,

app/lib/admin/actions/delete_publisher.dart

@@ -0,0 +1,60 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:pub_dev/admin/actions/actions.dart';
+import 'package:pub_dev/package/models.dart';
+import 'package:pub_dev/publisher/models.dart';
+import 'package:pub_dev/shared/datastore.dart';
+
+final deletePublisher = AdminAction(
+    name: 'delete-publisher',
+    options: {
+      'publisher': 'name of publisher to delete',
+    },
+    summary: 'Deletes publisher <publisher>.',
+    description: '''
+Deletes publisher <publisher>.
+
+The publisher must have no packages. If not the operation will fail.
+
+All member-info will be lost.
+
+The publisher can be regenerated later (no tombstoning).
+''',
+    invoke: (args) async {
+      final publisherId = args['publisher'];
+      if (publisherId == null) {
+        throw InvalidInputException('Missing `publisher` argument');
+      }
+
+      final packagesQuery = dbService.query<Package>()
+        ..filter('publisherId =', publisherId);
+      final packages = await packagesQuery.run().toList();
+      if (packages.isNotEmpty) {
+        throw NotAcceptableException(
+            'Publisher "$publisherId" cannot be deleted, as it has package(s): '
+            '${packages.map((e) => e.name!).join(', ')}.');
+      }
+
+      int? memberCount;
+      await withRetryTransaction(dbService, (tx) async {
+        final key = dbService.emptyKey.append(Publisher, id: publisherId);
+        final publisher = await tx.lookupOrNull<Publisher>(key);
+        final membersQuery = tx.query<PublisherMember>(key);
+        final members = await membersQuery.run().toList();
+        memberCount = members.length;
+        if (publisher != null) {
+          tx.delete(key);
+        }
+        for (final m in members) {
+          tx.delete(m.key);
+        }
+      });
+
+      return {
+        'message': 'Publisher and all members deleted.',
+        'publisherId': publisherId,
+        'members-count': memberCount,
+      };
+    });

app/lib/admin/backend.dart

@@ -31,7 +31,6 @@ import '../shared/exceptions.dart';
 import '../tool/utils/dart_sdk_version.dart';
 import 'actions/actions.dart' show AdminAction;
 import 'tools/delete_all_staging.dart';
-import 'tools/delete_publisher.dart';
 import 'tools/list_package_blocked.dart';
 import 'tools/list_tools.dart';
 import 'tools/notify_service.dart';
@@ -58,7 +57,6 @@ typedef Tool = Future<String> Function(List<String> args);
 
 final Map<String, Tool> availableTools = {
   'delete-all-staging': executeDeleteAllStaging,
-  'delete-publisher': executeDeletePublisher,
   'list-package-blocked': executeListPackageBlocked,
   'notify-service': executeNotifyService,
   'package-discontinued': executeSetPackageDiscontinued,

app/test/admin/api_actions_test.dart

@@ -2,8 +2,6 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:convert';
-
 import 'package:_pub_shared/data/admin_api.dart';
 import 'package:clock/clock.dart';
 import 'package:pub_dev/account/backend.dart';
@@ -53,7 +51,7 @@ void main() {
       'create-publisher',
       AdminInvokeActionArguments(arguments: {
         'publisher': 'other.com',
-        'member-email': '[email protected]'
+        'member-email': '[email protected]',
       }),
     );
     expect(rs1.output, {
@@ -79,15 +77,13 @@ void main() {
     });
     final p1 = await publisherBackend.getPublisher('other.com');
     expect(p1, isNotNull);
-    // TODO(sigurdm): Migrate delete-publisher to be an Action.
-    final rs2 = await client.adminExecuteTool(
-      'delete-publisher',
-      Uri(pathSegments: [
-        '--publisher',
-        'other.com',
-      ]).toString(),
-    );
-    expect(utf8.decode(rs2), 'Publisher and 1 member(s) deleted.');
+    final rs2 = await client.adminInvokeAction('delete-publisher',
+        AdminInvokeActionArguments(arguments: {'publisher': 'other.com'}));
+    expect(rs2.output, {
+      'message': 'Publisher and all members deleted.',
+      'publisherId': 'other.com',
+      'members-count': 1,
+    });
     final p2 = await publisherBackend.getPublisher('other.com');
     expect(p2, isNull);
   });

app/test/admin/api_tool_test.dart

@@ -5,6 +5,8 @@
 import 'dart:convert';
 
 import 'package:_pub_shared/data/account_api.dart' as account_api;
+import 'package:_pub_shared/data/admin_api.dart';
+import 'package:api_builder/_client_utils.dart';
 import 'package:pub_dev/account/backend.dart';
 import 'package:pub_dev/account/consent_backend.dart';
 import 'package:pub_dev/account/models.dart';
@@ -141,16 +143,24 @@ void main() {
     testWithProfile('publisher has packages', fn: () async {
       final p1 = await publisherBackend.getPublisher('example.com');
       expect(p1, isNotNull);
-      final rs =
-          await createPubApiClient(authToken: siteAdminToken).adminExecuteTool(
-        'delete-publisher',
-        Uri(pathSegments: [
-          '--publisher',
-          'example.com',
-        ]).toString(),
-      );
-      expect(utf8.decode(rs),
-          'Publisher "example.com" cannot be deleted, as it has package(s): neon.');
+
+      await expectLater(
+          createPubApiClient(authToken: siteAdminToken).adminInvokeAction(
+            'delete-publisher',
+            AdminInvokeActionArguments(
+              arguments: {'publisher': 'example.com'},
+            ),
+          ),
+          throwsA(isA<RequestException>().having(
+            (e) => e.bodyAsJson()['error'],
+            '',
+            {
+              'code': 'NotAcceptable',
+              'message':
+                  'Publisher \"example.com\" cannot be deleted, as it has package(s): neon.'
+            },
+          )));
+
       final p2 = await publisherBackend.getPublisher('example.com');
       expect(p2, isNotNull);
     });

app/test/shared/test_services.dart

@@ -8,6 +8,7 @@ import 'package:fake_gcloud/mem_datastore.dart';
 import 'package:fake_gcloud/mem_storage.dart';
 import 'package:gcloud/db.dart';
 import 'package:gcloud/service_scope.dart';
+import 'package:meta/meta.dart';
 import 'package:pub_dev/account/models.dart';
 import 'package:pub_dev/fake/backend/fake_auth_provider.dart';
 import 'package:pub_dev/fake/backend/fake_email_sender.dart';
@@ -133,6 +134,7 @@ class FakeAppengineEnv {
 
 /// Registers test with [name] and runs it in pkg/fake_gcloud's scope, populated
 /// with [testProfile] data.
+@isTest
 void testWithProfile(
   String name, {
   TestProfile? testProfile,