Skip to content

fix: Once loaded, only campaign Grades and Subjects are shown.#4743

Merged
bhanugummana merged 1 commit into
mainfrom
main-ng-9
Jul 17, 2026
Merged

fix: Once loaded, only campaign Grades and Subjects are shown.#4743
bhanugummana merged 1 commit into
mainfrom
main-ng-9

Conversation

@narayanangourav-sutara

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the CampaignAssignmentTab component to keep filters disabled while campaign assignments are loading and no subjects are present, and simplifies the grade filtering logic. The review feedback highlights a false-positive risk in the newly added test case, noting that the filters are disabled initially due to isLoadingFilters being true, and provides a code suggestion to robustly verify that filters remain disabled during assignment loading and become enabled once they resolve.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +276 to +290
it('keeps filters disabled until campaign grade metadata is loaded', async () => {
api.getAllGrades.mockResolvedValue(defaultGrades);
api.getCampaignAssignments.mockImplementation(
() => new Promise(() => undefined),
);

renderTab('campaign-1');

await waitFor(() => expect(api.getAllGrades).toHaveBeenCalled());
screen
.getAllByRole('combobox')
.forEach((combobox) =>
expect(combobox).toHaveAttribute('aria-disabled', 'true'),
);
});

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.

medium

The current test has a false-positive risk. It asserts that the filters are disabled immediately after api.getAllGrades is called, but before the promise resolves. At this point, isLoadingFilters is still true, so the filters are disabled regardless of the new logic. To make the test robust, we should resolve api.getAllGrades and verify that the filters remain disabled while assignments are loading, and then verify they become enabled once assignments resolve.

  it('keeps filters disabled until campaign grade metadata and assignments are loaded', async () => {
    api.getAllGrades.mockResolvedValue(defaultGrades);

    let resolveAssignments: (value: any) => void = () => {};
    const assignmentsPromise = new Promise((resolve) => {
      resolveAssignments = resolve;
    });
    api.getCampaignAssignments.mockReturnValue(assignmentsPromise);

    renderTab('campaign-1');

    await waitFor(() => expect(api.getAllGrades).toHaveBeenCalled());

    screen
      .getAllByRole('combobox')
      .forEach((combobox) =>
        expect(combobox).toHaveAttribute('aria-disabled', 'true'),
      );

    resolveAssignments({
      assignments: defaultAssignments,
      uniqueSubjects: defaultSubjects,
      total: 1,
    });

    await waitFor(() => {
      screen
        .getAllByRole('combobox')
        .forEach((combobox) =>
          expect(combobox).not.toHaveAttribute('aria-disabled', 'true'),
        );
    });
  });

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3a663ecf9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return relevantGradeIds.size === 0
? grades
: grades.filter((grade) => relevantGradeIds.has(grade.id));
return grades.filter((grade) => relevantGradeIds.has(grade.id));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve grades when subject metadata has no grade IDs

When get_campaign_assignments returns the shape in the checked-in Supabase contract (src/services/database.ts:7867-7870), unique_subjects has no grade_ids; the parser therefore assigns every subject gradeIds: [] (SupabaseApi.ts:10895-10902). This unconditional filter then removes every grade after loading any campaign, whereas the deleted fallback kept the grade selector usable. Either ensure the RPC contract and deployment supply grade_ids, or retain a fallback for metadata that lacks them.

Useful? React with 👍 / 👎.

@bhanugummana
bhanugummana merged commit 6961a13 into main Jul 17, 2026
1 check passed
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.

2 participants