Skip to content

4499 add controller tests for high priority untested modules - #4628

Open
DeboraSerra wants to merge 7 commits into
developfrom
4499-add-controller-tests-for-high-priority-untested-modules
Open

4499 add controller tests for high priority untested modules#4628
DeboraSerra wants to merge 7 commits into
developfrom
4499-add-controller-tests-for-high-priority-untested-modules

Conversation

@DeboraSerra

Copy link
Copy Markdown
Contributor

Describe your changes

Six of the seven controllers named here already had passing suites, so this covers the real
gaps: a new advisor.ctrl.ts suite (73 tests; SSE streaming excluded) and branch top-ups on
fileManager and approvalRequest. On the frontend, existing error tests asserted against a bare thrown Error, so the MSW error scaffold now spans six domains with 25 tests using the real { message, data } envelope. Two findings for reviewers: the scaffold's envelope was inverted and is fixed here; getFileErrorMessage branches on statusCode while apiServices throws .status, so those branches are dead — pinned by tests, left unfixed as it changes user-facing copy.

Write your issue number after "Fixes "

Fixes #4499

Please ensure all items are checked off before requesting a review:

  • I deployed the code locally.
  • I have performed a self-review of my code.
  • I have included the issue # in the PR.
  • I have labelled the PR correctly.
  • The issue I am working on is assigned to me.
  • I have avoided using hardcoded values to ensure scalability and maintain consistency across the application.
  • I have ensured that font sizes, color choices, and other UI elements are referenced from the theme.
  • My pull request is focused and addresses a single, specific feature.
  • If there are UI changes, I have attached a screenshot or video to this PR.
  • If I added or modified an API endpoint, the change is reflected in the generated OpenAPI spec (npm run generate:swagger).
  • If the endpoint requires authentication, it uses authenticateJWT and the generated spec declares bearerAuth security.
  • I ran npm run check:api-drift and committed the regenerated swagger.yaml and endpoints.ts.
  • If this PR adds or modifies an organization-scoped table, the tenant isolation registry and test matrix are updated. See the tenant isolation runbook for details.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

✅ Coverage Gate Passed

All coverage thresholds are met.

@MuhammadKhalilzadeh MuhammadKhalilzadeh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @DeboraSerra
Let's fix the failing github checks as well

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.

Add controller tests for high-priority untested modules

2 participants