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

Fix - Sale price effective date #2809

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

vinkmeta
Copy link
Contributor

@vinkmeta vinkmeta commented Oct 3, 2024

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 incorrect. Instead we should not pass any value for sale price effective date, start date and end date when sale price is not populated.

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 - Sale price effective date

@vinkmeta vinkmeta changed the title Fix Sale Price Effective Date Fix - Sale price effective date Oct 3, 2024
@rawdreeg rawdreeg requested a review from a team November 4, 2024 12:33
Copy link
Contributor

@rawdreeg rawdreeg left a comment

Choose a reason for hiding this comment

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

I have tested these changes; they effectively fix the issue. LGTM!

@vahidkay-meta vahidkay-meta merged commit f38a0b7 into vahidkay-meta:develop Dec 4, 2024
@vahidkay-meta vahidkay-meta mentioned this pull request Dec 6, 2024
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants