Skip to content

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented Jun 12, 2024

Replaces &Option<UserListParams> with &UserListParams & &Option<PluginListParams> with &PluginListParams.

All fields in UserListParams & PluginListParams are optional, so this is mostly a syntax sugar change. I think using Option<&T> for these types have better semantics, because it tells the user that this params might or might not be there and if it's there, it's a reference to it. However, we are unable to use that with uniffi and have to opt for &Option<T> which has both different semantics and different handling rules for it.

I had reluctantly implemented it with &Option<T> back when I was first working on this, but it has been bugging me all this time. Now that I am working on generating these endpoints and request builders, this became more than a minor annoyance as I'd have to juggle between &Option<T> and Option<&T> from request builder to endpoint types.

Using UserListParams & PluginListParams is a good middle ground solution for this and will let me work on generating the request builders without the extra friction.


Somewhat unrelated to this PR, I've added test cases for empty fields for filter users & filter plugins. I was just trying to make sure we were handling empty UsersListParams & PluginListParams and ended up adding these cases. Surprisingly, I realized that the server will return all fields if the _fields= is empty. I am not entirely sure if we are handling this case properly at the moment, but I think adding these tests will help make sure we pay attention to it and maybe iterate on it at a later time.


To Test

  • make test-server && make dump-mysql && make backup-wp-content-plugins
  • cargo test --test '*' -- --nocapture --test-threads 1
  • cd native/kotlin && ./gradlew :api:kotlin:integrationTest cAT

@oguzkocer oguzkocer added the Rust label Jun 12, 2024
@oguzkocer oguzkocer added this to the 0.1 milestone Jun 12, 2024
@oguzkocer oguzkocer force-pushed the remove_ref_option_t branch from 981047d to 8676602 Compare June 12, 2024 18:33
@oguzkocer oguzkocer marked this pull request as ready for review June 12, 2024 18:35
@oguzkocer oguzkocer merged commit 9127ef2 into trunk Jun 12, 2024
@oguzkocer oguzkocer deleted the remove_ref_option_t branch June 12, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants