Skip to content

247 Added Missing Endpoints to Reports Lambda - #287

Merged
mehanana merged 6 commits into
mainfrom
247-Reports-Missing-Endpoints
Jul 28, 2026
Merged

247 Added Missing Endpoints to Reports Lambda#287
mehanana merged 6 commits into
mainfrom
247-Reports-Missing-Endpoints

Conversation

@mehanana

Copy link
Copy Markdown
Contributor

ℹ️ Issue

Closes #247

📝 Description

Added the endpoints GET /reports/{id} and DELETE /reports/id to the reports lambda with authentication. Also wrote tests for these

Briefly list the changes made to the code:

  1. Added the 2 new endpoints
  2. Wrote jest tests (e2e and unit)

✔️ Verification

Ran jest tests & tested through swagger UI

Provide screenshots of any new components, styling changes, or pages.

Jest:
image

Unauthorized:
Screenshot 2026-07-19 133403
Screenshot 2026-07-19 133419

Success:
Screenshot 2026-07-19 133840
Screenshot 2026-07-19 133851

Not Found:
Screenshot 2026-07-19 133904
Screenshot 2026-07-19 133913

🏕️ (Optional) Future Work / Notes

Did you notice anything ugly during the course of this ticket? Any bugs, design challenges, or unexpected behavior? Write it down so we can clean it up in a future ticket!

@mehanana
mehanana requested a review from nourshoreibah as a code owner July 19, 2026 17:46
@github-actions
github-actions Bot requested a review from Vaibhav978 July 19, 2026 17:46
github-actions Bot added a commit that referenced this pull request Jul 19, 2026
Comment thread apps/backend/lambdas/reports/handler.ts Outdated
// <<< ROUTES-END

// GET /reports/{id}
if ((/^\/reports\/\d+$/.test(normalizedPath) || /^\/\d+$/.test(normalizedPath)) && method === 'GET') { const id = normalizedPath.split('/').pop() as string;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I'm not wrong the route matching is duplicated for both GET and DELETE. Maybe you could extract that out?

Comment thread apps/backend/lambdas/reports/handler.ts Outdated
if ((/^\/reports\/\d+$/.test(normalizedPath) || /^\/\d+$/.test(normalizedPath)) && method === 'DELETE') {
const id = normalizedPath.split('/').pop() as string;

if (!/^\d+$/.test(id)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a redundant check, I could be wrong though.

Comment thread apps/backend/lambdas/reports/handler.ts Outdated

const authContext = await authenticateRequest(event);
if (!authContext.isAuthenticated || !authContext.user) {
return json(401, { message: 'Authentication required' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the auth logic here is duplicated from line 274, maybe we should extract that out?

@Vaibhav978 Vaibhav978 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved pending fixing lambda tests, great job!

@mehanana
mehanana added this pull request to the merge queue Jul 28, 2026
Merged via the queue into main with commit 95ebd98 Jul 28, 2026
16 checks passed
@mehanana
mehanana deleted the 247-Reports-Missing-Endpoints branch July 28, 2026 01:59
github-actions Bot added a commit that referenced this pull request Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Backend] Missing CRUD endpoints across lambdas

3 participants