Replies: 6 comments
-
Yeah media should have been a reserved one since we use it interchangeably with attachment in our internals too. |
Beta Was this translation helpful? Give feedback.
-
We should look and see if there are any other names that are problematic for REST API and avoid setting those when we set the REST API base. |
Beta Was this translation helpful? Give feedback.
-
I think the best action we can take here is to create a blacklist of custom post names that we warn users not to use (but still allow if they insist) when creating the Pod itself. IMO, "media" and "user" arguably should have been blacklist CPT names all along. Having a post type named "media" will block you from extending the actual media content type as well as probably being full of pitfalls. We haven't enforced such a blacklist to this point so the toothpaste is already out of the tube. Disallowing CPTs with those names would cause a headache for sites in the wild already using one of them, thus I think a healthy warning when creating the Pod is the most authoritarian we can be. After thinking it through I believe Pods automatically changing the REST base by adding a prefix/suffix to avoid conflicts is a bad idea. This changes the path to the REST route behind the developer's back and that could very well lead to far more mysterious and confusing things than a 404. I think I'd rather people with these rare cases come through support if they have to and manually change the REST base accordingly (or much much better, rename the post type to 'media_post' or something). This did uncover at least one spot we need to fix up. For REST insert hooks we blindly change 'media' to 'attachment' in the hook name if the name of the Pod is 'media'. This should be checking the type as well or instead, so there is at least one bug in Pods that probably breaks inserting records into a post type named "media" even if WordPress didn't choke on it. https://github.com/pods-framework/pods/blob/34d1022/classes/PodsRESTFields.php?ts=4#L93-L94 Besides the above, which I'll create a new issue for, I think this is best closed as a "support issue". Best solution: don't use "media" or "user" and opt for something like "site_media" or "media_posts" instead. If you must use "media" as the name of a post type for some arcane reason then manually change the REST base, so you know that was done and no surprises, and be prepared for other hiccups. |
Beta Was this translation helpful? Give feedback.
-
Thanks. I think a MVP outcome here could still be a blacklist-based warning? Not letting people know, when we clearly could since we've identified all relevant details - would be delivering a UX failure. |
Beta Was this translation helpful? Give feedback.
-
I agree; I believe we should be warning at the time you create the Pod. |
Beta Was this translation helpful? Give feedback.
-
Related to #4122 |
Beta Was this translation helpful? Give feedback.
-
Describe the bug
It's possible to set up a REST API namespace conflict, where Gutenberg, for example, simply can't find the pod object (404).
{"code":"rest_post_invalid_id","message":"Invalid post ID.","data":{"status":404}}
Maybe
media
is the only one, maybe there's more "magic" words, that need to be blacklisted for this.To Reproduce
Steps to reproduce the behavior:
media
slugmedia
pod REST APImedia
pod postsExpected behavior
Ability to edit
media
posts.Pods Version
2.7.9
WordPress Environment
4.9.8
Possible Workaround
Renaming REST API base
prefix_media
or similar seems to work well.Pods should probably validate this against a blacklist when REST API is enabled for a pod. I bet REST API enabling will exponentially increase as Gutenberg spreads.
Beta Was this translation helpful? Give feedback.
All reactions