Skip to content

Commit

Permalink
chore: Remove isActive from User #4338
Browse files Browse the repository at this point in the history
  • Loading branch information
svenaas committed Dec 22, 2023
1 parent 415dc9d commit 30cc860
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 97 deletions.
8 changes: 0 additions & 8 deletions admin-client/src/components/UserTable.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
export let users = [];
export let borderless = false;
const stateColor = (isActive) => (isActive ? 'bg-mint' : 'bg-gray-30');
</script>

<DataTable data={users} {borderless}>
Expand All @@ -17,7 +15,6 @@
<th>Created</th>
<th>Last Signed In</th>
<th>Last Pushed</th>
<th class="center">Status</th>
</tr>
<tr slot="item" let:item={user}>
<td><a href="/users/{user.id}">{user.id}</a></td>
Expand All @@ -27,10 +24,5 @@
<td>{formatDateTime(user.createdAt)}</td>
<td>{formatDateTime(user.signedInAt)}</td>
<td>{formatDateTime(user.pushedAt)}</td>
<td class="center">
<span class="usa-tag radius-pill {stateColor(user.isActive)}">
{user.isActive ? 'Active' : 'Inactive'}
</span>
</td>
</tr>
</DataTable>
5 changes: 0 additions & 5 deletions admin-client/src/pages/user/Show.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,6 @@
<div class="tablet:grid-col-fill padding-bottom-1">
<LabeledItem label="id" value={user.id} />
<LabeledItem label="username" value={user.username} />
<LabeledItem label="status">
<span class="usa-tag {user.isActive ? 'bg-mint' : 'bg-gray-30'}">
{user.isActive ? 'Active' : 'Inactive'}
</span>
</LabeledItem>

</div>
<div class="tablet:grid-col-auto padding-bottom-1">
Expand Down
5 changes: 0 additions & 5 deletions api/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ const attributes = DataTypes => ({
unique: true,
allowNull: false,
},
isActive: {
type: DataTypes.BOOLEAN,
defaultValue: true,
allowNull: false,
},
pushedAt: {
type: DataTypes.DATE,
},
Expand Down
1 change: 0 additions & 1 deletion api/serializers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const adminAttributes = {
updatedAt: 'date',
signedInAt: 'date',
pushedAt: 'date',
isActive: '',
adminEmail: '',
deletedAt: 'date',
};
Expand Down
3 changes: 0 additions & 3 deletions api/services/Webhooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
Expand Down
10 changes: 10 additions & 0 deletions migrations/20231221154733-remove-is-active-from-user.js
Original file line number Diff line number Diff line change
@@ -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');
};
8 changes: 1 addition & 7 deletions test/api/requests/auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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);
Expand Down
70 changes: 2 additions & 68 deletions test/api/unit/services/Webhooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } })
Expand All @@ -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);
Expand Down Expand Up @@ -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');

Expand All @@ -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);


Expand All @@ -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;
});
});
});

0 comments on commit 30cc860

Please sign in to comment.