Fix: add supported subscriptions features before checking for existing attached hooks

[Mayisha]

Fixes #4014 (comment)

Changes proposed in this Pull Request:

• Added supported subscription features before checking for existing attached hooks.

Testing instructions

• Code review.

[Mayisha]

@diegocurbelo do you see any possible issues if we move the self::$has_attached_integration_hooks check after declaring the supported features? 👀

[diegocurbelo]

@diegocurbelo do you see any possible issues if we move the self::$has_attached_integration_hooks check after declaring the supported features? 👀

None, I like the solution, it won't affect what we fixed in #4014

[kaushikasomaiya]

Thanks for working on this!
I do have one question in mind - the check was to protect against double attachment of payment hook and I guess without the check, $this->supports was being declared twice as well which wasn't an issue however no declaration makes me wonder if the woocommerce_scheduled_subscription_payment_ hook would also be getting skipped due to the check now?

[diegocurbelo]

Thanks for working on this! I do have one question in mind - the check was to protect against double attachment of payment hook and I guess without the check, $this->supports was being declared twice as well which wasn't an issue however no declaration makes me wonder if the woocommerce_scheduled_subscription_payment_ hook would also be getting skipped due to the check now?

We want to skip the hooks if this is not the first instance of this class, to ensure we are registering/attaching the hooks only once per payment method (ex woocommerce_scheduled_subscription_payment_sepa_debit).

Having the check before the $this->supports = ... was a mistake because that variable is not static, so subsequent instantiations of the same class would not have it set; everything else in the constructor needs to be skipped as they are all add_actions/add_filters calls.

[diegocurbelo]

LGTM 🚢