Feature/timestamps #1559

michaeljcollinsuk · 2021-08-19T11:06:04Z

This work:

Updates models that were using the Timestampable model to use TimeStampedModel for codebase consistency
Exposes the created and modified fields on the Organization, Party, and People models serializers
Allows models to be filtered by the modified date on each models list endpoint

ynr/apps/popolo/api/next/filters.py

- Update models that were using the Timestampable model to use TimeStampedModel for consistency across the codebase - Data migrations to copy accross the existing created_at values - Change references to created_at and updated_at to created and modified

- Helper to get date four weeks before election

ynr/apps/parties/api/next/serializers.py

michaeljcollinsuk · 2021-09-08T11:10:02Z

ynr/apps/people/api/next/filters.py

+        model = Person
+        fields = ["last_updated"]
+
+    last_updated = filters.DateTimeFilter(


Was having a play around with this locally, and see what you meant about the date formatting being a bit awkward. Then I came across the IsoDateTimeFilter which I think we should use instead of the DateTimeFilter - as then when we use the API in WCIVF, it will mean we can use the isoformat() helper method on a datetime, rather than having to format the date ourselves.

Should I do this for all filters expecting an ISO datetime?

Thinks its good on just the new ones for now, as dont want to break anything by changing the pre-existing ones.

symroe · 2021-09-08T11:13:51Z

I've not looked yet, but another thing to review / check here is that the documentation gets generated properly for this.

For example, the params get generated for the results endpoint here: https://candidates.democracyclub.org.uk/api/docs/next/endpoints/#results_list

But not the people one here: https://candidates.democracyclub.org.uk/api/docs/next/endpoints/#people_list

ynr/apps/people/api/next/filters.py

ynr/apps/popolo/api/next/filters.py

ynr/apps/parties/migrations/0014_add_last_updated.py

VirginiaDooley · 2021-09-09T13:44:06Z

I've not looked yet, but another thing to review / check here is that the documentation gets generated properly for this.

For example, the params get generated for the results endpoint here: https://candidates.democracyclub.org.uk/api/docs/next/endpoints/#results_list

But not the people one here: https://candidates.democracyclub.org.uk/api/docs/next/endpoints/#people_list

Working as expected ✅

michaeljcollinsuk · 2021-09-09T16:25:23Z

ynr/apps/api/next/filters.py

+from django_filters import filterset, filters
+
+
+class LastUpdatedFilter(filterset.FilterSet):


Sorry one last comment! So this is the case where you would use Mixin in the name - as you wouldn't use this on its own, its purpose is to be inherited elsewhere

That makes sense, will change.

michaeljcollinsuk

I can't approve this as since I opened the PR myself! But added one last minor comment about naming, otherwise think this is good to be merged. I know there is more work to do, specifically about when/how we ensure the modified value is up to date when a child relation is updated, but I think that can be tackled outside of this PR.

michaeljcollinsuk · 2021-09-13T10:07:59Z

...I know there is more work to do, specifically about when/how we ensure the modified value is up to date when a child relation is updated, but I think that can be tackled outside of this PR.

Just added a thought about this to related issue in WCIVF

symroe

One minor comment, otherwise 👍

symroe · 2021-09-13T14:25:17Z

ynr/apps/people/api/next/serializers.py

-        fields = ("contact_type", "label", "note", "value")
+        fields = ("contact_type", "label", "note", "value", "created")
+
+        last_updated = serializers.DateTimeField(source="modified")


Does this need to be in the fields tuple?

VirginiaDooley self-assigned this Aug 30, 2021

VirginiaDooley force-pushed the feature/timestamps branch 4 times, most recently from 0d18b6f to 8741ae1 Compare September 6, 2021 08:55

michaeljcollinsuk commented Sep 6, 2021

View reviewed changes

ynr/apps/popolo/api/next/filters.py Outdated Show resolved Hide resolved

VirginiaDooley force-pushed the feature/timestamps branch from 8741ae1 to 897ae18 Compare September 6, 2021 12:08

VirginiaDooley changed the title ~~WIP Feature/timestamps~~ Feature/timestamps Sep 6, 2021

VirginiaDooley marked this pull request as ready for review September 6, 2021 12:10

VirginiaDooley force-pushed the feature/timestamps branch from 897ae18 to ff4b3f5 Compare September 6, 2021 12:22

michaeljcollinsuk and others added 4 commits September 6, 2021 13:35

Add timestamps to Ballot with data migrations

6bd3661

- Helper to get date four weeks before election

Add last_updated field to serializers

fef85b1

Note v0.9 as legacy code

230c0a1

VirginiaDooley force-pushed the feature/timestamps branch from ff4b3f5 to e56013c Compare September 6, 2021 12:36

michaeljcollinsuk commented Sep 8, 2021

View reviewed changes

ynr/apps/parties/api/next/serializers.py Show resolved Hide resolved

michaeljcollinsuk commented Sep 8, 2021

View reviewed changes

ynr/apps/people/api/next/filters.py Outdated Show resolved Hide resolved

michaeljcollinsuk commented Sep 8, 2021

View reviewed changes

ynr/apps/popolo/api/next/filters.py Outdated Show resolved Hide resolved

VirginiaDooley reviewed Sep 8, 2021

View reviewed changes

ynr/apps/parties/migrations/0014_add_last_updated.py Outdated Show resolved Hide resolved

VirginiaDooley force-pushed the feature/timestamps branch 2 times, most recently from e5b9d94 to 1bcbd68 Compare September 9, 2021 13:43

VirginiaDooley force-pushed the feature/timestamps branch 2 times, most recently from 5a122f4 to 3bd5a6c Compare September 9, 2021 14:14

VirginiaDooley added 2 commits September 9, 2021 15:52

Add filter by last_updated

4b36e81

Fix static tag error

7d551a6

VirginiaDooley force-pushed the feature/timestamps branch from 3bd5a6c to 7d551a6 Compare September 9, 2021 14:52

michaeljcollinsuk commented Sep 9, 2021

View reviewed changes

Rename Filter to Mixin

d01ba72

VirginiaDooley requested a review from symroe September 10, 2021 11:26

symroe approved these changes Sep 13, 2021

View reviewed changes

Remove created from fields tuple

c79c39d

VirginiaDooley merged commit c106177 into master Sep 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/timestamps #1559

Feature/timestamps #1559

michaeljcollinsuk commented Aug 19, 2021 •

edited by VirginiaDooley

Loading

michaeljcollinsuk Sep 8, 2021

VirginiaDooley Sep 9, 2021

michaeljcollinsuk Sep 9, 2021

symroe commented Sep 8, 2021

VirginiaDooley commented Sep 9, 2021

michaeljcollinsuk Sep 9, 2021

VirginiaDooley Sep 10, 2021

michaeljcollinsuk left a comment

michaeljcollinsuk commented Sep 13, 2021

symroe left a comment

symroe Sep 13, 2021

		from django_filters import filterset, filters


		class LastUpdatedFilter(filterset.FilterSet):

Feature/timestamps #1559

Feature/timestamps #1559

Conversation

michaeljcollinsuk commented Aug 19, 2021 • edited by VirginiaDooley Loading

michaeljcollinsuk Sep 8, 2021

Choose a reason for hiding this comment

VirginiaDooley Sep 9, 2021

Choose a reason for hiding this comment

michaeljcollinsuk Sep 9, 2021

Choose a reason for hiding this comment

symroe commented Sep 8, 2021

VirginiaDooley commented Sep 9, 2021

michaeljcollinsuk Sep 9, 2021

Choose a reason for hiding this comment

VirginiaDooley Sep 10, 2021

Choose a reason for hiding this comment

michaeljcollinsuk left a comment

Choose a reason for hiding this comment

michaeljcollinsuk commented Sep 13, 2021

symroe left a comment

Choose a reason for hiding this comment

symroe Sep 13, 2021

Choose a reason for hiding this comment

michaeljcollinsuk commented Aug 19, 2021 •

edited by VirginiaDooley

Loading