feat(speakers): display unique activities count on speakers/submitter…#921
feat(speakers): display unique activities count on speakers/submitter…#921mulldug wants to merge 2 commits into
Conversation
…s list Add total filtered activities count to the page header alongside the speaker count, and selected activities count above the grid, for both the Speakers and Submitters views. Activity counts are deduplicated across accepted, alternate, and rejected presentations using a Set client-side for selections and a dedicated backend endpoint for the filtered total. Also fix null guards in current-summit-reducer for reg-lite and print-app settings payloads, and switch dev server to http.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughPR adds activities-count thunks and action types for speakers and submitters, dispatches those counts alongside list fetches, stores counts in reducers, surfaces totals in the speakers UI with new translations, adds defensive payload checks in current-summit reducer, and changes dev server to HTTP. ChangesActivities Count Feature
Sequence Diagram(s)sequenceDiagram
participant UI as SpeakersPage
participant Redux as Redux (dispatch)
participant SpeakersThunk as getSpeakersActivitiesCount
participant SubmittersThunk as getSubmittersActivitiesCount
participant API_S as /speakers/all/events/count
participant API_Sub as /submitters/all/events/count
participant Reducer as ListReducers
UI->>Redux: dispatch(getSpeakersBySummit)
Redux->>SpeakersThunk: call with summitId, filter, token
SpeakersThunk->>API_S: GET with filter[] & access_token
API_S-->>SpeakersThunk: { count }
SpeakersThunk->>Redux: dispatch(RECEIVE_SPEAKERS_ACTIVITIES_COUNT)
Redux->>Reducer: update speakers.totalActivities
UI->>Redux: dispatch(getSubmittersBySummit)
Redux->>SubmittersThunk: call with summitId, filter, token
SubmittersThunk->>API_Sub: GET with filter[] & access_token
API_Sub-->>SubmittersThunk: { count }
SubmittersThunk->>Redux: dispatch(RECEIVE_SUBMITTERS_ACTIVITIES_COUNT)
Redux->>Reducer: update submitters.totalActivities
Reducer->>UI: state changes reflected in headers and title
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/summit_speakers/summit-speakers-list-page.js`:
- Around line 719-731: The selectedActivities computation in the
selectedActivities IIFE only scans the current-page items, so cross-page
selections (selectedAll with exclusions or selectedItems from other pages) are
missed; update the logic to derive the count from a global selection model of
activity ids instead of page-local items: when selectedAll is true compute count
as totalActivities minus the number of activity ids represented by excludedItems
(or fetch excluded activity ids via the backend), and when selectedAll is false
compute count as the size of the union of activity ids tracked in a global
selectedActivityIds set/state (or an API-provided selected-count), replacing the
current per-page traversal in the selectedActivities calculation and using
symbols selectedActivities, selectedAll, excludedItems, selectedItems, items,
totalActivities to locate and update the code.
In `@src/reducers/summit_speakers/summit-speakers-list-reducer.js`:
- Around line 210-212: The RECEIVE_SPEAKERS_ACTIVITIES_COUNT reducer is
unconditionally writing payload.response.count and can be clobbered by stale
responses; change it to only update totalActivities when the incoming response
matches the current request context (e.g., compare payload.requestId or a
filterHash and payload.summitId against the request-tracking fields stored in
state such as
state.currentRequestId/state.currentFilterHash/state.currentSummitId). Ensure
the action that triggers the async fetch includes a stable identifier (requestId
or filterHash + summitId) and that the reducer for the request start stores that
identifier on state (e.g., in the handler that sets loading state), then in the
RECEIVE_SPEAKERS_ACTIVITIES_COUNT case only set totalActivities =
payload.response.count if payload.requestId === state.currentRequestId (or
payload.filterHash === state.currentFilterHash && payload.summitId ===
state.currentSummitId); otherwise return state unchanged.
In `@src/reducers/summit_submitters/summit-submitters-list-reducer.js`:
- Around line 198-200: The reducer unconditionally sets totalActivities on
RECEIVE_SUBMITTERS_ACTIVITIES_COUNT allowing stale async responses to overwrite
current totals; modify the flow so the count request includes a query context/id
(e.g., requestId or filterHash and currentSummitId) in the action payload and
store the latestRequestId (or latestFilterHash/currentSummitId) on the reducer
state (e.g., state.latestSubmittersCountRequestId); in the
RECEIVE_SUBMITTERS_ACTIVITIES_COUNT branch, compare payload.requestId (or
payload.filterHash + payload.summitId) with state.latestSubmittersCountRequestId
(or state.currentFilterHash/state.currentSummitId) and only set totalActivities
= payload.response.count when they match, otherwise ignore the action.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 811de08d-7885-4f26-b72a-74ce3b5481d9
📒 Files selected for processing (8)
src/actions/speaker-actions.jssrc/actions/submitter-actions.jssrc/i18n/en.jsonsrc/pages/summit_speakers/summit-speakers-list-page.jssrc/reducers/summit_speakers/summit-speakers-list-reducer.jssrc/reducers/summit_submitters/summit-submitters-list-reducer.jssrc/reducers/summits/current-summit-reducer.jswebpack.dev.js
| const selectedActivities = (() => { | ||
| if (selectedAll && excludedItems.length === 0) return totalActivities; | ||
| const relevant = selectedAll | ||
| ? items.filter((item) => !excludedItems.includes(item.id)) | ||
| : items.filter((item) => selectedItems.includes(item.id)); | ||
| const ids = new Set(); | ||
| relevant.forEach((item) => { | ||
| (item.accepted_presentations || []).forEach((p) => ids.add(p.id)); | ||
| (item.alternate_presentations || []).forEach((p) => ids.add(p.id)); | ||
| (item.rejected_presentations || []).forEach((p) => ids.add(p.id)); | ||
| }); | ||
| return ids.size; | ||
| })(); |
There was a problem hiding this comment.
Selected activities count is incorrect for cross-page selections.
At Lines 721-724, the calculation only inspects current-page items. When selections span multiple pages (or selectedAll with exclusions), selectedActivities no longer reflects all selected entities, so the UI shows inaccurate totals.
You’ll need a selection model that tracks activity ids beyond the current page (or a backend selected-count endpoint) to keep this number correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/summit_speakers/summit-speakers-list-page.js` around lines 719 -
731, The selectedActivities computation in the selectedActivities IIFE only
scans the current-page items, so cross-page selections (selectedAll with
exclusions or selectedItems from other pages) are missed; update the logic to
derive the count from a global selection model of activity ids instead of
page-local items: when selectedAll is true compute count as totalActivities
minus the number of activity ids represented by excludedItems (or fetch excluded
activity ids via the backend), and when selectedAll is false compute count as
the size of the union of activity ids tracked in a global selectedActivityIds
set/state (or an API-provided selected-count), replacing the current per-page
traversal in the selectedActivities calculation and using symbols
selectedActivities, selectedAll, excludedItems, selectedItems, items,
totalActivities to locate and update the code.
There was a problem hiding this comment.
The two fixes suggested both have significant cost. A simpler option: show the activities count only when it's guaranteed to be accurate, and omit it from the label otherwise. That covers the most common workflow — selecting all filtered speakers and sending emails — which is where the count is actually meaningful. Individual page-by-page checkbox selection is a much less common path.
There was a problem hiding this comment.
@mulldug That's a sensible tradeoff — the accurate path (selectedAll, no exclusions) is exactly the high-value workflow, and showing a possibly-wrong count on individual-page checkbox selections is arguably worse than showing nothing.
Here's a minimal implementation of that approach:
1. Simplify the IIFE — return null for any case that can't be proven accurate:
const selectedActivities =
selectedAll && excludedItems.length === 0 ? totalActivities : null;2. Conditionally render the activities count in the label:
<span>
<b>
{T.translate(
this.state.source === sources.speakers
? "summit_speakers_list.items_qty"
: "summit_submitters_list.items_qty",
{ qty: selectedCount }
)}
{selectedActivities !== null &&
` | ${selectedActivities} ${T.translate("general.activities")}`}
</b>
</span>This keeps the i18n items_qty strings simpler (no activitiesQty placeholder needed), and the activities suffix is appended directly in JSX only when the value is reliable. You'd also want to revert the i18n items_qty changes for both summit_speakers_list and summit_submitters_list back to their original form (without the activitiesQty placeholder) since it's now handled inline.
(ᐢ·ᴗ·ᐢ)
✏️ Learnings added
Learnt from: mulldug
Repo: fntechgit/summit-admin PR: 921
File: src/pages/summit_speakers/summit-speakers-list-page.js:719-731
Timestamp: 2026-05-12T14:19:55.721Z
Learning: In `src/pages/summit_speakers/summit-speakers-list-page.js`, the `selectedActivities` count is only reliable when `selectedAll === true && excludedItems.length === 0` (uses backend `totalActivities`). For partial/page-by-page selections, only current-page items are available. The project preference is to display the activities count only when accurate and omit it otherwise, rather than building a global activity-id tracking model or a dedicated backend selected-count endpoint.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: priscila-moneo
Repo: fntechgit/summit-admin PR: 896
File: src/pages/summits/summit-directory-page.js:220-221
Timestamp: 2026-05-06T22:24:01.344Z
Learning: When using `MuiTable` from `openstack-uicore-foundation/lib/components/mui/table` in this codebase, verify the callback payload types: `onDelete` is called with the primitive row identifier (e.g., `id`/`rowId`), not the full row object, while `onEdit` and `onSelect` receive the full row object. Use `onDelete={(id) => deleteSummit(id)}` (or equivalent) so the delete handler receives only the row ID. Note that the `getName` prop is used by `MuiTable` for displaying the delete dialog, not to transform the `onDelete` argument.
| case RECEIVE_SPEAKERS_ACTIVITIES_COUNT: { | ||
| return { ...state, totalActivities: payload.response.count }; | ||
| } |
There was a problem hiding this comment.
Prevent stale count responses from clobbering current speaker totals.
At Line 211, the reducer accepts whichever count response arrives last. Under rapid filter/search changes, stale responses can overwrite the active query’s totalActivities.
Please gate this update with request context (request id/filter hash/current summit) so only the latest relevant response updates state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/reducers/summit_speakers/summit-speakers-list-reducer.js` around lines
210 - 212, The RECEIVE_SPEAKERS_ACTIVITIES_COUNT reducer is unconditionally
writing payload.response.count and can be clobbered by stale responses; change
it to only update totalActivities when the incoming response matches the current
request context (e.g., compare payload.requestId or a filterHash and
payload.summitId against the request-tracking fields stored in state such as
state.currentRequestId/state.currentFilterHash/state.currentSummitId). Ensure
the action that triggers the async fetch includes a stable identifier (requestId
or filterHash + summitId) and that the reducer for the request start stores that
identifier on state (e.g., in the handler that sets loading state), then in the
RECEIVE_SPEAKERS_ACTIVITIES_COUNT case only set totalActivities =
payload.response.count if payload.requestId === state.currentRequestId (or
payload.filterHash === state.currentFilterHash && payload.summitId ===
state.currentSummitId); otherwise return state unchanged.
There was a problem hiding this comment.
Every count request is fired at the exact same moment, with the exact same filter, as the main list request. Any rapid user interaction that could produce stale count responses would produce equally stale main list responses — and those have no guard. Adding a guard to just the new calls isn't solving a real problem that doesn't also exist on the main list, which the rest of the code base accepts without protection. Adding those guards everywhere they are needed would be more appropriately done as a separate project rather than in the context of this PR.
There was a problem hiding this comment.
@mulldug, that's a fair and well-reasoned point. Since the count request is fired simultaneously with the main list request using the same filter, any stale-response race condition would affect RECEIVE_SPEAKERS_BY_SUMMIT equally — and the existing reducer has no guard there either. Applying the guard selectively only to the new count action would indeed be inconsistent without broader refactoring. I agree this is better addressed as a dedicated follow-up across the codebase rather than piecemeal here.
✏️ Learnings added
Learnt from: mulldug
Repo: fntechgit/summit-admin PR: 921
File: src/reducers/summit_speakers/summit-speakers-list-reducer.js:210-212
Timestamp: 2026-05-12T14:24:56.196Z
Learning: In fntechgit/summit-admin, the existing speaker/submitter list reducers (e.g., `RECEIVE_SPEAKERS_BY_SUMMIT` in `src/reducers/summit_speakers/summit-speakers-list-reducer.js`) have no stale-response guard. The count action (`RECEIVE_SPEAKERS_ACTIVITIES_COUNT`) is dispatched at the same moment and with the same filter as the main list request, so adding a guard to only the count handler would be inconsistent. The team considers adding stale-response guards a separate, broader refactoring project, not in scope for individual feature PRs.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: priscila-moneo
Repo: fntechgit/summit-admin PR: 896
File: src/pages/summits/summit-directory-page.js:220-221
Timestamp: 2026-05-06T22:24:01.344Z
Learning: When using `MuiTable` from `openstack-uicore-foundation/lib/components/mui/table` in this codebase, verify the callback payload types: `onDelete` is called with the primitive row identifier (e.g., `id`/`rowId`), not the full row object, while `onEdit` and `onSelect` receive the full row object. Use `onDelete={(id) => deleteSummit(id)}` (or equivalent) so the delete handler receives only the row ID. Note that the `getName` prop is used by `MuiTable` for displaying the delete dialog, not to transform the `onDelete` argument.
| case RECEIVE_SUBMITTERS_ACTIVITIES_COUNT: { | ||
| return { ...state, totalActivities: payload.response.count }; | ||
| } |
There was a problem hiding this comment.
Guard against stale activity-count responses overwriting current filters.
At Line 199, totalActivities is updated from any arriving response. If users change filters/search quickly, older async responses can land late and replace the latest count, so header totals drift from current results.
Consider attaching query context (e.g., currentSummitId + filter hash/request id) to the count request and ignoring mismatched responses in the reducer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/reducers/summit_submitters/summit-submitters-list-reducer.js` around
lines 198 - 200, The reducer unconditionally sets totalActivities on
RECEIVE_SUBMITTERS_ACTIVITIES_COUNT allowing stale async responses to overwrite
current totals; modify the flow so the count request includes a query context/id
(e.g., requestId or filterHash and currentSummitId) in the action payload and
store the latestRequestId (or latestFilterHash/currentSummitId) on the reducer
state (e.g., state.latestSubmittersCountRequestId); in the
RECEIVE_SUBMITTERS_ACTIVITIES_COUNT branch, compare payload.requestId (or
payload.filterHash + payload.summitId) with state.latestSubmittersCountRequestId
(or state.currentFilterHash/state.currentSummitId) and only set totalActivities
= payload.response.count when they match, otherwise ignore the action.
There was a problem hiding this comment.
Every count request is fired at the exact same moment, with the exact same filter, as the main list request. Any rapid user interaction that could produce stale count responses would produce equally stale main list responses — and those have no guard. Adding a guard to just the new calls isn't solving a real problem that doesn't also exist on the main list, which the rest of the code base accepts without protection. Adding those guards everywhere they are needed would be more appropriately done as a separate project rather than in the context of this PR.
There was a problem hiding this comment.
@mulldug, that's a fair point. Since both the count and list requests are dispatched simultaneously with the same filter, any race condition would affect both equally, and the rest of the codebase doesn't guard the list responses either. Addressing stale-response protection consistently across the board is a reasonable follow-up project rather than a partial fix here. I'll drop this concern.
(ᵔ.ᵔ)
(_/)
✏️ Learnings added
Learnt from: mulldug
Repo: fntechgit/summit-admin PR: 921
File: src/reducers/summit_submitters/summit-submitters-list-reducer.js:198-200
Timestamp: 2026-05-12T14:25:09.422Z
Learning: In `fntechgit/summit-admin`, the `RECEIVE_SUBMITTERS_ACTIVITIES_COUNT` (and analogous `RECEIVE_SPEAKERS_ACTIVITIES_COUNT`) action is always dispatched at the same moment and with the same filter as the corresponding main list request (`RECEIVE_SUBMITTERS_BY_SUMMIT` / `RECEIVE_SPEAKERS_BY_SUMMIT`). The codebase does not guard any of these reducers against stale async responses; adding such guards only to the new count actions would be inconsistent. Codebase-wide stale-response protection is intentionally deferred to a separate future project.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: priscila-moneo
Repo: fntechgit/summit-admin PR: 896
File: src/pages/summits/summit-directory-page.js:220-221
Timestamp: 2026-05-06T22:24:01.344Z
Learning: When using `MuiTable` from `openstack-uicore-foundation/lib/components/mui/table` in this codebase, verify the callback payload types: `onDelete` is called with the primitive row identifier (e.g., `id`/`rowId`), not the full row object, while `onEdit` and `onSelect` receive the full row object. Use `onDelete={(id) => deleteSummit(id)}` (or equivalent) so the delete handler receives only the row ID. Note that the `getName` prop is used by `MuiTable` for displaying the delete dialog, not to transform the `onDelete` argument.
|
@mulldug please add the following header as the first line of every PR description: |
…iltered results Show "Selected N Speakers | M Activities" only when the full filtered set is selected (selectedAll with no exclusions), where totalActivities from the backend is accurate. Fall back to "Selected N Speakers" for partial or cross-page selections where a correct count cannot be determined client-side.
2 participants
…s list
ref: https://app.clickup.com/t/86b9b1qrk
Add total filtered activities count to the page header alongside the speaker count, and selected activities count above the grid, for both the Speakers and Submitters views. Activity counts are deduplicated across accepted, alternate, and rejected presentations using a Set client-side for selections and a dedicated backend endpoint for the filtered total.
Also fix null guards in current-summit-reducer for reg-lite and print-app settings payloads, and switch dev server to http.
Summary by CodeRabbit
New Features
Bug Fixes
Chores