Skip to content

Created API endpoints and React hooks for application cards#218

Open
bodhiYG wants to merge 4 commits intomainfrom
bg-integrate-app-numbers
Open

Created API endpoints and React hooks for application cards#218
bodhiYG wants to merge 4 commits intomainfrom
bg-integrate-app-numbers

Conversation

@bodhiYG
Copy link
Copy Markdown

@bodhiYG bodhiYG commented Mar 24, 2026

ℹ️ Issue

Closes

📝 Description

Write a short summary of what you added. Why is it important? Any member of C4C should be able to read this and understand your contribution -- not just your team members.

Briefly list the changes made to the code:

  1. Created API endpoints
  2. Created React hooks
  3. Added tests

✔️ Verification

What steps did you take to verify your changes work? These should be clear enough for someone to be able to clone the branch and follow the steps themselves.

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

🏕️ (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!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds backend count endpoints and frontend hooks to populate the Admin dashboard “application cards” with live counts instead of hardcoded values.

Changes:

  • Added 4 backend /applications/count/* endpoints and corresponding service methods for application status counts.
  • Added frontend ApiClient methods + React hooks to fetch those counts.
  • Added frontend and backend tests for the new count behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
apps/frontend/src/containers/AdminLanding.tsx Replaces hardcoded dashboard tile values with live counts via new hooks
apps/frontend/src/containers/AdminLanding.spec.tsx Adds a unit test verifying the dashboard renders counts from mocked hooks
apps/frontend/src/api/apiClient.ts Adds count-fetching API methods and introduces use*Count hooks
apps/backend/src/applications/applications.service.ts Adds repository count() queries for total/in-review/rejected/approved-or-active
apps/backend/src/applications/applications.controller.ts Exposes new GET endpoints returning { count } payloads
apps/backend/src/applications/applications.controller.spec.ts Adds controller tests for the new count endpoints
apps/backend/src/applications/application.service.spec.ts Adds service tests for the new counting methods

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const [error, setError] = useState<Error | null>(null);

useEffect(() => {
let mounted = true;
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

useCount never sets isLoading back to true when the effect re-runs (e.g., if a different getter is passed in). This can produce incorrect loading state semantics. Consider setting setIsLoading(true) at the start of the effect before invoking getter().

Suggested change
let mounted = true;
let mounted = true;
setIsLoading(true);

Copilot uses AI. Check for mistakes.
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.

^agree but see how it plays out in practice

Comment on lines +121 to +123
.catch((err: Error) => {
if (mounted) {
setError(err);
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The .catch((err: Error) => ...) annotation is unsound: promise rejections can be non-Error values (and Axios often rejects with AxiosError, typed as unknown in some configs). Consider catching unknown and normalizing to an Error instance before storing it in state.

Suggested change
.catch((err: Error) => {
if (mounted) {
setError(err);
.catch((err: unknown) => {
if (mounted) {
const normalizedError =
err instanceof Error ? err : new Error(String(err));
setError(normalizedError);

Copilot uses AI. Check for mistakes.
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.

^ Agree

Comment on lines +180 to +186
it('should return approved or active count', async () => {
mockRepository.count.mockResolvedValue(102);

const result = await service.countApprovedOrActive();

expect(repository.count).toHaveBeenCalled();
expect(result).toBe(102);
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The countApprovedOrActive test only asserts repository.count was called, but not that it was called with the expected where: { appStatus: In([ACCEPTED, ACTIVE]) } filter. This means the test would still pass if the implementation counted all applications. Tighten the assertion to validate the filter arguments.

Copilot uses AI. Check for mistakes.
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.

^ Agree with this

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@SamNie2027
Copy link
Copy Markdown
Collaborator

No additional comments - everything looks good besides what copilot said

@@ -0,0 +1,45 @@
import { render, screen } from '@testing-library/react';
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.

Woah frontend tests, cool. Usually we don't do frontend tests but I like it!

@bodhiYG bodhiYG requested a review from SamNie2027 March 31, 2026 08:45
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.

Integrate Total Applications, Pending Review, Rejected, Approved - numbers with backend

3 participants