Skip to content

Conversation

@mattwiebe
Copy link
Contributor

@mattwiebe mattwiebe commented Aug 12, 2024

The fields_attributes in api/v1/update_credentials are submitted in a multipart/form-data hash like:

# Simplifying to not show the boundary
fields_attributes[0][name]
Some Title
fields_attributes[0][value]
Some Value
fields_attributes[1][name]
Another Title
# etc

We were only handling the simple value type before.

This is required for Extra Fields editing in ActivityPub: Automattic/wordpress-activitypub#788

$data[ $name ] = $value;

// Handle different array forms.
if ( preg_match( '/^([^\[]+)\[\]$/', $name, $matches ) ) {
Copy link
Owner

Choose a reason for hiding this comment

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

I guess the ([^\[]+) is technically correct but could it be more readable to use something like ([a-zA-Z0-9_]+) for easier readability?

Copy link
Owner

Choose a reason for hiding this comment

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

Or even (\w+) as per the docs:

\w
any "word" character
A "word" character is any letter or digit or the underscore character

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it's pretty ugly! I wasn't sure how permissive to be on this. I'm also going to reduce the surface area of what this code handles: only enough to support our use-case.

I'm slowly working up a patch for Core WP that'll patch things at the REST API level and then hopefully we can strip this all out.

@mattwiebe
Copy link
Contributor Author

@akirk finally had a chance to circle back to this, thanks for your feedback, I hope it's good to go!

@mattwiebe mattwiebe changed the title PATCH routes: support array types PATCH routes: support field_attributes Sep 17, 2024
@akirk akirk merged commit cfedc80 into akirk:main Sep 25, 2024
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