From 30cc860f1c198602b76b65aba0aac73c995c28ef Mon Sep 17 00:00:00 2001 From: Sven Aas Date: Thu, 21 Dec 2023 12:37:11 -0500 Subject: [PATCH] chore: Remove isActive from User #4338 --- admin-client/src/components/UserTable.svelte | 8 --- admin-client/src/pages/user/Show.svelte | 5 -- api/models/user.js | 5 -- api/serializers/user.js | 1 - api/services/Webhooks.js | 3 - ...231221154733-remove-is-active-from-user.js | 10 +++ test/api/requests/auth.test.js | 8 +-- test/api/unit/services/Webhooks.test.js | 70 +------------------ 8 files changed, 13 insertions(+), 97 deletions(-) create mode 100644 migrations/20231221154733-remove-is-active-from-user.js diff --git a/admin-client/src/components/UserTable.svelte b/admin-client/src/components/UserTable.svelte index ec2043c3d..55361ddcb 100644 --- a/admin-client/src/components/UserTable.svelte +++ b/admin-client/src/components/UserTable.svelte @@ -4,8 +4,6 @@ export let users = []; export let borderless = false; - - const stateColor = (isActive) => (isActive ? 'bg-mint' : 'bg-gray-30'); @@ -17,7 +15,6 @@ Created Last Signed In Last Pushed - Status {user.id} @@ -27,10 +24,5 @@ {formatDateTime(user.createdAt)} {formatDateTime(user.signedInAt)} {formatDateTime(user.pushedAt)} - - - {user.isActive ? 'Active' : 'Inactive'} - - \ No newline at end of file diff --git a/admin-client/src/pages/user/Show.svelte b/admin-client/src/pages/user/Show.svelte index 2212b6eea..fbdec5305 100644 --- a/admin-client/src/pages/user/Show.svelte +++ b/admin-client/src/pages/user/Show.svelte @@ -77,11 +77,6 @@
- - - {user.isActive ? 'Active' : 'Inactive'} - -
diff --git a/api/models/user.js b/api/models/user.js index 0bcc9dfce..c1c1203e9 100644 --- a/api/models/user.js +++ b/api/models/user.js @@ -129,11 +129,6 @@ const attributes = DataTypes => ({ unique: true, allowNull: false, }, - isActive: { - type: DataTypes.BOOLEAN, - defaultValue: true, - allowNull: false, - }, pushedAt: { type: DataTypes.DATE, }, diff --git a/api/serializers/user.js b/api/serializers/user.js index 5dff5f1fc..8f67feb87 100644 --- a/api/serializers/user.js +++ b/api/serializers/user.js @@ -22,7 +22,6 @@ const adminAttributes = { updatedAt: 'date', signedInAt: 'date', pushedAt: 'date', - isActive: '', adminEmail: '', deletedAt: 'date', }; diff --git a/api/services/Webhooks.js b/api/services/Webhooks.js index f84b00dd4..b925fc434 100644 --- a/api/services/Webhooks.js +++ b/api/services/Webhooks.js @@ -51,9 +51,6 @@ const organizationWebhookRequest = async (payload) => { } if (user) { - if (isActive !== user.isActive) { - await user.update({ isActive }); - } EventCreator.audit(Event.labels.FEDERALIST_USERS_MEMBERSHIP, user, action); } }; diff --git a/migrations/20231221154733-remove-is-active-from-user.js b/migrations/20231221154733-remove-is-active-from-user.js new file mode 100644 index 000000000..a19edba39 --- /dev/null +++ b/migrations/20231221154733-remove-is-active-from-user.js @@ -0,0 +1,10 @@ +const TABLE = 'user'; + +exports.up = async db => { + await db.removeColumn(TABLE, 'isActive'); +}; + +exports.down = async db => { + await db.addColumn(TABLE, 'isActive', { type: 'boolean', notNull: true, defaultValue: true }); + await db.runSql('update "user" set "isActive" = TRUE'); +}; diff --git a/test/api/requests/auth.test.js b/test/api/requests/auth.test.js index c0afaaa1f..848a5f72b 100644 --- a/test/api/requests/auth.test.js +++ b/test/api/requests/auth.test.js @@ -117,11 +117,10 @@ describe('Authentication requests', () => { let user; let userCount; const oauthState = 'state-123abc'; - factory.user({ isActive: true }) + factory.user() .then((model) => { user = model; githubAPINocks.githubAuth(user.username, [{ id: 123456 }]); - expect(user.isActive).to.be.true; return User.count(); }) .then((count) => { @@ -135,11 +134,6 @@ describe('Authentication requests', () => { .then(() => User.count()) .then((count) => { expect(count).to.equal(userCount); - return user.reload(); - }) - .then((model) => { - user = model; - expect(user.isActive).to.be.true; done(); }) .catch(done); diff --git a/test/api/unit/services/Webhooks.test.js b/test/api/unit/services/Webhooks.test.js index 7b713595f..e12a52169 100644 --- a/test/api/unit/services/Webhooks.test.js +++ b/test/api/unit/services/Webhooks.test.js @@ -434,28 +434,6 @@ describe('Webhooks Service', () => { }); describe('organizationWebhookRequest', () => { - it('should set a user to inActive if removed from federalist-users', (done) => { - let user; - let payload; - - factory.user({ isActive: true }) - .then((model) => { - user = model; - expect(user.isActive).to.be.true; - payload = organizationWebhookPayload('member_removed', user.username); - - - return Webhooks.organizationWebhookRequest(payload); - }) - .then(() => user.reload()) - .then((model) => { - user = model; - expect(user.isActive).to.be.false; - expect(auditStub.args[0][0]).to.equal(Event.labels.FEDERALIST_USERS_MEMBERSHIP); - expect(auditStub.args[0][1].id).to.eql(user.id); - done(); - }); - }); it('should create a new user added to federalist-users', (done) => { const username = 'added_member'; User.findOne({ where: { username } }) @@ -468,36 +446,12 @@ describe('Webhooks Service', () => { }) .then(() => User.findOne({ where: { username } })) .then((user) => { - expect(user.isActive).to.be.true; expect(auditStub.args[0][0]).to.equal(Event.labels.FEDERALIST_USERS_MEMBERSHIP); expect(auditStub.args[0][1].id).to.eql(user.id); done(); }); }); - it('should set an existing user to Active if added to federalist-users', (done) => { - let user; - - factory.user({ isActive: false}) - .then((model) => { - user = model; - expect(user.isActive).to.be.false; - const payload = organizationWebhookPayload('member_added', user.username); - - - return Webhooks.organizationWebhookRequest(payload); - }) - .then(() => user.reload()) - .then((model) => { - user = model; - expect(user.isActive).to.be.true; - expect(auditStub.args[0][0]).to.equal(Event.labels.FEDERALIST_USERS_MEMBERSHIP); - expect(auditStub.args[0][1].id).to.eql(user.id); - done(); - }) - .catch(done); - }); - it('should do nothing if org webhook for non added/removed_member', async () => { const user = User.build({ username: 'rando-user' }) const payload = organizationWebhookPayload('member_invited', user.username); @@ -529,7 +483,7 @@ describe('Webhooks Service', () => { }); it('should do nothing if not federalist-org webhook', (done) => { - factory.user({ isActive: true }) + factory.user() .then((user) => { const payload = organizationWebhookPayload('member_removed', user.username, 'not-federalist-users'); @@ -542,9 +496,8 @@ describe('Webhooks Service', () => { }); it('should do nothing if action not added, removed nor invited', (done) => { - factory.user({ isActive: true }) + factory.user() .then((user) => { - expect(user.isActive).to.be.true; const payload = organizationWebhookPayload('member_ignored_action', user.username); @@ -555,24 +508,5 @@ describe('Webhooks Service', () => { done(); }); }); - - it('should do nothing if the user has a uaa identity', async () => { - const isActive = false; - const user = await factory.user({ isActive }); - await user.createUAAIdentity({ - uaaId: `${user.username}-placeholder-id`, - email: `${user.username}@example.com`, - userName: `${user.username}@example.com`, - origin: 'example.com', - }); - - const payload = organizationWebhookPayload('member_added', user.username); - - await Webhooks.organizationWebhookRequest(payload); - - await user.reload(); - - expect(user.isActive).to.be.false; - }); }); });