Skip to content
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

Bug/dynamic tags user author meta #1540

Merged
merged 16 commits into from
Dec 4, 2024

Conversation

iansvo
Copy link
Collaborator

@iansvo iansvo commented Nov 26, 2024

Fixes #1538

Requires

Fixed

  • Author meta was not showing the "data" keys from wp_users
  • User meta not populating at all due to invalid parameter type
  • Tag supports was not properly detected when the tag changed
  • Post record request was happening once before the tag was loaded
  • Various minor bug fixes

@iansvo iansvo requested a review from tomusborne November 26, 2024 17:19
@iansvo iansvo self-assigned this Nov 26, 2024
@tomusborne
Copy link
Owner

@iansvo Testing the user meta issue mentioned above. I'm still only seeing generateblocks_onboarding as the only available key when querying the current user. Should I be seeing more than that?

@iansvo
Copy link
Collaborator Author

iansvo commented Nov 28, 2024

@tomusborne yes, part of the reason for that is because author "data" and user meta aren't really the same thing. The former is made up of the columns in wp_user vs the rows in wp_usermeta.

So for the author, i have the data keys (such as display_name) in the dropdown, but I wasn't sure if it made sense to also do that for users.

Part of me thinks that having author/user meta tags is fuckin pointless. They are effectively the same thing, except the user in question can be "author of this post" vs a user you explicitly set.

Let me know what you think here. At the very least we could add the wp_user columns to the user_meta also, which I think still works even though they aren't actually meta.

@tomusborne
Copy link
Owner

As a user trying to spin up a dynamic tag for a user (or the current user), I think I would want to see options like display_name etc... We may just want to populate the list with those things to improve UX.

@iansvo
Copy link
Collaborator Author

iansvo commented Dec 2, 2024

@tomusborne I just pushed an update to both PRs that adds these keys to the user section as well. I also needed to create a new user record endpoint so we can have more control like we do with posts.

All of this is sorted, but there's now another components PR that goes with it: https://github.com/EDGE22-Studios/components/pull/12

@tomusborne
Copy link
Owner

@iansvo I'm still not seeing any user meta populating the dropdown.

Using both PRs, and both are linked with the components PR.

@tomusborne
Copy link
Owner

Here's a video of what I'm seeing. Looks like it's not firing on initial load - something else has to happen first.

user-meta.mp4

@iansvo
Copy link
Collaborator Author

iansvo commented Dec 3, 2024

@tomusborne I resolved the issue there. I had a flaw in the way I was handling the userRecordId. This now should work correctly. I'm going to continue additional testing for authors to make sure this is leveraging only that as well. Previously it was using the post record.

@iansvo
Copy link
Collaborator Author

iansvo commented Dec 3, 2024

@tomusborne I've made sure that authors and users now use the same user-record endpoint. Previously authors were using the post record for their data, which seems kinda pointless now.

This is all working in my tests. I can confirm both dropdowns populate as expected without any user interaction.

@tomusborne tomusborne merged commit 9ef1ffc into release/2.0.0 Dec 4, 2024
9 checks passed
@tomusborne tomusborne deleted the bug/dynamic-tags-user-author-meta branch December 4, 2024 15:12
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