humanmade / asset-manager-framework

A framework for overriding the WordPress media library with an external asset provider, such as a DAM
GNU General Public License v2.0
155 stars 7 forks source link

Add RequestOne trait to support `get-attachment` ajax action #71

Open roborourke opened 10 months ago

roborourke commented 10 months ago

In the media library you can view attachment details without actually triggering the "select" action from the media modal that localises images.

In some instances, e.g. when you have highlighted an image causing a URL update and then refresh WordPress will try to fetch the attachment individually using the get-attachment action.

Some APIs will provide a way to fetch just one item, so in those cases providers can opt in to using this trait and implement the request_one() method.

johnbillion commented 9 months ago

I agree this should be implemented. I added a review comment about the specific implementation though, I think this can be done in a better way.

roborourke commented 9 months ago

Thanks @johnbillion. I think we ended up going with interfaces (rather than traits) as a way to properly ensure certain methods were implemented and to make it obvious to devs how they should be written. The methods like supports_single_request - I'm not sure that's ideal because it doesn't provide any further guidance on how to implement that support, and doesn't enforce the expected parameters and return values etc that we would need.

I would change it to check for the method existing rather than whether the trait exists but I feel like interfaces and traits are going to be more robust as a pattern. I need to refresh my memory on that whole discussion.