fix: annotate conference list with has_errors, has_warnings, participants_count#22
Conversation
…ants_count After expand_fields was removed (PR peermetrics#19), the conference list no longer includes participants or issues arrays. This broke the dashboard charts for error/warning icons and participant counts. Adds lightweight annotations via Exists() subqueries and Count() so the list includes: - has_errors (boolean) - has_warnings (boolean) - participants_count (integer) No N+1 queries — all computed in the same SQL query.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8f7df3394
ℹ️ 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".
| has_warnings=Exists( | ||
| Issue.objects.filter(conference=OuterRef('pk'), type='w', is_active=True) | ||
| ), | ||
| participants_count=Count('participants', distinct=True), |
There was a problem hiding this comment.
When participantId is present, Conference.filter(**filters) introduces a join on participants constrained to that specific participant, and Count('participants', distinct=True) then counts only that filtered join row(s). In that path, participants_count no longer reflects the conference total (often returning 1), which regresses the participant-count chart for participant-scoped queries. This needs an aggregation that is independent of the request’s participant filter (e.g., a separate subquery over the M2M table).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good approach. Annotations are the right call here.
One bug codex found: participants_count will be wrong when participantId is in the query string. Django reuses the filtered JOIN on the M2M table, so Count('participants') only counts the one filtered participant (returns 1 instead of the real total).
has_errors/has_warnings don't have this problem because Exists() runs its own subquery.
Claude suggested fix: use a subquery for the count too:
participants_count=Subquery(
Participant.objects.filter(
conferences=OuterRef('pk')
).order_by().values('conferences').annotate(
cnt=Count('id', distinct=True)
).values('cnt')[:1],
output_field=IntegerField(),
)
Count('participants') reuses the filtered JOIN when participantId is in
the query string, returning 1 instead of the real total. Switched to a
Subquery that counts independently of the request's participant filter.
Addresses review feedback from Codex and Alberto.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 638cab3556
ℹ️ 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".
| Participant.objects.filter( | ||
| conferences=OuterRef('pk') |
There was a problem hiding this comment.
The participants_count subquery uses Participant.objects.filter(...), which bypasses the model’s Participant.filter(...) wrapper that enforces is_active=True. This means soft-deleted participants are still included in participants_count, so conference list metrics can be overstated after participant deletions. Previously, participant expansion paths used the wrapper and implicitly excluded inactive rows, so this changes API semantics for dashboards that rely on active participant totals.
Useful? React with 👍 / 👎.
The Subquery used Participant.objects.filter() which bypasses the is_active=True check, overcounting if any participants were soft-deleted.
2 participants
Problem
After expand_fields was removed from the conference list (PR #19), the dashboard lost:
Fix
Annotate the conference queryset with lightweight fields computed in the same SQL query:
No N+1 queries — all computed as subqueries/aggregations in one query.
API response
{ "conference_id": "sessionshealth-abc123", "has_errors": false, "has_warnings": true, "participants_count": 2, ... }Test plan
participants_count=2in API response