Skip to content

fix: correct PollIsEndedFilter logic and showcase page poll display#114

Merged
imorland merged 5 commits intoFriendsOfFlarum:1.xfrom
iPurpl3x:fix/poll-showcase-ended-filter
Apr 15, 2026
Merged

fix: correct PollIsEndedFilter logic and showcase page poll display#114
imorland merged 5 commits intoFriendsOfFlarum:1.xfrom
iPurpl3x:fix/poll-showcase-ended-filter

Conversation

@iPurpl3x
Copy link
Copy Markdown
Contributor

Summary

  • Fix inverted negate/non-negate branches in PollIsEndedFilter, and scope the orWhereNull inside a closure to prevent query leakage
  • Split the showcase page into two separate API requests (filter[-isEnded]=1 for active, filter[isEnded]=1 for ended) instead of fetching a single combined list capped at 20 and filtering client-side
  • Add integration tests for the GET /api/fof/polls endpoint covering the isEnded filter
  • Fix missing settings column in DeletePollGroupTest fixture data

Problem

The polls showcase page (/polls) was not displaying all active polls. Two bugs contributed:

  1. PollIsEndedFilter had its $negate branches swapped — filter[isEnded]=1 returned active polls and filter[-isEnded]=1 returned ended polls. Additionally, orWhereNull('end_date') was not wrapped in a closure, causing it to match unrelated rows.

  2. PollsShowcasePage fetched a single unfiltered list of all polls (hardcoded page size of 20), then split them client-side using hasEnded(). When more than 20 total polls existed, active polls beyond position 20 were silently dropped.

Changes

File Change
src/Filter/PollIsEndedFilter.php Swap negate branches; scope orWhereNull in closure
js/src/forum/components/PollsShowcasePage.tsx Two PollListState instances with server-side filters
js/src/forum/components/Poll/PollShowcase.tsx Accept separate activeState/endedState; add Load More for ended polls
tests/integration/api/ListGlobalPollsTest.php 6 new integration tests for isEnded filter
tests/integration/api/AbstractPollGroupTestCase.php Add settings column to poll fixture

Test plan

  • All 55 integration tests pass (including 6 new + 3 previously broken)
  • Manual verification: /polls page shows all active polls in "Active" section and all ended polls in "Ended" section
  • Verify two separate API requests in browser devtools (filter[-isEnded]=1 and filter[isEnded]=1)

iPurpl3x and others added 4 commits February 19, 2026 19:08
The filter had two bugs:
- The negate/non-negate branches were swapped, causing
  filter[isEnded]=1 to return active polls and vice versa.
- The orWhere for null end_date was not scoped inside a
  closure, leaking into the outer query and returning
  unrelated rows.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously the showcase page fetched a single list of all polls
(capped at 20) and split them client-side using hasEnded(). When
more than 20 polls existed, active polls beyond position 20 were
silently dropped.

Now the page makes two separate filtered API requests — one for
active polls (filter[-isEnded]=1) and one for ended polls
(filter[isEnded]=1) — ensuring both sections display all results
independently. Adds a "Load More" button for ended polls pagination.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds ListGlobalPollsTest with 6 test cases covering the isEnded
filter on the GET /api/fof/polls endpoint:
- List all global polls without filter
- Filter ended polls (isEnded=1)
- Filter active polls (-isEnded=1)
- Verify null end_date included in active results
- Verify future end_date included in active results
- Verify null/future end_date excluded from ended results

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The polls table has a CHECK constraint requiring the settings
column to be valid JSON. The test fixture was missing this column,
causing all DeletePollGroupTest tests to fail with a constraint
violation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@iPurpl3x iPurpl3x requested a review from a team as a code owner February 19, 2026 13:43
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@imorland imorland merged commit 557556c into FriendsOfFlarum:1.x Apr 15, 2026
18 checks 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