Thanks for adding the ShiftRequest API work. The feature direction is useful and aligns with the need for shift swap / leave request workflows.

I think this needs changes before merge.

1. Lint currently fails on this branch

I checked out the PR branch locally and ran:

npm run lint

The branch currently reports 18 ESLint errors. Even if some of these may be pre-existing or outside the new ShiftRequest files, the PR doesn't currently pass the backend lint check locally, so this should be resolved or clearly separated before merge.

2. Endpoint contract does not match the ticket

The ticket specifies:

• POST /api/v1/shifts/request
• GET /api/v1/shifts/requests
• PATCH /api/v1/shifts/request/{id}

However, the implementation appears to mount the new routes under:

• /api/v1/shiftrequests/requests
• /api/v1/shiftrequests/requests/{id}

The controller comments also refer to /api/v1/shifts/requests, while the route file documents /api/v1/shiftrequests/requests, so the endpoint contract is inconsistent across the ticket, controller comments, Swagger docs, and actual route mounting.

Could this please be aligned before merge, or confirmed if the ticket/API contract has intentionally changed?

3. No dedicated ShiftRequest tests found

I could not find a dedicated ShiftRequest test file for this new workflow. Since this introduces a new approval process, I think we need targeted regression coverage for at least:

• guard creates a SWAP request
• guard creates a LEAVE request
• non-guard cannot create requests
• duplicate pending request is blocked
• target guard accepts/declines a SWAP
• employer/admin approves or rejects a request
• employer cannot view or approve requests for shifts they do not own
• approved SWAP/LEAVE updates the Shift assignment state consistently

4. Employer authorization/scoping needs tightening

Please confirm that employers can only view/approve/reject requests connected to shifts they created or own. In a multi-employer system, role-level checks are not enough; the request needs data-level ownership scoping as well.

5. Shift assignment updates need confirmation

For approved SWAP/LEAVE requests, please confirm the implementation updates all Shift assignment fields used elsewhere in the backend/frontend, not only one field such as acceptedBy. If other parts of the system rely on guardIds, acceptedBy, status, or applicant fields, partial updates could leave shifts in an inconsistent state.

6. Notification/socket changes look out of scope or incomplete

This PR also changes notification/socket-related files. If these are required for the ShiftRequest workflow, they should be fully integrated and tested. If they are future groundwork, I think they should be deferred to a separate PR to keep this feature focused.

Related to that, existing Swagger documentation appears to have been removed from notification.routes.js. Since this ticket is for the ShiftRequest API, could that documentation be restored unless there is a specific reason to remove it?

Summary: The feature direction is good, but I think this will only be merge-ready when lint passes, the endpoint contract is aligned, the notification/swagger changes are clarified, and the new approval workflow has basic tests.

Thanks for the updates! I can see progress here, especially that the ShiftRequest route file and Swagger docs have been moved closer to the ticket path, and a ShiftRequest test file has been added.

I still think this needs changes before approval.

1. Actual mounted endpoint still doesn't match the ticket

The ticket requires:

• POST /api/v1/shifts/request
• GET /api/v1/shifts/requests
• PATCH /api/v1/shifts/request/{id}

The route file now defines paths like:

router.post('/shifts/request', protect, createShiftRequest);
router.get('/shifts/requests', protect, getShiftRequests);
router.patch('/shifts/request/:id', protect, updateShiftRequest);

However, routes/index.js still mounts the router at:

router.use('/shiftrequests', shiftRequestRoutes);

So the actual runtime endpoints become:

• /api/v1/shiftrequests/shifts/request
• /api/v1/shiftrequests/shifts/requests
• /api/v1/shiftrequests/shifts/request/:id

That still doesn't match the ticket or the Swagger docs.

To match the ticket, this probably needs to be mounted under the existing /shifts route, with the child routes defined as /request, /requests, and /request/:id, or otherwise integrated directly into shift.routes.js.

2. Lint still fails

I reran:

npm run lint

and the branch currently fails lint. Some errors appear pre-existing, but there are now also errors in the new ShiftRequest work, including:

• src/controllers/shiftrequest.controller.js
• src/tests/shiftrequest.test.js

The new test file appears to use Jest globals such as describe, it, and expect without importing them or otherwise satisfying the repo’s ESLint config.

3. Tests were added, but currently need cleanup

It is good that a ShiftRequest test file has been added. However, because it currently fails lint, it needs to be cleaned up before the PR is merge-ready.

4. Remaining logic concerns

Before approval, I'd still like to see confirmation that:

• employer access is data-scoped so employers can only view/approve requests for shifts they own
• SWAP/LEAVE approval updates the same Shift assignment fields used elsewhere in the app
• the route, controller comments, Swagger docs, and actual mounted endpoint all describe the same API contract

The PR is closer to the ticket specs and moving in the right direction, but best not to approve while the actual endpoint still differs from the ticket and lint is failing.