Skip to content

Add scheduled sale fields #34538

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.

Sign up for GitHub

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

Jump to bottom
Merged
mattsherman merged 20 commits into from
Nov 14, 2022
Merged

Add scheduled sale fields #34538

mattsherman merged 20 commits into from
Nov 14, 2022

Conversation

@mattsherman
Copy link
Contributor

@mattsherman mattsherman commented Aug 31, 2022 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

This PR adds scheduled sale support to the new product editing experience.

Closes 43-gh-woocommerce/mothra-private

How to test the changes in this Pull Request:

  1. Install the WC-Admin Test Helper and enable the new-product-management-experience.
  2. Go to Products > Add New (MVP).
  3. Go to the Pricing section.
  4. Verify that acceptance criteria from 43-gh-woocommerce/mothra-private is met.

Known issues

  • After saving a product with a scheduled sale, the product will be marked as dirty immediately (causing the "Update" button to be enabled). This was fixed by DateTimePickerControl: Only call onChange when the date actually changes #35397
  • If you change the input of the scheduled to or from fields to the same date by typing, two onChange events get fired. This baffles me (none should be fired in this case). I will create an issue for follow-up at some later point (it's pretty edge-case). This was fixed in 8229b6f
  • If the input of a DateTimePickerControl field is blank, the picker shows the current date as selected (this appears to be a Gutenberg regression) -- this should be followed up separately with Gutenberg
  • If the site's timezone is not the same as the browser's timezone, the date selected in the picker is incorrect -- this should be addressed in a followup issue/PR

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@mattsherman mattsherman self-assigned this Aug 31, 2022
@mattsherman mattsherman changed the title [WIP] Add schedule sale toggle and from and to pickers [WIP] Schedule price field Aug 31, 2022
@mattsherman mattsherman changed the title [WIP] Schedule price field [WIP] Add scheduled sale fields Aug 31, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2022 •
edited
Loading

Test Results Summary

Commit SHA: 135cc98

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900302621m 10s
E2E Tests186006019214m 22s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@github-actions github-actions bot added the package: @woocommerce/components issues related to @woocommerce/components label Sep 1, 2022
@mattsherman mattsherman mentioned this pull request Sep 8, 2022
Merged
8 tasks
This was referenced Sep 22, 2022
Merged
Merged
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 92814bf to 272e95b Compare October 3, 2022 20:30
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Oct 3, 2022
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 272e95b to 5c95ca9 Compare October 4, 2022 14:02
@mattsherman mattsherman mentioned this pull request Oct 5, 2022
Merged
8 tasks
mdperez86
mdperez86 previously requested changes Oct 7, 2022
View reviewed changes
@github-actions github-actions bot added the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Oct 9, 2022
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 9c7caeb to 58d824f Compare October 9, 2022 11:20
This was referenced Oct 13, 2022
Merged
Merged
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 03c8b35 to 488c54c Compare October 19, 2022 11:43
@github-actions github-actions bot removed the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Oct 19, 2022
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 29db82c to a121bb9 Compare October 21, 2022 16:28
@github-actions github-actions bot added package: @woocommerce/components issues related to @woocommerce/components and removed package: @woocommerce/components issues related to @woocommerce/components labels Oct 21, 2022
This was referenced Oct 24, 2022
Merged
Merged
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 089c70c to a0fa928 Compare October 26, 2022 18:39
@github-actions github-actions bot removed the package: @woocommerce/components issues related to @woocommerce/components label Oct 26, 2022
@mattsherman mattsherman mentioned this pull request Oct 27, 2022
Merged
8 tasks
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from ddc939d to 890313c Compare October 27, 2022 19:17
@mattsherman
Fix margins for scheduled sale fields
5d82fe6
@mattsherman
Set date_on_sale_from_gmt to beginning of day and date_on_sale_to_gmt…
07ab851 
… to end of day
@mattsherman
Add scheduled sale tooltip
b1314e7
@mattsherman
Format dates using site options
85f8a10
@mattsherman
Simplify scheduled sale price toggle disabled check
3dcd784
@mattsherman
Pull scheduled sale fields validation out into separate function
fb5dcea
@mattsherman
Disable eslint error for ts-ignore
c6969da
@mattsherman
Make control styles more specific to avoid bleeding into areas (like …
f802f96 
…the rick text editor toolbar)
@mattsherman
Fix SCSS lint issues (didn't have Stylelint set up properly in VSCode)
b75a74a
@mattsherman
Add Tracks event for when schedule sale toggle is clicked
8cf79de
@mattsherman
Close DateTimePickerControl's dropdown when blurring from input.
8918f5f
@mattsherman
Remove unused code from stories file
f6f918d
@mattsherman
Log when onChange is called for Controlled stories
76f5ca0
@mattsherman
Tweak how DateTimePickControl's dropdown gets the date
fc41dbe
@mattsherman
Handle when input changes occur but the end date is the same
a86eada
@mattsherman
Fixed missed linter error
7e6682e
@mattsherman
Fix failing Form test
cdb4dfc
@mattsherman
Fix DateTimePickerControl usage when uncontrolled
91fb8e4
@mattsherman mattsherman force-pushed the add/product_schedule_sale branch from 21d4f00 to 91fb8e4 Compare November 6, 2022 20:43
@mattsherman
Copy link
Contributor Author

mattsherman commented Nov 6, 2022

The non-controlled examples in Storybook seem to have broken in this branch

@joshuatf I've fixed this in 91fb8e4

Sorry about that regression! And, thanks for catching it!

I believe everything is a-okay now. I rebased to address a file conflict.

@mattsherman
Copy link
Contributor Author

mattsherman commented Nov 7, 2022

@joshuatf I just noticed the failed Form test. I'm investigating. I'll ping you again when I have it fixed.

@mattsherman
Copy link
Contributor Author

mattsherman commented Nov 8, 2022

@joshuatf Okay, all test pass again, the Storybook stories all appear to be working correctly, as does the product screen. So, Hopefully this is ready for a re-review now!

There are a few known issues I've listed in the PR description that I think can be handled in follow up PRs.

joshuatf
joshuatf approved these changes Nov 14, 2022
View reviewed changes
Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

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

LGTM! Testing well in both the new product experience and the storybook examples. Thanks for all the changes here 💯

@mattsherman
Copy link
Contributor Author

mattsherman commented Nov 14, 2022

Thanks for the reviews, @joshuatf ! 🙌

@mattsherman mattsherman merged commit 5b06618 into trunk Nov 14, 2022
@mattsherman mattsherman deleted the add/product_schedule_sale branch November 14, 2022 21:03
@github-actions github-actions bot added this to the 7.2.0 milestone Nov 14, 2022
@github-actions github-actions bot mentioned this pull request Dec 15, 2022
Closed
@github-actions github-actions bot mentioned this pull request Jan 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@octaedro octaedro octaedro left review comments

@joshuatf joshuatf joshuatf approved these changes

@mdperez86 mdperez86 mdperez86 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mattsherman mattsherman

Labels

package: @woocommerce/components issues related to @woocommerce/components plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

7.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@mattsherman @octaedro @joshuatf @mdperez86