-
-
Notifications
You must be signed in to change notification settings - Fork 348
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
[feature] Implement Filter API v2 #2936
[feature] Implement Filter API v2 #2936
Conversation
Wow! Thanks for this PR :D Re: those |
@tsmethurst sounds good. I'll remove that feature from this PR and we can resurrect it later if necessary. |
Great PR! I have only a few small comments and questions about error handling + return values. Once those are resolved this can be squerged imo. Excellent work, thank you for this :) |
Great stuff! |
* Use correct entity name * We support server-side filters now * Document filter v1 methods that can throw a 409 * Validate v1 filter phrase as filter title * Always check v1 filter API status codes in tests * Document keyword minimum requirement on filter API v1 * Make it possible to specify filter keyword update columns per filter keyword * Implement v2 filter API * Fix lint and tests * Update Swagger spec * Fix filter update test * Update Swagger spec *correctly* * Update actual files Swagger spec was generated from * Remove keywords_attributes and statuses_attributes * Add test for serialization of empty filter * More helpful messages when object is owned by wrong account
* Use correct entity name * We support server-side filters now * Document filter v1 methods that can throw a 409 * Validate v1 filter phrase as filter title * Always check v1 filter API status codes in tests * Document keyword minimum requirement on filter API v1 * Make it possible to specify filter keyword update columns per filter keyword * Implement v2 filter API * Fix lint and tests * Update Swagger spec * Fix filter update test * Update Swagger spec *correctly* * Update actual files Swagger spec was generated from * Remove keywords_attributes and statuses_attributes * Add test for serialization of empty filter * More helpful messages when object is owned by wrong account
Description
This pull request implements the Mastodon v2 filter API. v2 allows filters to be grouped and named, but the most important addition is the ability to filter individual statuses, which is not possible with v1. Want to hide an inflammatory, irrelevant, or unfunny post that your friends keep boosting? Now you can.
This is a follow-up to #2793, which implemented server-side filtering but only exposed it through the v1 API.
It also fixes several tests and minor bugs in the v1 filter API.
Impact
Phanpy only supports v2 filters, so this should make Phanpy more useful as a GtS frontend. (Tusky seems to be able to fall back to v1 if v2 isn't available, Feditext doesn't support v2 yet feditext/feditext#57, and Semaphore is EOL and only supports v1.)
Open questions
This version implements the
keywords_attributes
param for creating and updating filter keywords in the same API call as creating or updating a filter, and extends this to filter statuses withstatuses_attributes
parameters.However, this functionality doesn't seem to be very well thought out. (More detail in https://princess.industries/@vyr/statuses/01HXDEFGK0W8R4M4X7TKB2RKNJ which is an unlisted post on a GtS instance so open that link in your Fedi client.) It duplicates separate calls for creating and updating filter keywords, I've had to make a lot of assumptions implementing it because it's based on a poorly documented Rails feature which doesn't map naturally to form parameters, and I haven't found a client that uses it to test against. Tusky actually has a comment suggesting that they avoided it on purpose:
So maybe it'd be smarter to omit
keywords_attributes
andstatuses_attributes
, on the basis that they can't be incompatible with the Mastodon implementation if we don't ship them at all. However, I'd like to hear from client devs and make sure nobody is expecting to use them first.Resolved: we're dropping these.
Checklist
go fmt ./...
andgolangci-lint run
.