Developer Experience for extending the capabilities of data liberation #123
Conversation
… since controlling just the post_type isn't enough in itself
…ring better means
There was a problem hiding this comment.
Good stuff, great that we're thinking about how extending Try WordPress might work. I have couple of notes/questions.
AFAICT, this documentation is only about overriding/extending the transformation process. If so, I think it could make sense to make the document dedicated to that use case. The idea being that there will be other types of "extending", for example, providing additional subject types. Just an idea, we can also split things into separate documents later.
Concerning the suspend/resume:
If I understood correctly, the way suspend/resume here works is on a subject-by-subject basis: the 3PP has the choice to override/extend the transformation for each subject. So it might override for subject with ID 42, and do nothing for subject with ID 84.
I'm wondering if this is a use case that we will have. I would think that typically, if a 3PP would override/extend a transformation, it would want to do it for all subjects of that type. Can you think of a specific use case that would require overriding/extending only certain posts?
Let's say that instead of a 3PP completely overriding the default transformations, they would want to "extend", as in do something in addition to the default transformation. Would that be possible with the approach currently described in the doc?
|
This is a great first iteration! A couple of thoughts:
WordPress WayI'd like to suggest to design the "API" in a more WordPress-y way. Specifically, instead of calling a Handle function to register the plugin, let's design WordPress hooks that are generated dynamically and can thus be just subscribed to. For example in our code we'd have (mind the first parameter being the post_id passed through the filters): $post_id = apply_filters( "liberate_data_{$content_type}", null, $data );and we'd recommend to register by add_filter( 'liberate_data_product', function( $post_id, DotOrg\TryWordPress\Subject $data ) {
// run your transformations and return the post id that was created
$post_id = wp_insert_post( array( 'post_type' => 'your_post_type' ) );
return $post_id;
}, 2, 10 );This could also be used to just observe: add_filter( 'liberate_data_product', function( $post_id, DotOrg\TryWordPress\Subject $data ) {
// do meta stuff like SEO tags
return $post_id;
}, 2, 100 );Or we introduce an action: $post_id = apply_filters( "liberate_data_{$content_type}", null, $data );
if ( $post_id ) {
do_action( "liberated_data_{$content_type}", $post_id, $data );
}TransformationsI am not sure I fully understand what you are proposing. What's the need for suspending and resuming? Could it be done with the above filter depending on whether a post was created or not and/or you'd hook in with a later priority? More specific examplesI'd also vote for providing a generic description of an available API but then provide a very concrete example that will do something useful if you copy and paste it. In my opinion it's much easier to relate and understand example code if it doesn't leave any gaps for the reader to fill. btw, there's also a couple of typos (e.g. collabrates), so please run it through a spell checker in the end. |
|
Also in light of what I wrote in #101 we should have our own reference implementations of these hooks for posts and pages and we could also link or use them. The beauty about hooks is that this means that also plugin authors can improve our reference implementations by modifying the data that we inserted, or even fully replace our implementation. |
|
@psrpinto See my responses below:
Yes! The markdown file name is temporary, but this is dedicated to the transformation aspect. We can find the final name/location after this week ends when bringing all exploration together.
Not exactly! 3PP authors have the choice of either overriding transformation of a specific type of subject and provide theirs OR just observe data to do whatever. Specifically choosing to process a record of a subject_type and not the entire records of a subject_type isn't planned in this iteration. What would be the use-case that warrants this? One example of processing a type of product comes to mind, but then it might be better to define products as multiple subject types since they are essentially different and requires different handle. Eg: A e-commerce plugin wanting to handle only the digital products that are downloadable and physical products that require physical shipping. A subject type should have consistent behavior of handling and shouldn't vary. If it does, we need to define them as different subject types.
I classified the possible options as either "complete override" or "don't touch at all" because it keeps things simple. You can also think of it as "do I need to disable native transformations or not?". If the use-case requires having to extend, as you say it, then second option of just "observing" is more appropriate.
I realised I didn't explain this part at all. cc: @akirk These are only relevant in running transformations post liberation, not during. Let's say, we have a native transformation of a product (t-shirt) being handled by Woocommerce. Now, if another e-commerce plugin (lets call it Doo) comes in, it needs to suspend Woo's product and offer a preview where only Doo is powering product pages and nothing from Woo's side. This is where "suspend" comes in. It suppresses Woo's products as if they are not even there. Now, Doo can offer a much better preview of how the website looks if it's all Doo and nothing from Woo. Should the user choose to switch to Doo from Woo, they can "persist", which deletes Woo's data making Doo's data permanent. In case, the user doesn't like Doo and wants to revert to Woo, that's where the "resume" comes in. Resume just un-suspends stuff. This behavior is the guideline of how any 3PP author should act keeping user's best interest in mind. |
|
@akirk See my responses below:
I have left the final name/location for next week, once we are about to merge the explorations of this week. I believe we can only do that then and make the entire documentation coherent.
Sure! I can rephrase it in the final version.
Is there any advantage to this approach? I am a fan of actions and filters myself, but the code examples you wrote for example doesn't immediately conveys the meaning as they should & requires reading up. I specifically designed it the way I did, to make it really easy to understand i.e. "handle" when you want to handle transformations by overriding and "observe" to indicate the read-only nature.
I explained that in my response to Paulo and tagged you.
Yes, we would have more examples from the consumption side as well. Wanted to get feedback on the architecture. By no means this is the final state of the document itself, sorry if it looked like that.
Surely!
We can allow hooks & filters within our code at various points, but the user journey for this plugin starts with the extension & is not of a typical WP plugin that would be installed on a WP site. So, I am trying to be mindful of what options we provide and where t best guide the user. The only use-case I can think of which would benefit from hooks/filters to modify our behavior/data is making bug fixes or customizations but that's again a totally different kind of user for our tool and not the initial segment which we want to target. It also makes it harder to decipher what happened because any hook or filter could have been used. So, I am wondering if you have identified more use-cases that would require this? |
Co-authored-by: Paulo Pinto <[email protected]>
An important part of this work is to try and see this through the eyes of a plugin developer. What if you pretended that you have an already finished shopping plugin and want to receive liberated products. How can you answer the question well "what do I need to add?"
I don't agree that I think there are several upsides to using hooks as I prosed:
Don't get distracted by my preliminary naming of the hook. It could be also called
I think you might have misunderstood me here. What I am saying is that we should be customers of those provided hooks ourselves. When receiving a post, we should subscribe to the same hook that a plugin would subscribe to and implement it.
I don't agree with this direction. If I have two shopping plugins, they should both be able to register products for them. I could decide for one or the other by deactivating the other plugin. Overall, I have been trying to establish a "delete everything and retry" approach. We are in playground and in a safe sandbox. If something didn't work out, or if I want to try a new plugin, I belive we should delete everything except for our liberated post types and re-run the hooks as if these content types were just imported. Thus I think we can completely remove the suspend/resume that you are suggesting. |
I think we're saying the same thing :) I don't see a use case for this either. |
|
@akirk Please allow me to address the points you raise in detail. I am not married to a particular implementation but I would like to present the case of why I believe they are a superior choice since its quite possible I didn't explain everything that I have in my head. If there is something I am not considering, I would like to understand that a bit better :)
I was merely pointing to the fact that Paulo's and mine documentation would merge at the end of the week & we need it to be coherent as a whole, a single documentation for the project. Eg: Calling a piece of content as subject and not mixing it with content_types in between. Similarly for subject_type. Much better to name the markdown file and title it that way, once we are at the point of merge. Also, we identified three different types of documentation we needed (this was discussed separately so looping you in):
That's already presented in the architecture proposal in this PR: Since it's for "want to receive liberated products", they would use the loop method Does that answer your question?
I was going for a short method name, preferably a single word to keep the api simple. handle X with Y is a natural way to express the operation, where X is subject_type and Y is the handler. And since there is no ambiguity to resolve here, Current architecture avoids competing for the same subject type for "during liberation", because there should be a singular output for a subject & needs to error out when multiple handlers are registered so that user can make a choice as to which one should run.
WP hooks/filters isn't a suitable choice in my opinion here because of the following reasons:
Additionally, the entire API proposed is designed with these principles in mind:
I see. It's not straight forward unfortunately since we don't have a clean separation of raw data being stored and when native transformation runs on it. It's all together, but we abstract away those implementation details. I thought about separating them, but would be quirky and not good enough of a ROI, I think.
Why delete everything when we can do this soft-delete approach of suspend/resume? Especially when we abstract away the complexity for 3PP authors to just call the functions and not implement themselves. Re-running transformation would be a bit tricky, since all native transformations are not PHP-only and if we are storing a copy of them anyway, to avoid that, we are implementing a kinda soft-delete approach. Also, there is no guarantee the user would get the exact output back, they might get slightly different since latest logic for transformation available at that point would run. Because of these reasons, I like the suspend/resume (Soft-delete) and revert approach better. 3PP authors would only have to implement a "confirm" or "revert" button in their UI for this "post liberation" integration. |
|
Concerning the design of the public API, I think I agree with @akirk that it would be best do it the "WordPress-y" way, for the reasons he has outlined above. That doesn't mean however that we cannot use an object-oriented approach in our own code if we want to, that would not be exposed to consumers of the public API. These abstractions can then be called by the hooks. I think maybe this PR is going too deep, it touches on important topics that I personally had not considered before, like the suspend/resume thing. I think it would make sense to look at that topic in specific in it's own PR or issue, as it's an important one to get right, I believe. I think maybe here we could focus on the public API that we will expose to consumers, without thinking too much about the internals of how it all works. I think that if we ourselves are wondering about internals when designing the public API, that's a sign that something could be off in the way we're looking at things, because the goal of the public API should be to abstract how things are done internally. Just my 2 cents. |
|
I would like us to expose the API using WordPress filters. It's the WordPress way. While it is true that the parameters don't have strict typing like function signatures, the hook system is what plugin authors understand and can use in powerful ways. If we want to make it easy for plugin authors, we should not introduce a new (superior or not) way of doing this. I also am a strong believer in that we should be consuming the APIs ourselves for providing support for native WordPress post types: ---
title: Extraction Data Flow
---
flowchart TD
A[Extension] -->|extracted data| B(try-wp Plugin)
B --> |store in liberated post type| C{Call Hook}
C -->|Post| D[try-wp Plugin]
C -->|Page| E[try-wp Plugin]
C -->|Other| F[Other Plugins]
For retries, I don't think the plugins should be able to trigger this but the try-wp plugin would provide a UI where the user can decide to reimport products. It would thus just loop over the already imported liberated post types and call the hook with the exact same parameters as if it was just received from the extension. ---
title: Retry Flow
---
flowchart LR
B(Liberated Post Type) --> |foreach| C(Hook)
C -->|Post| D[try-wp Plugin]
C -->|Page| E[try-wp Plugin]
C -->|Other| F[Other Plugins]
D --> G(Repeat)
E --> G
F --> G
G --> C
Thus I believe the API for plugin authors could be a single callback hook. All they need to know they will receive the structured data as extracted from the frontend, and the full HTML alongside. They don't need to know if the data is coming "fresh" from the extension or whether it's a retry. |
|
I will comply. Closing this PR in favor of #125 |
|
Think of it this way: we need to make it super attractive for plugin authors to add support. Thus it needs to be very easy to understand and to add with the same tools that they are already using. Having to learn something, even if it is superior, will make it unappealing to add support for. |
This PR offers a Rendered view of the documentation and some code changes supporting the architecture described. More code changes are required for them to be in effect. My hope is that this PR itself will implement those changes and support the final version of architecture before merge.
TODO:
@TODOin the codebase