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

Remove /terms endpoint and update filter fields #189

Merged
merged 10 commits into from
Feb 18, 2022

Conversation

HaSistrunk
Copy link
Member

Fixes #188 and #187

I think I've gone too far in removing /terms here? This is probably something that we'll want to add back in in the future when data is better, and maybe I shouldn't be trying to take all of this out, but just keep it out of the browseable API?

Also, as evidenced by the failing tests I'm having trouble extracting this gracefully. I'd appreciate some help focusing what I should be trying to do to solve issue #187 .

@HaSistrunk HaSistrunk requested a review from helrond February 17, 2022 18:43
@helrond
Copy link
Member

helrond commented Feb 17, 2022

I think I would suggest leaving the Views and Serializers in place, but removing the URL from the router and templates (as you've done). That should resolve the failing tests and also leave that code in place for when we need it, which might be sooner than we think...

@HaSistrunk HaSistrunk marked this pull request as ready for review February 17, 2022 21:28
@HaSistrunk HaSistrunk merged commit 28780a2 into development Feb 18, 2022
@HaSistrunk HaSistrunk deleted the template_updates branch February 18, 2022 13:50
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