woocommerce / facebook-for-woocommerce

A first-party extension plugin built for WooCommerce. Development is managed by Ventures.
https://woocommerce.com/products/facebook/
GNU General Public License v2.0
211 stars 140 forks source link

Fixing Sale Price Effective Date #2789

Closed vinkmeta closed 3 months ago

vinkmeta commented 3 months ago

Changes proposed in this Pull Request:

When User doesn't provide any value for sale price start/end dates then by default we set Start Date as 1970-01-29 and End Date as 2038-01-17 which is not required as it simply mocks the following Meta's internal logic:

_**A. If sale_price_effective_date is not provided, we always consider the item as "On Sale". This means that we will always use the sale_price as the current price of the item.

B. If the sale doesn't have a start date (i.e., field sale_price_effective_date only contains end date) it will start showing the sale price immediately until the end date is reached.

C. If it doesn't have an end date (i.e., field sale_price_effective_date only contains start date) it will start showing the sale price as soon as the start date is reached and show that price indefinitely.

D. If the sale has both start and end dates (i.e., field sale_price_effective_date contains both dates) the saleprice will be used during the period of the sale.**

Hence to avoid surfacing generic Sale Price Effective Date, we should keep it empty to keep the behaviour consistent with other partners.

Screenshots:

Screenshots from Meta Commerce Manager

Before:

Screenshot 2024-08-19 at 10 17 14

After:

Screenshot 2024-08-19 at 10 17 30

Detailed test instructions:

  1. Unit test - ./vendor/bin/phpunit --filter test_sale_price_and_effective_date
  2. This change only impacts sale_price_effective_date and no other product fields or user experience
  3. Earlier default value for sale_price_effective_date use to be '1970-01-29T00:00+00:00/2038-01-17T23:59+00:00' when no value was set for $sale_price_start_date and $sale_price_end_date by the plugin user but now this will set as empty in that case.

Changelog entry

Fix - Took care of something that wasn't working.