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

Cast bill action dates to date objects, not date time objects #272

Merged
merged 7 commits into from
Sep 15, 2020

Conversation

hancush
Copy link
Member

@hancush hancush commented Sep 15, 2020

Description

Bill action dates come to us as date strings. Originally, we cast them to date times, but this caused issues, as described in Metro-Records/la-metro-councilmatic#638.

This PR:

  • Casts bill action date strings to date objects.
  • Updates last_action_date to a DateField.
  • Adds a to_datetime util to coerce these date objects to date time objects where needed, predominantly in feeds.py.

We should treat this as a breaking change, in case downstream instances do anything with the last action date, e.g., in the Chicago search index definition.

Also, we should do this for memberships, too, but I want to do that separately for the sake of a neat diff and an expedient fix for action dates, which are displayed in lots of places, now. Opened #273 to track that work.

Testing instructions

N.b., I've already done this.

  • Spin up a local version of Metro Councilmatic. View a board report with actions, noting the date.
  • Install this version of django-councilmatic into a local install of Metro Councilmatic. Refresh the board report page and confirm that the date is one day later, i.e., the correct date. (You can verify this by calling up the board report in a Python shell.)

@hancush hancush changed the title Cast dates to dates, not date times Cast bill action dates to date objects, not date time objects Sep 15, 2020
@hancush hancush requested a review from fgregg September 15, 2020 15:53
Copy link
Member

@fgregg fgregg left a comment

Choose a reason for hiding this comment

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

looks good, one question.

return cls._cast(field, models.DateField())

@classmethod
def cast_to_datetime(cls, field):
Copy link
Member

Choose a reason for hiding this comment

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

do we need this path around?

Copy link
Member Author

@hancush hancush Sep 15, 2020

Choose a reason for hiding this comment

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

Good question! We're still using it for event start dates, as well as membership start and end dates. We'll eventually transition membership start/end dates to date objects, but events actually come with a date time string, so we'll still use it there.

Copy link
Member

Choose a reason for hiding this comment

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

ty.

@fgregg fgregg self-requested a review September 15, 2020 16:25
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