Kai/habitica
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move user deletion to worker (#15586)

Browse Source 
* WIP(delete): remove business logic from controller

* fix(deletion): handle group leave logic on app server still

* fix(lint): unused import

* fix(import): bracket syntax

* fix(test): adapt test for worker flow

* fix(deletion): update delete/feedback form copy

* fix(text): don't break to new paragraph about Gems

* fix(deletion): remove orphaned chat messages

* Revert "fix(deletion): handle group leave logic on app server still"

This reverts commit 9db541f4c3.

* fix(tests): remove tests
These can potentially be tested in the worker's suite? They target functionality that the group leave route handles within the deletion flow

* fix(lint): no-undef

* refactor redis setup into own file and use ioredis

* use bullmq directly to schedule jobs

* add space

* add key prefix

* add semicolon

* fix(jobs): update redis package

---------

Co-authored-by: Phillip Thelen <phillip@habitica.com>
This commit is contained in:
Kalista Payne
2026-05-26 17:43:49 -05:00
committed by GitHub
parent b57fb94579
commit fb2eaa3950
17 changed files with 399 additions and 188 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/api/unit/libs/email.test.js
+22 -25
View File
@@ -1,9 +1,9 @@
/* eslint-disable global-require */
import got from 'got';
import nconf from 'nconf';
import requireAgain from 'require-again';
import { TAVERN_ID } from '../../../../website/server/models/group';
import { defer } from '../../../helpers/api-unit.helper';
import worker from '../../../../website/server/libs/worker';
function getUser () {
return {
@@ -127,7 +127,7 @@ describe('emails', () => {
let sendTxn = null;
beforeEach(() => {
sandbox.stub(got, 'post').returns(defer().promise);
sandbox.stub(worker, 'sendJob').returns(defer().promise);
const nconfGetStub = sandbox.stub(nconf, 'get');
nconfGetStub.withArgs('IS_PROD').returns(true);
@@ -149,13 +149,12 @@ describe('emails', () => {
};
sendTxn(mailingInfo, emailType);
expect(got.post).to.be.called;
expect(got.post).to.be.calledWith('http://example.com/job', sinon.match({
json: {
data: {
emailType: sinon.match.same(emailType),
to: sinon.match(value => Array.isArray(value) && value[0].name === mailingInfo.name, 'matches mailing info array'),
},
expect(worker.sendJob).to.be.called;
expect(worker.sendJob).to.be.calledWith('email', sinon.match({
identifier: emailType,
data: {
emailType: sinon.match.same(emailType),
to: sinon.match(value => Array.isArray(value) && value[0].name === mailingInfo.name, 'matches mailing info array'),
},
}));
});
@@ -168,7 +167,7 @@ describe('emails', () => {
};
sendTxn(mailingInfo, emailType);
expect(got.post).not.to.be.called;
expect(worker.sendJob).not.to.be.called;
});
it('throws error when mail target is only a string', async () => {
@@ -233,13 +232,12 @@ describe('emails', () => {
const mailingInfo = getUser();
sendTxn(mailingInfo, emailType);
expect(got.post).to.be.called;
expect(got.post).to.be.calledWith('http://example.com/job', sinon.match({
json: {
data: {
emailType: sinon.match.same(emailType),
to: sinon.match(val => val[0]._id === mailingInfo._id),
},
expect(worker.sendJob).to.be.called;
expect(worker.sendJob).to.be.calledWith('email', sinon.match({
identifier: emailType,
data: {
emailType: sinon.match.same(emailType),
to: sinon.match(val => val[0]._id === mailingInfo._id),
},
}));
});
@@ -253,15 +251,14 @@ describe('emails', () => {
const variables = [];
sendTxn(mailingInfo, emailType, variables);
expect(got.post).to.be.called;
expect(got.post).to.be.calledWith('http://example.com/job', sinon.match({
json: {
data: {
variables: sinon.match(value => value[0].name === 'BASE_URL', 'matches variables'),
personalVariables: sinon.match(value => value[0].rcpt === mailingInfo.email
&& value[0].vars[0].name === 'RECIPIENT_NAME'
expect(worker.sendJob).to.be.called;
expect(worker.sendJob).to.be.calledWith('email', sinon.match({
identifier: emailType,
data: {
variables: sinon.match(value => value[0].name === 'BASE_URL', 'matches variables'),
personalVariables: sinon.match(value => value[0].rcpt === mailingInfo.email
&& value[0].vars[0].name === 'RECIPIENT_NAME'
&& value[0].vars[1].name === 'RECIPIENT_UNSUB_URL', 'matches personal variables'),
},
},
}));
});
test/api/v3/integration/quests/POST-groups_groupId_quests_force-start.test.js
-17
View File
@@ -193,23 +193,6 @@ describe('POST /groups/:groupId/quests/force-start', () => {
expect(questingGroup.quest.members[notInPartyUser._id]).to.not.exist;
});
it('removes users who have been deleted from quest.members', async () => {
await leader.post(`/groups/${questingGroup._id}/quests/invite/${PET_QUEST}`);
await partyMembers[0].post(`/groups/${questingGroup._id}/quests/accept`);
await partyMembers[0].del('/user', {
password: 'password',
});
await leader.post(`/groups/${questingGroup._id}/quests/force-start`);
await sleep(0.5);
await questingGroup.sync();
expect(questingGroup.quest.members[partyMembers[0]._id]).to.not.exist;
});
it('removes users who don\'t have true value in quest.members from quest.members', async () => {
const partyMemberThatRejects = partyMembers[1];
const partyMemberThatIgnores = partyMembers[2];
test/api/v3/integration/user/DELETE-user.test.js
+15 -63
View File
@@ -1,13 +1,7 @@
import {
each,
map,
} from 'lodash';
import {
checkExistence,
createAndPopulateGroup,
generateGroup,
generateUser,
generateChallenge,
translate as t,
} from '../../../../helpers/api-integration/v3';
import {
@@ -15,6 +9,7 @@ import {
sha1Encrypt as sha1EncryptPassword,
} from '../../../../../website/server/libs/password';
import * as email from '../../../../../website/server/libs/email';
import sendJob from '../../../../../website/server/libs/worker';
const DELETE_CONFIRMATION = 'DELETE';
@@ -47,12 +42,13 @@ describe('DELETE /user', () => {
});
});
it('deletes the user', async () => {
await expect(checkExistence('users', user._id)).to.eventually.eql(true);
it('sends deletion job to worker', async () => {
const workerStub = sandbox.stub(sendJob, 'sendJob');
await user.del('/user', {
password,
});
await expect(checkExistence('users', user._id)).to.eventually.eql(false);
expect(workerStub).to.be.calledOnce;
workerStub.restore();
});
it('returns an error if excessive feedback is supplied', async () => {
@@ -84,53 +80,6 @@ describe('DELETE /user', () => {
});
});
it('deletes the user\'s tasks', async () => {
await user.post('/tasks/user', {
text: 'test habit',
type: 'habit',
});
await user.sync();
// gets the user's tasks ids
const ids = [];
each(user.tasksOrder, idsForOrder => {
ids.push(...idsForOrder);
});
expect(ids.length).to.be.above(0); // make sure the user has some task to delete
await user.del('/user', {
password,
});
await Promise.all(map(ids, id => expect(checkExistence('tasks', id)).to.eventually.eql(false)));
});
it('reduces memberCount in challenges user is linked to', async () => {
const populatedGroup = await createAndPopulateGroup({
members: 2,
});
const { group } = populatedGroup;
const authorizedUser = populatedGroup.members[1];
const challenge = await generateChallenge(populatedGroup.groupLeader, group);
await populatedGroup.groupLeader.post(`/challenges/${challenge._id}/join`);
await authorizedUser.post(`/challenges/${challenge._id}/join`);
await challenge.sync();
expect(challenge.memberCount).to.eql(2);
await authorizedUser.del('/user', {
password,
});
await challenge.sync();
expect(challenge.memberCount).to.eql(1);
});
it('sends feedback to the admin email', async () => {
sandbox.spy(email, 'sendTxn');
@@ -158,10 +107,10 @@ describe('DELETE /user', () => {
});
it('deletes the user with a legacy sha1 password', async () => {
await expect(checkExistence('users', user._id)).to.eventually.eql(true);
const textPassword = 'mySecretPassword';
const salt = sha1MakeSalt();
const sha1HashedPassword = sha1EncryptPassword(textPassword, salt);
const workerStub = sandbox.stub(sendJob, 'sendJob');
await user.updateOne({
'auth.local.hashed_password': sha1HashedPassword,
@@ -179,7 +128,8 @@ describe('DELETE /user', () => {
await user.del('/user', {
password: textPassword,
});
await expect(checkExistence('users', user._id)).to.eventually.eql(false);
expect(workerStub).to.be.calledOnce;
workerStub.restore();
});
context('last member of a party', () => {
@@ -213,11 +163,12 @@ describe('DELETE /user', () => {
});
it('deletes a Google user', async () => {
await expect(checkExistence('users', user._id)).to.eventually.eql(true);
const workerStub = sandbox.stub(sendJob, 'sendJob');
await user.del('/user', {
password: DELETE_CONFIRMATION,
});
await expect(checkExistence('users', user._id)).to.eventually.eql(false);
expect(workerStub).to.be.calledOnce;
workerStub.restore();
});
});
@@ -232,12 +183,13 @@ describe('DELETE /user', () => {
});
});
it('deletes a Apple user', async () => {
await expect(checkExistence('users', user._id)).to.eventually.eql(true);
it('deletes an Apple user', async () => {
const workerStub = sandbox.stub(sendJob, 'sendJob');
await user.del('/user', {
password: DELETE_CONFIRMATION,
});
await expect(checkExistence('users', user._id)).to.eventually.eql(false);
expect(workerStub).to.be.calledOnce;
workerStub.restore();
});
});
});