Introduce edd_item_supports() function

[pippinsplugins]

We should introduce an edd_item_supports() function that would allow developers to determine if a product supports various features.

For example:

edd_item_supports( 'quantities' );

edd_item_supports( 'taxes' );

edd_item_supports( 'shipping' );

This should be introduced as a method in the EDD_Download class that then has a global wrapper function.

[evertiro]

oooh! I like that idea!

[cklosowski]

👍

[danieliser]

How would you implement it? I'm assuming in the Download class you would have a filter that developers could hook into to check meta and enable features on the fly storing them in a private class variable?

Sounds relatively straight forward. I can take this one.

[pippinsplugins]

The plan is to have it work just like add/remove_theme_support() and add/remove_post_type_supports().

PRs always welcome :)

[danieliser]

So the question becomes, since both of those are based on static items. Post Types & Themes aren't quite as dynamic as say download 242 supports feature x.

So this becomes a template function that uses the current query args similar to how get_the_content( $id = 0 ) works.

In which case I would just make functions to wrap new methods in EDD_Download class.

Unless I misunderstood the usage. But I can't quite see a general download_supports('key') functions usage.

But a download_support( $id, $key ) sounds like something I would use during templating or in a loop mainly.

If this is correct by your original idea then I can take this one as well.

[pippinsplugins]

We need to introduce both a method in EDD_Download and a global helper
function that calls the method.

On Sun, Jan 18, 2015 at 7:19 PM, Daniel Iser notifications@github.com
wrote:

So the question becomes, since both of those are based on static items.
Post Types & Themes aren't quite as dynamic as say download 242 supports
feature x.

So this becomes a template function that uses the current query args
similar to how get_the_content( $id = 0 ) works.

In which case I would just make functions to wrap new methods in
EDD_Download class.

Unless I misunderstood the usage. But I can't quite see a general
download_supports('key') functions usage.

But a download_support( $id, $key ) sounds like something I would use
during templating or in a loop mainly.

If this is correct by your original idea then I can take this one as well.

—
Reply to this email directly or view it on GitHub
#2955 (comment)
.

[danieliser]

OK, that's how I had it worked in my head. I can get this done. Just wanted to be sure it matched your original Idea, in that it is a function to check that each individual download supports a feature.

[pippinsplugins]

If you end up working on it, post a proof of concept before going all the way through it, just to ensure everyone is on the same page and work doesn't get wasted.

[danieliser]

No problem. What do you suggest? The functionality itself is rather simple. You want an outline of how it will be integrated and used? Can do that for sure.

[pippinsplugins]

Just a basic proposal of the API itself.

[danieliser]

Sure thing. WIll post it here.

[pippinsplugins]

I'm going to go ahead and punt this to 2.4 since it's still a nice to have.

[danieliser]

Will get this written up this week. That way it will be ready for testing early in the 2.4 cycle.

[pippinsplugins]

👍

On Tue, Feb 10, 2015 at 9:23 PM, Daniel Iser notifications@github.com
wrote:

Will get this written up this week. That way it will be ready for testing
early in the 2.4 cycle.

—
Reply to this email directly or view it on GitHub
#2955 (comment)
.

[danieliser]

So here is what I have worked out. Initially the way pippin proposed it, it was described to be on a per download basis, but the functions he said it would be similar to are not built the same way.

Referring to his mention of add/remove_theme_support() and add/remove_post_type_supports().

Both of those assume that the download is a global thing, not something that could have multiple instances.

I'm proposing that it be done via a method in EDD_Download just like he initially mentioned. This will include an array of supported feaures.

private $supports = array(
    'quantities' => false,
    'taxes'      => false,
    'shipping'   => false,
);

This will be passed through a filter edd_download_supports which will allow both the setting of each feature to true or false, as well as the addition of extra features from extensions.

Usage would be something along these lines.

apply_filter( 'edd_download_supports', 'edd_download_supports_taxes' );
function edd_download_supports_taxes( $supports, $download_id ) {
    if ( edd_item_quantities_enabled( $download_id ) ) {
        $supports['quantity'] = true;
    }
    return $supports;
}

If not in the loop you would use something like

edd_download_supports( 123, 'quantity' );

If in the loop then something like

edd_item_supports( 'quantity' );

I have the commits done locally, as long as usage is along the lines of how the core team wants it I will make a PR.

[danieliser]

I have also considered allowing an array to be passed to boeth edd_download_supports and edd_item_supports to check if multiple features are supported at once.

That way you could check if a download supports both shipping & taxes in one check. This doesn't have an immediately apparent use, but it may in the future allow checking that multiple items are true before showing/doing something.

Would use type checking so it shouldn't be too difficult to make this change and support passing a string or array as the $feature argument.

[pippinsplugins]

That's exactly what I'd like to see 👍

On Saturday, February 21, 2015, Daniel Iser notifications@github.com
wrote:

I have also considered allowing an array to be passed to boeth
edd_download_supports and edd_item_supports to check if multiple features
are supported at once.

That way you could check if a download supports both shipping & taxes in
one check. This doesn't have an immediately apparent use, but it may in the
future allow checking that multiple items are true before showing/doing
something.

Would use type checking so it shouldn't be too difficult to make this
change and support passing a string or array as the $feature argument.

—
Reply to this email directly or view it on GitHub
#2955 (comment)
.

[danieliser]

@pippinsplugins perfect then. I have the commits done and will make a PR shortly. All that's missing is adding the built in features and checks to set the properly for each download. The setting and support check functions are all there already. It just needs to be integrated into existing systems and tested.

[pippinsplugins]

Excellent, that will be a great start.

On Sunday, February 22, 2015, Daniel Iser notifications@github.com wrote:

@pippinsplugins https://github.com/Pippinsplugins perfect then. I have
the commits done and will make a PR shortly. All that's missing is adding
the built in features and checks to set the properly for each download. The
setting and support check functions are all there already. It just needs to
be integrated into existing systems and tested.

—
Reply to this email directly or view it on GitHub
#2955 (comment)
.

[danieliser]

PR submitted.

[pippinsplugins]

I've merged the WIP code eom @danieliser into issue/2955

[pippinsplugins]

Doh, turns out we can't use the filter name of edd_download_supports as we already use that to determine which features the download post type supports.

[pippinsplugins]

I don't love it but for now I"m going to propose edd_does_download_support.

[pippinsplugins]

I'm not sure we should be marking taxes as a feature here. We already support edd_download_is_tax_exclusive() via a checkbox in the UI. Should we have two filters that can affect this?

[pippinsplugins]

I'm also wondering if we should move some of our existing features to this item supports API, such as variable prices, file downloads, buy now buttons, etc.

Should we go 100% or just leave it to select features?

[cklosowski]

Going to punt for now. As discussed in the team meeting, we need to get some details worked out about how this will work with backwards compatibility.

[danieliser]

@pippinsplugins - I think it makes since that nearly all core stuff should be run through it at some point in the future. Though you may plan it over several release cycles.