Good-Bids / goodbids

0 stars 0 forks source link

na - static values for threshold #920

Closed jaspercroome closed 6 months ago

jaspercroome commented 6 months ago

Summary

Adjusting the threshold for determining whether an auction is ending soon, and setting a lower limit so that users don't get spammed (12 hour extension windows could send notifications up to 4 times).

@bd-viget / @nathan-schmidt-viget - I'm not sure the logic for comparing makes sense in php-land, please double-check. Thanks!

bd-viget commented 6 months ago

@jaspercroome If the intention is to use static values for the threshold, then what you may want to go back to the original concept that gets Auctions ending between 2 dates/times:

https://github.com/Good-Bids/goodbids/pull/916/commits/104d8ca433eb85f3baa7aebefc8c881d07357c0f#diff-a5e78537023a274831bc7ccaf6ba88e1a8fafa50b97e60e887246b090fd9f65eR410-R434

/**
 * Get any live auctions that are within a given range of their extension time of closing.
 *
 * @since 1.0.1
 *
 * @param string $threshold_start
 * @param string $threshold_end
 *
 * @return array
 */
public function get_auctions_by_close_date( string $threshold_start, string $threshold_end ): array {
    $query_args = [
        'meta_query' => [
            [
                'key'     => self::AUCTION_CLOSED_META_KEY,
                'value'   => '0',
            ],
            [
                'key'     => self::AUCTION_CLOSE_META_KEY,
                'value'   => $threshold_start,
                'compare' => '>=',
            ],
            [
                'key'     => self::AUCTION_CLOSE_META_KEY,
                'value'   => $threshold_end,
                'compare' =>'<=',
            ],
        ]
    ];

    $auctions = $this->get_all( $query_args );

    return $auctions->posts;
}

And call it like this:

// Get Auctions ending between 1hr and 1hr15min from now.
$threshold_start = current_datetime()->add( new DateInterval( 'PT1H' ) )->format( 'Y-m-d h:i:00' );
$threshold_end   = current_datetime()->add( new DateInterval( 'PT1H15M' ) )->format( 'Y-m-d h:i:59' );

$ending_soon = $this->get_auctions_by_close_date( $threshold_start, $threshold_end );

The changes I made removed this because the original threshold was based on the Auction Bid Extension metadata, which couldn't be queried with WP_Query.

Cron Schedule

This change will result in missed notifications based on the current Cron Schedule. The Cron Schedule is simply based on when the first Cron is scheduled, and that happened whenever this code was deployed and the first request was made. So if the code was deployed at 3:24pm, that's when the schedule will be set up, and will run hourly after that (4:24pm, 5:24pm, etc). Even if we specify an hour (say 4pm), each cron will still fire on the hour (4pm, 5pm, 6pm, etc).

Using a 1 hr to 1hr and 15min window, only auctions ending within the first 15min of the window will get notified. Anything in the last 45min period will not.

Example:

Cron Schedule:

Notice how Auction B and D never fit within that window, therefore never get notified.

So what you must do (if the window is 15min) is run the cron every 15min.

Original Concept

Going back to the original concept where Ending Soon was based on the Bid Extension window, since the Bid Extension window is dynamic, you really need to run a cron every minute to ensure all possible Auctions have notifications sent out when they're ending soon. This gets tricky when trying to avoid spam because you don't want to send out multiple emails every minute for the same Auction, so you need to flag Auctions that already have had emails go out, so they aren't sent out again.

EXCEPT, since the Auction can be Extended, you'll then want to REMOVE that flag so the NEXT window to send out notifications includes the Auction based on the new Close Date.

jaspercroome commented 6 months ago

Thanks for the feedback, @bd-viget !

using your example:

Auction A: Ends at 5:05pm, hits the 'ending soon' criteria between 3:10p and 4:05p Auction B: Ends at 5:30pm, hits the 'ending soon' criteria between 3:35p and 4:30p Auction C: Ends at 6:02pm, hits the 'ending soon' criteria between 4:07p and 5:02p Auction D: Ends at 7:45pm, hits the 'ending soon' criteria between 5:52p and 6:45p

Cron Schedule: 4pm: Auctions classified as 'ending soon': A & B 5pm: Auctions classified as 'ending soon': C 6pm: Auctions classified as 'ending soon': D

jaspercroome commented 6 months ago

merging in, to verify & test on develop

bd-viget commented 6 months ago

Ah! Sorry for the confusion! I totally misread that! In that case, I would recommend setting the threshold for 1hr through 1hr 59 minutes to cover all possibilities.