fix: Subject Dropdown Is Not Displaying All Subjects in campaign#4742
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the campaign assignment tab to dynamically filter available grades based on the selected subjects by associating gradeIds with each unique subject. It includes updates to the API interfaces, Supabase integration, CSS styling for selected menu items, and comprehensive unit tests. The review feedback highlights two main improvement opportunities: first, adjusting state during the render phase instead of using a useEffect hook when campaignId changes to prevent redundant API calls and race conditions; second, adding a defensive fallback (?? []) when mapping over subject.gradeIds to avoid potential runtime crashes if the property is undefined.
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.
| useEffect(() => { | ||
| setSelectedGrades([]); | ||
| setSelectedSubjects([]); | ||
| setSubjects([]); | ||
| setPage(1); | ||
| }, [campaignId]); |
There was a problem hiding this comment.
When campaignId changes, this useEffect resets the filter states (selectedGrades, selectedSubjects, subjects, page). However, because state updates are scheduled asynchronously, the loadAssignments effect (which also depends on campaignId) will immediately trigger a fetch using the new campaignId but the old filter states. This results in a redundant API call that is immediately cancelled once the state resets are processed and trigger a second render.
To prevent this redundant network request and potential race conditions, adjust the state during the render phase instead of using a useEffect hook.
const [prevCampaignId, setPrevCampaignId] = useState(campaignId);
if (campaignId !== prevCampaignId) {
setPrevCampaignId(campaignId);
setSelectedGrades([]);
setSelectedSubjects([]);
setSubjects([]);
setPage(1);
}
| const availableGrades = useMemo(() => { | ||
| const relevantGradeIds = new Set( | ||
| subjects.flatMap((subject) => subject.gradeIds), | ||
| ); |
There was a problem hiding this comment.
To prevent potential runtime crashes if gradeIds is missing or undefined (e.g., due to incomplete test mocks or API anomalies), add a defensive fallback ?? [] when calling flatMap.
| const availableGrades = useMemo(() => { | |
| const relevantGradeIds = new Set( | |
| subjects.flatMap((subject) => subject.gradeIds), | |
| ); | |
| const availableGrades = useMemo(() => { | |
| const relevantGradeIds = new Set( | |
| subjects.flatMap((subject) => subject.gradeIds ?? []), | |
| ); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c77cd21bca
ℹ️ 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".
| gradeIds: Array.isArray(subject.grade_ids) | ||
| ? subject.grade_ids.map(String) | ||
| : [], |
There was a problem hiding this comment.
Return grade IDs from the RPC before filtering grades
When using the checked-in get_campaign_assignments contract, each unique_subjects entry contains only subject_id and subject_name (src/services/database.ts:7862-7868); this commit does not update that contract or provide an RPC migration adding grade_ids. Consequently every subject is mapped to gradeIds: [], and availableGrades falls back to every value from getAllGrades(), so campaigns continue to show unrelated grades and selecting one can produce an empty assignment list. The new test avoids this production path by supplying gradeIds directly in its mock response.
Useful? React with 👍 / 👎.
No description provided.