Skip to content

add current filter to leaflets endpoint - #431

Open
chris48s wants to merge 1 commit into
masterfrom
currentfilter20260812
Open

add current filter to leaflets endpoint#431
chris48s wants to merge 1 commit into
masterfrom
currentfilter20260812

Conversation

@chris48s

Copy link
Copy Markdown
Member

Refs https://app.asana.com/1/1204880536137786/project/1204880927741389/task/1217118151808079?focus=true

LLM assisted

The definition of "current" in this context is:

  • The leaflet is attached to one or more ballots that are happening in the future
  • The leaflet is attached to one or more ballots that happened within the last 20 days
  • Leaflets which are attached to zero ballots are not current
  • Leaflets which are attached only to ballots that happened more than 20 days ago are not current

I think that's about the best we can do.
This will also want a follow up PR to consume it on the WCIVF side.

Comment on lines +30 to +42
is_current = RawSQL(
"""
EXISTS (
SELECT 1
FROM jsonb_array_elements(ballots) AS ballot
WHERE substring(
ballot->>'ballot_paper_id' from '\\d{4}-\\d{2}-\\d{2}$'
)::date >= %s
)
""",
(current_from,),
output_field=BooleanField(),
)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the easiest to read query. However we don't store the poll date in the JSON as a standalone field so in order to do this efficiently we have to parse it out of the ballot ids with regex.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it might end up using a lot of CPU. Can we investigate either:

  1. Also filtering on the upload date...anything that was uploaded more than....a year ago can't be in a current election, I assume, so we can eliminate the majority of the data in the database that's being parsed for JSON? (I assume the postgres planner will figure this out)
  2. Add a date field to the leaflet model: more work and maybe not an ideal direction of travel, but would make this a lot faster.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I will have to come back to this one after the move, but..

  1. I like the idea of filtering on upload date to reduce the initial search space more 👍

  2. At the moment, a Leaflet object has a JSONField called ballots, which is an array of (ballot) objects with properties ballot_paper_id and.. something else. The name or title of the ballot basically. This means that in principle a Leaflet can be related to ballots taking place on multiple different dates. I expect this would be irregular, but it is possible.

When you say "Add a date field to the leaflet model", do you mean storing a single date on the Leaflet object (in which case, we have to decide what to do in the case I just described) or do you mean change the shape of the JSON object so that each ballot in the array also has an explicit poll_date? The second one would still need a deep json object query, but should be more efficient than parsing the ballot id with regex.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to leave this until later. On dates: understood, but I think we can use ArrayField(models.DateField()) and still query faster than regexing JSON.

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