Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Significant Date facet and add Fiscal Year facet #727

Merged
merged 11 commits into from
Sep 3, 2021

Conversation

fgomez828
Copy link
Contributor

@fgomez828 fgomez828 commented Jun 1, 2021

Overview

See title

Checklist

  • PR has a descriptive enough title to be useful in changelogs

Testing Instructions

[for @fatima3558 to do]

  • Deploy to staging by shelling into the lametro server
  • run the refresh_guid command on the staging server
  • visit the staging site and check that the Significant Date facet is properly populated

Handles #716
Handles #370

@fgomez828 fgomez828 changed the title [WIP] make Date facet singular fix facet in refresh_guid to populate Significant Date facet Jun 1, 2021
@fgomez828 fgomez828 changed the title fix facet in refresh_guid to populate Significant Date facet fix Significant Date facet and add Fiscal Year facet Jun 2, 2021
Copy link
Collaborator

@hancush hancush left a comment

Choose a reason for hiding this comment

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

Thanks for your tenacity on this, @fatima3558. The new code is in the right place – the logic just needs a little revision!

Let's chat at the start of your next Metro block. I want to sync up on some of the finer (and undocumented 🙀) parts of search indexing before you bring this over the line.

lametro/management/commands/refresh_guid.py Outdated Show resolved Hide resolved
lametro/models.py Outdated Show resolved Hide resolved
@@ -21,6 +21,7 @@ class LAMetroBillIndex(BillIndex, indexes.Indexable):
significant_date = indexes.MultiValueField(faceted=True)
motion_by = indexes.MultiValueField(faceted=True)
plan_program_policy = indexes.MultiValueField(faceted=True)
legislative_session = indexes.MultiValueField(faceted=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already defined on the base index in django-councilmatic – why redefine here?

lametro/search_indexes.py Outdated Show resolved Hide resolved
lametro/search_indexes.py Outdated Show resolved Hide resolved
Comment on lines 55 to 56
most_recent = sorted(obj.actions_and_agendas, key=lambda i: i['date'],reverse=True)[0]
action_date = most_recent['event'].start_time
Copy link
Collaborator

@hancush hancush Jun 11, 2021

Choose a reason for hiding this comment

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

This isn't quite right. The desired logic is to return the session associated with the most recent agenda date. If there are no agenda dates, return the session associated with the most recent action date. If there is neither an agenda date nor an action date, return None.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you propose how you'd go about getting the desired date, using the output of obj.actions_and_agendas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I'm thinking about it, I would have to add additional logic to check if the most_recent event is an agenda (description being SCHEDULED) or an action. I would use the agenda if possible, otherwise using an action as the most_recent.

Either way, I can use most_recent['date'], unless a datetime instance is preferable to a date instance for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hancush am I on the right track with this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd recommend something like:

actions_and_agendas = sorted(obj.actions_and_agendas, key=lambda i: i['date'], reverse=True)
agendas = [item for item in actions_and_agendas if it is an agenda]  # will be sorted bc list order is preserved in python
if agendas:
    # return the most recent date from your agendas list
else:
    # there are no agendas, i.e., everything in actions_and_agendas is an action
    # return the most recent date from your actions_and_agendas list

lametro/search_indexes.py Outdated Show resolved Hide resolved
@hancush hancush dismissed their stale review September 3, 2021 19:18

Stale review

@hancush hancush merged commit 5caa4e4 into master Sep 3, 2021
hancush added a commit that referenced this pull request Sep 7, 2021
@hancush hancush deleted the significant_date-facet branch February 11, 2022 16:24
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