Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new filters to the OrderLimiter #41

Merged
merged 3 commits into from
Jul 15, 2020
Merged

Conversation

stevegrunwell
Copy link
Contributor

Based on some of the feedback we've received through WordPress.org, it seems it would be helpful for store owners to be able to further customize the logic around what counts against the limit.

This PR introduces two new filters: limit_orders_pre_count_qualifying_orders and limit_orders_pre_get_remaining_orders.

Both follow a preemptive pattern (much like pre_http_request), where the first parameter starts as a boolean false, and behavior is changed only if the return value is different:

$value = apply_filters( 'some_pre_filter', false );

if ( false !== $value ) {
    return $value;
}

// Otherwise, do something more complicated.

This pattern lets us bypass potentially-expensive queries, rather than performing them and then offering the opportunity to replace it.

limit_orders_pre_count_qualifying_orders

This filter lets store owners add logic around which orders count against the limit:

limit_orders_pre_get_remaining_orders

This filter lets stores dynamically change the limit, letting them trigger the store's catalog mode based on whatever parameters they deem necessary.

@stevegrunwell stevegrunwell added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 15, 2020
@stevegrunwell stevegrunwell added this to the Version 1.3.0 milestone Jul 15, 2020
@stevegrunwell stevegrunwell requested a review from bswatson July 15, 2020 15:32
* number of remaining orders that can be accepted.
* @param OrderLimiter $limiter The current OrderLimiter object.
*/
$remaining = apply_filters( 'limit_orders_pre_get_remaining_orders', false, $this );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry about passing $this for the implementation as it requires the developer to explicitly to include OrderLimiter in their own plugin / theme if they want to test it. Instead, is there specific data that we could provide that would allow customization without coupling their code to ours?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is your concern around developers having the explicitly require_once the class definition, or the fact that we're passing the object in its entirety?

If the former, the class would already be loaded into memory as the filter is being applied from within the OrderLimiter object. Additionally, as long as the Limit Orders plugin is active, the autoloader defined in the base plugin file will also be registered.

The reasoning for passing the limiter object rather than individual parameters is that developers may need any number of the properties exposed by the OrderLimiter class — enabled/disabled status, limit, interval length, current + next interval DateTime objects, whether or not there have been orders in the interval, etc. We could either pass a half-dozen or more properties or a single object that exposes well-documented public methods.

@bswatson bswatson merged commit 2a1cae9 into develop Jul 15, 2020
@bswatson bswatson deleted the feature/limiter-filters branch July 15, 2020 20:02
@stevegrunwell stevegrunwell mentioned this pull request Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants