add tests and handling for deleted users #953
Conversation
harishv7
changed the title
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Datadog ReportBranch report: ✅ 0 Failed, 249 Passed, 36 Skipped, 48.4s Total Time |
harishv7
force-pushed
the
12-20-add_tests_for_soft_delete
branch
from
23cf858 to
e611093
Compare
| describe("user.service", () => { | ||
| beforeAll(async () => { | ||
| await resetTables("User") | ||
| await setUpWhitelist({ email: "@example.com" }) |
There was a problem hiding this comment.
do we need this? don't see any logic within isUserDeleted that's using the whitelisting table + believe this is handled by isEmailWhitelisted?
| // Setup active user | ||
| await setupUser({ | ||
| name: "Active User", | ||
| userId: "active123", | ||
| email: "active@example.com", | ||
| phone: "12345678", | ||
| isDeleted: false, | ||
| }) | ||
|
|
||
| // Setup deleted user | ||
| await setupUser({ | ||
| name: "Deleted User", | ||
| userId: "deleted123", | ||
| email: "deleted@example.com", | ||
| phone: "12345678", | ||
| isDeleted: true, | ||
| }) |
There was a problem hiding this comment.
nit - might be better to move each of this into the respective test case in the "arrange" stage?
There was a problem hiding this comment.
nit - do we want to make the non-essential fields (name, userId, phone) to fallback to a default value in setupUser instead of having to specify them? doing so will make this setup cleaner and more readable since the "attention" will be on the isDeleted arg
Problem
Deleted users were still able to access the system and view sites they previously had access to.
Closes ISOM-1701
Solution
Added checks to prevent deleted users from logging in and viewing sites they previously had access to.
Breaking Changes:
Features:
Improvements:
Tests
New scripts:
None
New dependencies:
None
New dev dependencies:
None