1
0
mirror of https://github.com/Unleash/unleash.git synced 2024-12-22 19:07:54 +01:00

fix: inactive users query was too wide (#6133)

Knex wasn't formatting the query like I expected. This changes the query
to use more AND, less ORs
This commit is contained in:
Christopher Kolstad 2024-02-05 15:31:04 +01:00 committed by GitHub
parent ea38877b0c
commit 1da59abb2d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 18 additions and 11 deletions

View File

@ -42,20 +42,15 @@ export class InactiveUsersStore implements IInactiveUsersStore {
) )
.leftJoin( .leftJoin(
'personal_access_tokens AS pat', 'personal_access_tokens AS pat',
'users.id',
'pat.user_id', 'pat.user_id',
'users.id',
) )
.where('deleted_at', null) .where('deleted_at', null)
.andWhereRaw(`users.seen_at < now() - INTERVAL '?? DAYS'`, [
daysInactive,
])
.orWhereRaw(
`users.seen_at IS NULL AND users.created_at < now() - INTERVAL '?? DAYS'`,
[daysInactive],
)
.andWhereRaw( .andWhereRaw(
`pat.seen_at IS NULL OR pat.seen_at < now() - INTERVAL '?? DAYS'`, `(users.seen_at IS NULL OR users.seen_at < now() - INTERVAL '?? days')
[daysInactive], AND (users.created_at IS NULL OR users.created_at < now() - INTERVAL '?? days')
AND (pat.seen_at IS NULL OR pat.seen_at < now() - INTERVAL '?? days')`,
[daysInactive, daysInactive, daysInactive],
); );
stopTimer(); stopTimer();
return inactiveUsers; return inactiveUsers;

View File

@ -110,7 +110,8 @@ describe('Inactive users service', () => {
VALUES (9595, 'test user with active PAT', 'nedryerson', 'ned@ryerson.com', VALUES (9595, 'test user with active PAT', 'nedryerson', 'ned@ryerson.com',
now() - INTERVAL '200 DAYS')`); now() - INTERVAL '200 DAYS')`);
await db.rawDatabase.raw( await db.rawDatabase.raw(
`INSERT INTO personal_access_tokens(secret, user_id, expires_at, seen_at, created_at) VALUES ('user:somefancysecret', 9595, now() + INTERVAL '6 MONTHS', now() - INTERVAL '1 WEEK', now() - INTERVAL '8 MONTHS')`, `INSERT INTO personal_access_tokens(secret, user_id, expires_at, seen_at, created_at)
VALUES ('user:somefancysecret', 9595, now() + INTERVAL '6 MONTHS', now() - INTERVAL '1 WEEK', now() - INTERVAL '8 MONTHS')`,
); );
const users = await inactiveUserService.getInactiveUsers(); const users = await inactiveUserService.getInactiveUsers();
expect(users).toBeTruthy(); expect(users).toBeTruthy();
@ -127,6 +128,17 @@ describe('Inactive users service', () => {
expect(users).toBeTruthy(); expect(users).toBeTruthy();
expect(users).toHaveLength(1); expect(users).toHaveLength(1);
}); });
test('A user with a pat that was seen 7 months ago, but logged in yesterday should not be inactive', async () => {
await db.rawDatabase.raw(
`INSERT INTO users(id, name, username, email, created_at, seen_at) VALUES (9595, 'test user with active login and old PAT', 'nedryerson', 'ned@ryerson.com', now() - INTERVAL '1 YEAR', now() - INTERVAL '1 DAY')`,
);
await db.rawDatabase.raw(
`INSERT INTO personal_access_tokens(secret, user_id, expires_at, seen_at, created_at) VALUES ('user:somefancysecret', 9595, now() + INTERVAL '6 MONTHS', now() - INTERVAL '7 MONTHS', now() - INTERVAL '8 MONTHS')`,
);
const users = await inactiveUserService.getInactiveUsers();
expect(users).toBeTruthy();
expect(users).toHaveLength(0);
});
}); });
describe('Deleting inactive users', () => { describe('Deleting inactive users', () => {
test('Deletes users that have never logged in but was created before our deadline', async () => { test('Deletes users that have never logged in but was created before our deadline', async () => {