Skip to content

Add error handling and validation tests for Comments API - #126

Open
devin-ai-integration[bot] wants to merge 2 commits into
developfrom
devin/1779282842-add-comments-api-error-tests
Open

devin-ai-integration[bot] wants to merge 2 commits into
developfrom
devin/1779282842-add-comments-api-error-tests

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented May 20, 2026

Copy link
Copy Markdown

Summary

Adds 8 new test cases to cypress/tests/api/api-comments.spec.ts covering error handling and validation for the Comments API:

Unauthenticated access (401):

  • GET /comments/:transactionId without auth → 401 + { error: "Unauthorized" }
  • POST /comments/:transactionId without auth → 401

Invalid transactionId (422):

  • GET with malformed transactionId → 422, non-empty errors array
  • POST with malformed transactionId → 422, non-empty errors array

Invalid POST body (422):

  • Empty body {} → 422, errors reference content field (param === "content")
  • Non-string content { content: 12345 } → 422
  • Unknown field only { notACommentField: "foo" } → 422

Structural change: Moved cy.loginByApi() from the top-level beforeEach into per-context beforeEach blocks so the Unauthenticated context can run without login. The db:seed and transactionId lookup remain in the top-level beforeEach.

All new tests use failOnStatusCode: false and follow the patterns from api-transactions.spec.ts and api-notifications.spec.ts.

Review & Testing Checklist for Human

  • Verify the unauthenticated tests actually skip login (the Unauthenticated context has no beforeEach calling loginByApi)
  • Confirm the existing happy-path tests still pass after the beforeEach restructuring
  • Run the full API test suite locally: npx cypress run --spec 'cypress/tests/api/api-comments.spec.ts'

Notes

  • The beforeEach restructuring only moves the login call — db:seed and comment lookup remain shared across all contexts
  • Error response shapes match the backend middleware: ensureAuthenticated (401), validateMiddleware (422), and shortIdValidation (422)
  • express-validator uses param (not path) as the field name key in error objects

Link to Devin session: https://app.devin.ai/sessions/9750ca3b5695429782d9bc38110acce0
Requested by: @araza-cog


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Open in Devin Review (Staging)
Open in Devin Review

- Add Unauthenticated context: GET and POST without auth return 401
- Add malformed transactionId tests (422) for both GET and POST
- Add invalid POST body tests (422): empty body, non-string content, unknown field
- Restructure beforeEach so login only runs for authenticated contexts
- Use failOnStatusCode: false on all error tests

Co-Authored-By: Ahmad Raza <ahmad.raza@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

});
});

it("errors with malformed transactionId", function () {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can you add a comment here explaining the change

…tor error field

Co-Authored-By: Ahmad Raza <ahmad.raza@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

Test Results

Ran api-comments.spec.ts via Cypress CLI against the local dev server. All 9 tests pass.

New error handling tests (7 tests)
  • passed — GET with malformed transactionId → 422, errors array non-empty
  • passed — POST with malformed transactionId → 422, errors array non-empty
  • passed — POST with empty body {} → 422, errors[0].param === "content"
  • passed — POST with non-string content { content: 12345 } → 422
  • passed — POST with unknown field { notACommentField: "foo" } → 422
  • passed — GET without auth → 401, error === "Unauthorized"
  • passed — POST without auth → 401
Regression: existing happy-path tests (2 tests)
  • passed — GET /comments/:transactionId → 200, comments array length 1
  • passed — POST /comments/:transactionId → 200
Cypress CLI output
  Comments API
    GET /comments/:transactionId
      ✓ gets a list of comments for a transaction (234ms)
      ✓ errors with malformed transactionId (154ms)
    POST /comments/:transactionId
      ✓ creates a new comment for a transaction (164ms)
      ✓ errors with malformed transactionId (163ms)
    POST /comments/:transactionId - invalid body
      ✓ errors when POST body is empty (152ms)
      ✓ errors when content is non-string (149ms)
      ✓ errors when unknown field sent without content (157ms)
    Unauthenticated
      ✓ rejects GET /comments/:transactionId without auth (42ms)
      ✓ rejects POST /comments/:transactionId without auth (40ms)

  9 passing (1s)

Note: Initial run caught a bug in the test — errors[0].path should be errors[0].param (express-validator uses param). Fixed in 6a83e2c.

Devin session

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.

1 participant