Merge pull request #8994 from hicommonwealth/8990.jake.remove-bans-table

Eliminate Bans table.
hicommonwealth · Aug 26, 2024 · 4dd9ad6 · 4dd9ad6
2 parents 6d390cf + 820c42d
commit 4dd9ad6
Show file tree

Hide file tree

Showing 19 changed files with 105 additions and 112 deletions.
diff --git a/libs/model/src/models/address.ts b/libs/model/src/models/address.ts
@@ -54,6 +54,11 @@ export default (
       wallet_id: { type: Sequelize.STRING, allowNull: true },
       wallet_sso_source: { type: Sequelize.STRING, allowNull: true },
       block_info: { type: Sequelize.STRING, allowNull: true },
+      is_banned: {
+        type: Sequelize.BOOLEAN,
+        allowNull: false,
+        defaultValue: false,
+      },
       hex: {
         type: Sequelize.STRING(64),
         allowNull: true,

diff --git a/libs/model/src/models/associations.ts b/libs/model/src/models/associations.ts
@@ -60,7 +60,6 @@ export const buildAssociations = (db: DB) => {
     })
     .withMany(db.Notification)
     .withMany(db.Webhook)
-    .withMany(db.Ban)
     .withMany(db.CommunityBanner)
     .withMany(db.CommunityTags, {
       onDelete: 'CASCADE',

diff --git a/libs/model/src/models/ban.ts b/libs/model/src/models/ban.ts
diff --git a/libs/model/src/models/factories.ts b/libs/model/src/models/factories.ts
@@ -2,7 +2,6 @@ import Sequelize from 'sequelize';
 import type { Associable } from './types';
 
 import Address from './address';
-import Ban from './ban';
 import ChainNode from './chain_node';
 import Collaboration from './collaboration';
 import Comment from './comment';
@@ -50,7 +49,6 @@ import Webhook from './webhook';
 
 export const Factories = {
   Address,
-  Ban,
   ChainNode,
   Collaboration,
   Comment,
@@ -98,8 +96,8 @@ export const Factories = {
 };
 
 export type DB = {
-  [K in keyof typeof Factories]: ReturnType<typeof Factories[K]> &
-    Associable<ReturnType<typeof Factories[K]>>;
+  [K in keyof typeof Factories]: ReturnType<(typeof Factories)[K]> &
+    Associable<ReturnType<(typeof Factories)[K]>>;
 } & {
   sequelize: Sequelize.Sequelize;
   Sequelize: typeof Sequelize.Sequelize;

diff --git a/libs/model/src/models/index.ts b/libs/model/src/models/index.ts
@@ -44,7 +44,6 @@ export const buildDb = (sequelize: Sequelize): DB => {
 
 // TODO: avoid legacy exports to /packages/commonwealth/server (keep db models encapsulated behind DB)
 export * from './address';
-export * from './ban';
 export * from './chain_node';
 export * from './collaboration';
 export * from './comment';

diff --git a/libs/model/src/tester/seedDb.ts b/libs/model/src/tester/seedDb.ts
@@ -447,6 +447,7 @@ export const seedDb = async () => {
         role: 'admin' as Role,
         is_user_default: false,
         ghost_address: false,
+        is_banned: false,
       })),
     );
 

diff --git a/libs/schemas/src/entities/user.schemas.ts b/libs/schemas/src/entities/user.schemas.ts
@@ -62,6 +62,7 @@ export const Address = z.object({
   is_user_default: z.boolean().default(false),
   role: z.enum(Roles).default('member'),
   wallet_sso_source: z.nativeEnum(WalletSsoSource).nullish(),
+  is_banned: z.boolean().default(false),
   hex: z.string().max(64).nullish(),
 
   User: User.optional(),

diff --git a/packages/commonwealth/server/controllers/server_communities_methods/create_community.ts b/packages/commonwealth/server/controllers/server_communities_methods/create_community.ts
@@ -485,6 +485,7 @@ export async function __createCommunity(
       role: 'admin',
       last_active: new Date(),
       ghost_address: false,
+      is_banned: false,
     });
   }
 

diff --git a/packages/commonwealth/server/controllers/server_communities_methods/delete_community.ts b/packages/commonwealth/server/controllers/server_communities_methods/delete_community.ts
@@ -125,8 +125,6 @@ export async function __deleteCommunity(
             // @ts-expect-error StrictNullChecks
             this.models.DiscordBotConfig,
             // @ts-expect-error StrictNullChecks
-            this.models.Ban,
-            // @ts-expect-error StrictNullChecks
             this.models.Reaction,
             // @ts-expect-error StrictNullChecks
             this.models.Comment,

diff --git a/packages/commonwealth/server/controllers/server_communities_methods/update_community_id.ts b/packages/commonwealth/server/controllers/server_communities_methods/update_community_id.ts
@@ -78,8 +78,6 @@ export async function __updateCommunityId(
       // @ts-expect-error StrictNullChecks
       this.models.Address,
       // @ts-expect-error StrictNullChecks
-      this.models.Ban,
-      // @ts-expect-error StrictNullChecks
       this.models.Comment,
       // @ts-expect-error StrictNullChecks
       this.models.CommunityBanner,

diff --git a/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js b/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js
@@ -0,0 +1,74 @@
+'use strict';
+
+/** @type {import('sequelize-cli').Migration} */
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    return queryInterface.sequelize.transaction(async (transaction) => {
+      await queryInterface.addColumn(
+        'Addresses',
+        'is_banned',
+        {
+          type: Sequelize.BOOLEAN,
+          allowNull: false,
+          defaultValue: false,
+        },
+        { transaction },
+      );
+      await queryInterface.sequelize.query(
+        `
+        UPDATE "Addresses" AS a
+        SET is_banned = true
+        FROM "Bans" b
+        WHERE a.address = b.address AND a.community_id = b.community_id;
+       `,
+        {
+          transaction,
+        },
+      );
+      await queryInterface.dropTable('Bans', { transaction });
+    });
+  },
+
+  async down(queryInterface, Sequelize) {
+    return queryInterface.sequelize.transaction(async (transaction) => {
+      await queryInterface.createTable(
+        'Bans',
+        {
+          id: {
+            type: Sequelize.INTEGER,
+            autoIncrement: true,
+            primaryKey: true,
+          },
+          address: { type: Sequelize.STRING, allowNull: false },
+          community_id: {
+            type: Sequelize.STRING,
+            allowNull: false,
+            references: { model: 'Communities', key: 'id' },
+          },
+          created_at: { type: Sequelize.DATE, allowNull: false },
+          updated_at: { type: Sequelize.DATE, allowNull: false },
+        },
+        {
+          timestamps: true,
+          underscored: true,
+          indexes: [{ fields: ['community_id'] }],
+          transaction,
+        },
+      );
+      await queryInterface.sequelize.query(
+        `
+        INSERT INTO "Bans" (address, community_id, created_at, updated_at)
+        SELECT address, community_id, NOW(), NOW()
+        FROM "Addresses"
+        WHERE is_banned = true;
+       `,
+        {
+          transaction,
+        },
+      );
+      await queryInterface.removeColumn('Addresses', 'is_banned', {
+        transaction,
+      });
+    });
+  },
+};
diff --git a/packages/commonwealth/server/passport/magic.ts b/packages/commonwealth/server/passport/magic.ts
@@ -73,6 +73,7 @@ async function createMagicAddressInstances(
         role: 'member',
         is_user_default: false,
         ghost_address: false,
+        is_banned: false,
       },
       transaction: t,
     });

diff --git a/packages/commonwealth/server/routes/banAddress.ts b/packages/commonwealth/server/routes/banAddress.ts
@@ -1,5 +1,5 @@
 import { AppError } from '@hicommonwealth/core';
-import type { BanAttributes, BanInstance, DB } from '@hicommonwealth/model';
+import type { DB } from '@hicommonwealth/model';
 import type { TypedRequestBody, TypedResponse } from '../types';
 import { success } from '../types';
 import { validateOwner } from '../util/validateOwner';
@@ -8,13 +8,15 @@ enum BanAddressErrors {
   NoAddress = 'Must supply an address',
   NoPermission = 'You do not have permission to ban an address',
   AlreadyExists = 'Ban for this address already exists',
+  NotFound = 'Address not found',
 }
 
-type BanAddressReq = Omit<BanInstance, 'id'> & {
+type BanAddressReq = {
+  community_id: string;
   address: string;
 };
 
-type BanAddressResp = BanAttributes;
+type BanAddressResp = {};
 
 const banAddress = async (
   models: DB,
@@ -40,25 +42,22 @@ const banAddress = async (
     throw new AppError(BanAddressErrors.NoAddress);
   }
 
-  // find or create Ban
-  const [ban, created] = await models.Ban.findOrCreate({
+  const addressInstance = await models.Address.findOne({
     where: {
-      // @ts-expect-error StrictNullChecks
-      community_id: community.id,
-      address,
-    },
-    defaults: {
-      // @ts-expect-error StrictNullChecks
-      community_id: community.id,
+      community_id: community!.id,
       address,
     },
   });
-
-  if (!created) {
+  if (!addressInstance) {
+    throw new AppError(BanAddressErrors.NotFound);
+  }
+  if (addressInstance.is_banned) {
     throw new AppError(BanAddressErrors.AlreadyExists);
   }
+  addressInstance.is_banned = true;
+  await addressInstance.save();
 
-  return success(res, ban.toJSON());
+  return success(res, {});
 };
 
 export default banAddress;
diff --git a/packages/commonwealth/server/routes/createAddress.ts b/packages/commonwealth/server/routes/createAddress.ts
@@ -215,6 +215,7 @@ const createAddress = async (
           role: 'member',
           is_user_default: false,
           ghost_address: false,
+          is_banned: false,
         },
         { transaction },
       );

diff --git a/packages/commonwealth/server/routes/getBannedAddresses.ts b/packages/commonwealth/server/routes/getBannedAddresses.ts
diff --git a/packages/commonwealth/server/routes/linkExistingAddressToCommunity.ts b/packages/commonwealth/server/routes/linkExistingAddressToCommunity.ts
@@ -170,6 +170,7 @@ const linkExistingAddressToCommunity = async (
           role: 'member',
           is_user_default: false,
           ghost_address: false,
+          is_banned: false,
         },
         { transaction },
       );

diff --git a/packages/commonwealth/server/routing/router.ts b/packages/commonwealth/server/routing/router.ts
@@ -75,7 +75,6 @@ import type ViewCountCache from '../util/viewCountCache';
 
 import type { DB, GlobalActivityCache } from '@hicommonwealth/model';
 import banAddress from '../routes/banAddress';
-import getBannedAddresses from '../routes/getBannedAddresses';
 import setAddressWallet from '../routes/setAddressWallet';
 import type BanCache from '../util/banCheckCache';
 
@@ -932,14 +931,6 @@ function setupRouter(
     databaseValidationService.validateCommunity,
     banAddress.bind(this, models),
   );
-  registerRoute(
-    router,
-    'get',
-    '/getBannedAddresses',
-    passport.authenticate('jwt', { session: false }),
-    databaseValidationService.validateCommunity,
-    getBannedAddresses.bind(this, models),
-  );
 
   // Custom domain update route
   registerRoute(

diff --git a/packages/commonwealth/server/util/banCheckCache.ts b/packages/commonwealth/server/util/banCheckCache.ts
@@ -46,10 +46,12 @@ export default class BanCache extends JobRunner<CacheT> {
       return [false, BanErrors.Banned];
     }
 
-    const ban = await this._models.Ban.findOne({
+    const ban = await this._models.Address.findOne({
+      attributes: ['id'], // no data needed; only checking existence
       where: {
         community_id: communityId,
         address,
+        is_banned: true,
       },
     });
 

diff --git a/packages/commonwealth/server/util/sanitizeDeletedComment.ts b/packages/commonwealth/server/util/sanitizeDeletedComment.ts
@@ -16,6 +16,7 @@ export function sanitizeDeletedComment(
       role: 'member',
       is_user_default: false,
       ghost_address: false,
+      is_banned: false,
     },
     address_id: 0,
     canvas_hash: null,