feature: improve speaker list activity count on select#998
Conversation
|
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 as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds selected-speaker and selected-submitter activity-count requests, stores the returned counts in list reducers, and updates the summit speakers list page to fetch and display the selected activity count with revised English strings. ChangesSelected activity counts
Sequence Diagram(s)sequenceDiagram
participant SummitSpeakersListPage
participant getSelectedSpeakersActivityCount
participant getSelectedSubmittersActivityCount
participant getRequest
participant summit_speakers_list_reducer
participant summit_submitters_list_reducer
SummitSpeakersListPage->>SummitSpeakersListPage: componentDidUpdate detects selectedCount change
alt speakers mode
SummitSpeakersListPage->>getSelectedSpeakersActivityCount: dispatch
getSelectedSpeakersActivityCount->>getRequest: GET /speakers/all/events/count with filter[]
getRequest-->>getSelectedSpeakersActivityCount: response.count
getSelectedSpeakersActivityCount->>summit_speakers_list_reducer: dispatch RECEIVE_SELECTED_SPEAKERS_ACTIVITY_COUNT
else submitters mode
SummitSpeakersListPage->>getSelectedSubmittersActivityCount: dispatch
getSelectedSubmittersActivityCount->>getRequest: GET /submitters/all/events/count with filter[]
getRequest-->>getSelectedSubmittersActivityCount: response.count
getSelectedSubmittersActivityCount->>summit_submitters_list_reducer: dispatch RECEIVE_SELECTED_SUBMITTERS_ACTIVITY_COUNT
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| const { selectedCount: currentCount } = this.props[subjectPropKey]; | ||
| const { selectedCount: prevCount } = prevProps[subjectPropKey]; | ||
|
|
||
| if (currentCount !== prevCount) { |
There was a problem hiding this comment.
componentDidUpdate only triggers getSelectedActivityCount() when selectedCount changes, but getSelectedSpeakersActivityCount (and the submitters variant) passes the active filters to the API. If the user has speakers selected and then changes a filter (track, selection plan, activity type, etc.), the list reloads and the filter context changes — but selectedCount stays the same, so no refresh fires. The displayed activity count will be stale until the next selection change.
Consider also diffing the active filter fields from prevProps[subjectPropKey] vs the current ones and triggering the count fetch when any of them changes.
There was a problem hiding this comment.
This is not true @smarcet . REQUEST_SPEAKERS_BY_SUMMIT called when filters change is resetting the selectedCount, so the selectedActivityCount will reset too
| const activityCount = selectedCount === 0 ? 0 : totalActivities; | ||
| dispatch( | ||
| createAction(RECEIVE_SELECTED_SPEAKERS_ACTIVITY_COUNT)({ | ||
| response: { count: activityCount } |
There was a problem hiding this comment.
The short-circuit here dispatches RECEIVE_SELECTED_SPEAKERS_ACTIVITY_COUNT synchronously (without a preceding REQUEST_SELECTED_SPEAKERS_ACTIVITY_COUNT). If a previous API call — triggered by a partial selection a moment earlier — is still in-flight when this runs, that HTTP response will arrive later and dispatch RECEIVE_SELECTED_SPEAKERS_ACTIVITY_COUNT again, silently overwriting the correct totalActivities with the stale partial count.
Same pattern exists in getSelectedSubmittersActivityCount. A simple fix is a request generation counter in state: increment on each REQUEST dispatch and ignore RECEIVE payloads that carry a stale generation.
| )(params)(dispatch); | ||
| }; | ||
|
|
||
| export const getSelectedSubmittersActivityCount = |
There was a problem hiding this comment.
getSelectedSubmittersActivityCount is a near-verbatim copy of getSelectedSpeakersActivityCount — same 70 lines, same logic, only the state key (currentSummitSubmittersListState vs currentSummitSpeakersListState) and the endpoint segment (submitters vs speakers) differ. Any fix applied to one (e.g. the filter-change staleness above) has to be manually mirrored in the other.
Consider extracting a shared action factory parameterized by state key and endpoint, and having both exports delegate to it.
There was a problem hiding this comment.
I agree , but this would be an improvement to the whole submitters vs speakers action file, imo out of scope for this task and rather work on it on a separate ticket
There was a problem hiding this comment.
Pull request overview
This PR adds “live” activity totals for the currently selected speakers/submitters in the Summit Speakers/Submitters list UI, replacing the previous “no activities” summary path and introducing dedicated Redux state/actions to fetch and display selected-activity counts.
Changes:
- Add Redux state + actions for fetching selected speakers/submitters activity counts.
- Update Speakers/Submitters list UI to display selected activity totals (with a loading placeholder).
- Remove the now-unused
items_qty_no_activitiesi18n strings and standardize on the single summary string.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/reducers/summit_submitters/summit-submitters-list-reducer.js | Adds state/actions handling for selected activity count (plus minor whitespace). |
| src/reducers/summit_speakers/summit-speakers-list-reducer.js | Adds state/actions handling for selected activity count. |
| src/pages/summit_speakers/summit-speakers-list-page.js | Triggers selected-activity count refresh on selection changes; updates selected-items summary rendering. |
| src/i18n/en.json | Removes the “no activities” summary keys; keeps a single count-based string. |
| src/actions/submitter-actions.js | Adds thunk to fetch selected submitters activity count using current filters/selection. |
| src/actions/speaker-actions.js | Adds thunk to fetch selected speakers activity count using current filters/selection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| componentDidUpdate(prevProps) { | ||
| const { source } = this.state; | ||
| const subjectPropKey = | ||
| source === sources.speakers ? "speakersProps" : "submittersProps"; | ||
| const { selectedCount: currentCount } = this.props[subjectPropKey]; | ||
| const { selectedCount: prevCount } = prevProps[subjectPropKey]; | ||
|
|
||
| if (currentCount !== prevCount) { | ||
| this.getSelectedActivityCount(); | ||
| } | ||
| } |
| return getRequest( | ||
| createAction(REQUEST_SELECTED_SPEAKERS_ACTIVITY_COUNT), | ||
| createAction(RECEIVE_SELECTED_SPEAKERS_ACTIVITY_COUNT), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/speakers/all/events/count`, | ||
| authErrorHandler | ||
| )(params)(dispatch); |
| return getRequest( | ||
| createAction(REQUEST_SELECTED_SUBMITTERS_ACTIVITY_COUNT), | ||
| createAction(RECEIVE_SELECTED_SUBMITTERS_ACTIVITY_COUNT), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/submitters/all/events/count`, | ||
| authErrorHandler | ||
| )(params)(dispatch); |
| import { buildSpeakersSubmittersList } from "../utils/methods"; | ||
|
|
||
|
|
||
|
|
||
| const DEFAULT_STATE = { |
…998) Keep the branch current with the master tip per smarcet's request (Core Team Sync 2026-06-26). Picks up 350c73d (#998 speaker-list activity count); clean auto-merge, en.json edits are in non-overlapping regions. No sponsor-reports files affected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
https://app.clickup.com/t/9014802374/86bajeea2
Summary by CodeRabbit
New Features
Bug Fixes
Documentation