updated logic for get_asset_by_tp_media_id() now takes queryargs so w…#24
updated logic for get_asset_by_tp_media_id() now takes queryargs so w…#24
Conversation
…e can include a platform-slug arg
|
@acrosman @jesseves @augustuswm I've made a proposed change to how get_asset_by_tp_media_id() works. I've added the ability to take an args array, same types of args we do for other endpoints (particularly the 'platform-slug' arg). The current behavior is if the returned asset URL generates a 404, we'd append '/edit' to the request to try and get the 'private' record. I didn't love this because theoretically we might not be able to hit that endpoint for assets the client id doesn't have edit rights on. Also, I don't love that this can return unpublished assets, which doesn't seem right. The proposed new behavior is to allow for passing a platform-slug arg (and other args) that would be appropriate for the asset we're looking for. I think the actual use-case we're looking for is usually going to be because the asset is platform restricted anyway. Thoughts? I could modify this so it could also take a "private" arg, or maybe fallback to the old behavior if no queryargs are passed. |
|
I don't believe we rely on the auto-lookup of the edit endpoint here. Currently we use the I would be in favor of seeing if anyone else is using it. If it is not being used I like the idea of separating the two (and increasing the version). Currently it is weird in that you can't necessarily determine based on a 404 why you got the error. ie. "Non-existence" is equivalent to "No access" |
|
To @augustuswm 's comment, I realize I also prefer not doing the auto-lookup of the edit endpoint because the new approach keeps the resolved asset URL unedited. That leaves it a bit more logical to do the get_updatable_object on the response. |
|
We aren't doing anything with the edit links at the moment. So any changes to how they are discovered or access should not effect our projects. We have picked up a couple more stations using the module, but as far as I can tell from issues they have opened on Drupal.org they are also just using the functions in the module and not the full library (although they can access it) – which is to say some time in the future we may have more feedback on and concern about editing related features. |
|
As @acrosman said, we're not currently doing anything with the edit links. So, as long as this doesn't introduce a breaking change to the core use of the function (send ID, get asset), then I don't see an issue. |
| $query = "/assets/legacy/?tp_media_id=" . $tp_media_id; | ||
| $response = $this->get_request($query); | ||
| $endpoint = "/assets/legacy/?tp_media_id=" . $tp_media_id; | ||
| $response = $this->get_request($endpoint); |
There was a problem hiding this comment.
I may be missing it, but is there a reason to perform this query without the $queryargs if they are passed in?
There was a problem hiding this comment.
I've confirmed -- /assets/legacy/?platform-slug=partnerplayer&tp_media_id=[someid] returns a 404, and the URL returned does not include the platform-slug (or any other) arg. So yeah, there's no point in including the queryargs on the initial call.
|
@jesseves @acrosman this might actually be 'breaking' in a certain way. You are probably going to get more 404s, unless you update your calls to include the queryargs with 'platform-slug' => 'partnerplayer' . The 'edit' endpoint we were failing over to ignores platform. I could easily conditionalize this like so: |
|
@augustuswm I'm not sure if PBS's actual legacy endpoint even understands additional queryargs. I'll check and see; if it does, I'll include the args in the initial query. |
|
@tamw-wnet Good call, I did not consider that |
4 participants
…e can include a platform-slug arg