Skip to content

Filter users #70

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

Merged
merged 146 commits into from
May 2, 2024
Merged

Filter users #70

merged 146 commits into from
May 2, 2024

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented Apr 30, 2024

Addresses #29.

This PR implements fields based filtering for /users endpoint, including list, retrieve and retrieve_me variants. It adds a SparseUserField enum and 3 new request builders that take a slice of these fields.

It adds quite a few integration tests using rstest, but it takes less than a second to run all of them, and they give us the peace of mind that filtering is working as expected, so I think it's well worth it.

Note that all tests use the Edit context as they test every single field type and not all fields are available for all contexts. We could add specific tests for Embed & View contexts, but I am not sure there is too much value in that. Let me know what you think!

oguzkocer added 30 commits April 3, 2024 13:06
@oguzkocer oguzkocer added this to the 0.1 milestone Apr 30, 2024
Base automatically changed from users_integration_tests to trunk May 1, 2024 19:12
@oguzkocer oguzkocer marked this pull request as ready for review May 1, 2024 19:17
@crazytonyli
Copy link
Contributor

Is there any plan supporting _fileds in the rest functions(update, create, etc)?

@oguzkocer
Copy link
Contributor Author

Is there any plan supporting _fields in the rest functions(update, create, etc)?

I don't think so. What do you think @jkmassel?

@jkmassel
Copy link
Contributor

jkmassel commented May 2, 2024

Interesting – I guess you might just want the id back or something from a create or update?

Does that work? Have you tested it against the API?

@oguzkocer oguzkocer merged commit e555ddc into trunk May 2, 2024
@oguzkocer oguzkocer deleted the filter_users branch May 2, 2024 15:40
@oguzkocer
Copy link
Contributor Author

Does that work? Have you tested it against the API?

@jkmassel It looks like it does - at least for creating users. Would you like me to implement it?

@jkmassel
Copy link
Contributor

jkmassel commented May 2, 2024

We should support it eventually, but I'd say put it on the backlog for now – returning a whole User after creation isn't a huge bandwidth waste – the data is pretty small.

By the time we're doing Post or Comment we might want it for the savings there.

@oguzkocer
Copy link
Contributor Author

I've created a separate issue for that: #74.

@oguzkocer oguzkocer mentioned this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants