Skip to content

Conversation

@louwie17
Copy link
Contributor

Changes proposed in this Pull Request:

This updates the updateLinkHref function to make sure undefined query params are removed from the URL, this is something that the parse function from qs was doing previously ( https://github.com/woocommerce/woocommerce/pull/35128/files#diff-e8cae4b4e030d5120191e67ffdbb1e3843ff79d92a445c8bcb0e4778cde4133e ), but this was removed to make use of URLSearchParams instead.

Closes #38538 .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Go to WooCommerce > Home. If you're taken to the OBW, skip it.
  2. Go to Analytics > Overview page. Notice that it loads just fine.
  3. Go to Analytics > Settings.
  4. Without changing anything in the Settings, click "Save settings".
  5. Open up the browser console.
  6. Go back to Analytics > Overview.
  7. The overview page should load fine and no errors within the console, also note that the URL doesn't contain undefined params within its query params.
  8. Change the date range to something different from the default (like last month)
  9. The Overview page should still load correctly
  10. Navigate to some of the other report pages, the date should persist (as it is set within the url query params)

@louwie17 louwie17 requested a review from a team May 31, 2023 10:57
@louwie17 louwie17 self-assigned this May 31, 2023
@github-actions github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 31, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

Hi @nathanss, @woocommerce/mothra

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

Test Results Summary

Commit SHA: 093192e

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 51s
E2E Tests1940010020414m 13s

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.

@nathanss nathanss self-requested a review May 31, 2023 12:07
Copy link
Contributor

@nathanss nathanss left a comment

Choose a reason for hiding this comment

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

Looks good and tests good!

@nathanss nathanss merged commit 7d4f65b into trunk May 31, 2023
@nathanss nathanss deleted the fix/38538_save_settings_error branch May 31, 2023 12:35
@github-actions github-actions bot added this to the 7.9.0 milestone May 31, 2023
@jonathansadowski jonathansadowski modified the milestones: 7.9.0, 7.8.0 May 31, 2023
github-actions bot pushed a commit that referenced this pull request May 31, 2023
* Filter out undefined query params

* Add changelog
jonathansadowski pushed a commit that referenced this pull request May 31, 2023
* Fix save settings error within Analytics (#38542)

* Filter out undefined query params

* Add changelog

* Prep for cherry pick 38542

---------

Co-authored-by: louwie17 <[email protected]>
Co-authored-by: WooCommerce Bot <[email protected]>
tomalec added a commit that referenced this pull request Jun 2, 2023
…)"

This reverts Layout- and Filter-related changes from commit 00c151f, reversing
changes made to eef417f.

Removes the fix (keeping the test) from #38542 as it's not needed for `qs`

Fixes #38582.
louwie17 pushed a commit that referenced this pull request Jun 5, 2023
…38593)

* Revert part of "Remove `qs` dependency from `woocommerce-admin` (#35128)"

This reverts Layout- and Filter-related changes from commit 00c151f, reversing
changes made to eef417f.

Removes the fix (keeping the test) from #38542 as it's not needed for `qs`

Fixes #38582.

* Simplify the use of query params in `ProductTour`

* Add changelog entry
moon0326 pushed a commit that referenced this pull request Jun 6, 2023
…38593)

* Revert part of "Remove `qs` dependency from `woocommerce-admin` (#35128)"

This reverts Layout- and Filter-related changes from commit 00c151f, reversing
changes made to eef417f.

Removes the fix (keeping the test) from #38542 as it's not needed for `qs`

Fixes #38582.

* Simplify the use of query params in `ProductTour`

* Add changelog entry
jonathansadowski added a commit that referenced this pull request Jun 6, 2023
…)"

This reverts Layout- and Filter-related changes from commit 00c151f, reversing
changes made to eef417f.

Removes the fix (keeping the test) from #38542 as it's not needed for `qs`

Fixes #38582.
jonathansadowski pushed a commit that referenced this pull request Jun 6, 2023
…)"

This reverts Layout- and Filter-related changes from commit 00c151f, reversing
changes made to eef417f.

Removes the fix (keeping the test) from #38542 as it's not needed for `qs`

Fixes #38582.
jonathansadowski added a commit that referenced this pull request Jun 6, 2023
* Revert part of "Remove `qs` dependency from `woocommerce-admin` (#35128)"

This reverts Layout- and Filter-related changes from commit 00c151f, reversing
changes made to eef417f.

Removes the fix (keeping the test) from #38542 as it's not needed for `qs`

Fixes #38582.

* Simplify the use of query params in `ProductTour`

* Update changelog entry in readme for 38593

---------

Co-authored-by: Tomek Wytrębowicz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

4 participants