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

For MPP-1639: Add RelayAddress.used_on field #1713

Merged
merged 2 commits into from
Apr 15, 2022

Conversation

groovecoder
Copy link
Member

@groovecoder groovecoder commented Mar 30, 2022

This PR is for MPP-1639.

How to test:

  1. python manage.py migrate
  2. http://127.0.0.1:8000/api/v1/relayaddresses/
    • You should see a new used_on field
  3. Use the django admin site to add some comma-separated domains into the new used_on field of an address. (e.g., disneyplus.com,netflix.com
  4. Go to http://127.0.0.1:8000/api/v1/relayaddresses/?used_on=disney
    • You should see the alias matching "disney" in the list!
  5. Go to http://127.0.0.1:8000/api/v1/relayaddresses/?used_on=hulu
    • You should NOT see the alias matching "disney" in the list!

@netlify
Copy link

netlify bot commented Apr 1, 2022

Deploy Preview for fx-relay-demo canceled.

Name Link
🔨 Latest commit f6da28c
🔍 Latest deploy log https://app.netlify.com/sites/fx-relay-demo/deploys/6259cacc9a9c4e0009e18d3d

@groovecoder groovecoder force-pushed the start-used_on-field-MPP-1639 branch 2 times, most recently from 591dc8c to fd6a53a Compare April 1, 2022 20:37
@groovecoder
Copy link
Member Author

Note: I updated this PR to add django-filter library so we can filter API list views on ANY of the model fields by adding a filterset_class to any ModelViewSet.

E.g.:

django-filter can allow any kind of url parameter matching described in the Django "Field lookups" docs - exact, partial/contains, greater/less-than (numbers or dates), etc.

So, are there other API endpoints we could add some of the url param lookups to simplify code on the front-end?

Add django-filter for API-wide filtering

more wip
@maxxcrawford maxxcrawford force-pushed the start-used_on-field-MPP-1639 branch from fd6a53a to a5ca9f9 Compare April 6, 2022 14:40
@groovecoder groovecoder requested a review from jwhitlock April 15, 2022 15:26
emails/models.py Outdated Show resolved Hide resolved
Copy link
Member

@jwhitlock jwhitlock left a comment

Choose a reason for hiding this comment

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

Thanks @groovecoder, the tests steps worked. I've got one question and one field type suggestion - see in-line comments.

api/serializers.py Show resolved Hide resolved
emails/models.py Outdated Show resolved Hide resolved
@groovecoder groovecoder force-pushed the start-used_on-field-MPP-1639 branch from 0c55b01 to f6da28c Compare April 15, 2022 19:43
@groovecoder
Copy link
Member Author

Changed to TextField. The field is written by the add-on, so it can't be in read_only_fields.

Copy link
Member

@jwhitlock jwhitlock left a comment

Choose a reason for hiding this comment

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

Thanks @groovecoder! 👏 good job re-writing the migration! (watch out @maxxcrawford if you migrated)

@groovecoder groovecoder merged commit 12019d3 into main Apr 15, 2022
@groovecoder groovecoder deleted the start-used_on-field-MPP-1639 branch April 15, 2022 20:45
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.

3 participants