Repository navigation
Add JoinRequestsService unit tests - #288
Conversation
✅ Deploy Preview for servicestart ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
bd1e8d3 to
46a32e9
Compare
|
Disclaimer: This comment was AI-generated and is not necessarily completely accurate. Please take code comments with a grain of salt.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const result = await JoinRequestsService.listByOrganization(org.id, { | ||
| limit: 2, | ||
| }); | ||
|
|
||
| expect(result).toHaveLength(2); |
There was a problem hiding this comment.
limit. The API supplies an offset for later pages, but listByOrganization ignores it, so this test passes even when later pages repeat the first page. A second-page assertion would cover that gap.
Knowledge Base Used: Membership and join requests
There was a problem hiding this comment.
Good catch. This was a real bug: listByOrganization ignored offset, so page 2 returned page 1. Fixed in b038d00, with a test that checks page 2.
| await JoinRequestsService.updateStatus( | ||
| idA, | ||
| JoinRequestStatus.Denied, | ||
| admin.user.id, | ||
| admin.headers, | ||
| undefined, | ||
| JoinRequestStatus.Pending, | ||
| ); |
There was a problem hiding this comment.
idA, which belongs to the administrator’s organization, then checks that unrelated idB was left alone. It cannot catch a regression in the PATCH route’s check that rejects a request from another organization. Testing that route with idB would cover the intended boundary.
Knowledge Base Used: Membership and join requests
There was a problem hiding this comment.
Agreed. I renamed this test to "only updates the given join request" and added tests/unit/api/joinrequests/route.test.ts, which checks that PATCH returns 404 for another organization's request and leaves it unchanged. Fixed in b038d00.
| await db | ||
| .update(joinRequestHistory) | ||
| .set({ resolvedAt: new Date("2025-01-01T00:00:00Z") }) | ||
| .where(eq(joinRequestHistory.action, "approved")); |
There was a problem hiding this comment.
approved history entry, not just the one created for this request. Current cleanup makes that harmless here, but adding another approved entry to the test would silently change its fixture data. Scope the update to this request.
There was a problem hiding this comment.
Scoped the update to this join request's ID in b038d00.
- Pass the offset through in JoinRequestsService.listByOrganization so later pages no longer repeat the first page - Test pagination offset - Test that PATCH /api/joinRequests rejects a request from another organization - Scope the history timestamp fixture to one join request
This branch was successfully deployed
2 participants
Closes #188
Checklist:
Description
Adds unit tests for every function in
JoinRequestsService, run against the real test Postgres like the other service tests.Changes Made
New
tests/unit/lib/services/JoinRequestService.test.tsandtests/unit/api/joinrequests/route.test.ts.create,findById,findByUserAndOrganization,findByIdAndOrganization: return shape, not found, and a request in another organization.listByOrganization: user fields, history, organization name, newest-first order,limit,offset, and tenant isolation.listHistory/addHistoryEntry: order, resolver name, and scoping to one request.updateStatus: approve (adds member, notifies, records history), deny with and without a reason, approved → pending (removes member, sends "Access Removed"), no notification when the status is unchanged, and no side effects on another organization's request.deleteById: deletes the request and its history, and leaves other requests alone.PATCH /api/joinRequests: updates a request in the admin's organization, and returns 404 for another organization's request without changing it.One production fix:
listByOrganizationignored theoffsetthe join requests list route passes in, so page 2 returned page 1 again. It now applies the offset.