Skip to content

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented May 22, 2024

As we discussed on Slack, it seems /users endpoint doesn't always return the avatar_urls field as promised in the documentation.
Screenshot 2024-05-22 at 18 38 46

Unfortunately, I haven't foreseen the possibility of adding Option<T> to our generated contextual types such as UserWithEditContext when I worked on wp_contextual. This PR adds a new field attribute WPContextualOption which will leave the type of the field as is - without any modification. Just as a reminder, with WPContext and WPContextualField attributes, the type was always modified. (by removing the Option wrapper & Sparse prefix respectively)

This new attribute should be used alongside WPContext as without it, the field wouldn't be included in the contextual type in the first place. It also can not be used with WPContextualField because while WPContextualField will remove the Sparse prefix from the type, WPContextualOption will leave it as-is. So, they are incompatible.

All these cases are handled with proper errors, and integration tested. The documentation is also updated.

Before reviewing the implementation, I suggest taking a look at the new integration tests and the documentation to better understand the changes.


To Test

  • make test-server && make dump-mysql && make backup-wp-content-plugins
  • cargo test --test '*' -- --nocapture --test-threads 1

@oguzkocer oguzkocer added the Rust label May 22, 2024
@oguzkocer oguzkocer added this to the 0.1 milestone May 22, 2024
@oguzkocer oguzkocer requested review from crazytonyli and jkmassel May 22, 2024 22:44
@oguzkocer
Copy link
Contributor Author

@jkmassel @crazytonyli I'll update the PR with the base branch once it's reviewed and approved. There is a lot of traffic in trunk at the moment, so it'll likely go out of date soon anyway.

@oguzkocer oguzkocer marked this pull request as ready for review May 22, 2024 22:47
@oguzkocer oguzkocer force-pushed the wp-contextual-option branch from b03137b to 3b13323 Compare May 23, 2024 15:04
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

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

I only understand about 80% of this, but the tests are very understandable (as is the new branch for is_wp_contextual_option_ident so I'm confident this will be solid.

Feel free to merge when convenient for you

@jkmassel jkmassel enabled auto-merge (squash) May 23, 2024 21:29
@jkmassel jkmassel force-pushed the wp-contextual-option branch from 3b13323 to e2b539d Compare May 23, 2024 21:29
@jkmassel jkmassel merged commit 8c3cb43 into trunk May 23, 2024
@jkmassel jkmassel deleted the wp-contextual-option branch May 23, 2024 21:57
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.

2 participants