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

Onboarding: Clean up unused MC settings data #2584

Conversation

kt-12
Copy link
Collaborator

@kt-12 kt-12 commented Sep 5, 2024

Changes proposed in this Pull Request:

Closes #2572 .

Setting up free listings during onboarding (step 2) or editing free listings after onboarding still work as expected (no regressions)

Screenshots:

Detailed test instructions:

  1. Setting up free listings during onboarding (step 2) or editing free listings after onboarding still work as expected

Additional details:

Changelog entry

@kt-12 kt-12 requested a review from joemcgill September 5, 2024 08:13
Copy link

codecov bot commented Sep 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.8%. Comparing base (c5c547a) to head (504167a).
Report is 177 commits behind head on feature/2458-streamline-onboarding.

Additional details and impacted files

Impacted file tree graph

@@                           Coverage Diff                           @@
##             feature/2458-streamline-onboarding   #2584      +/-   ##
=======================================================================
- Coverage                                  65.2%   63.8%    -1.4%     
=======================================================================
  Files                                       800     325     -475     
  Lines                                     22899    5083   -17816     
  Branches                                   1230    1231       +1     
=======================================================================
- Hits                                      14928    3241   -11687     
+ Misses                                     7803    1674    -6129     
  Partials                                    168     168              
Flag Coverage Δ
js-unit-tests 63.8% <100.0%> (-0.1%) ⬇️
php-unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ponents/free-listings/setup-free-listings/index.js 6.8% <100.0%> (ø)
js/src/data/actions.js 7.9% <ø> (ø)

... and 488 files with indirect coverage changes

@kt-12 kt-12 linked an issue Sep 5, 2024 that may be closed by this pull request
2 tasks
Copy link
Collaborator

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

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

This looks right to me. I tested this by editing the free listings from the dashboard after completing onboarding and everything seems to be working still, as expected.

@ankitguptaindia can you confirm?

Base automatically changed from feature/2494-remove-pre-launch-checklist-items to feature/2492-remove-pre-launch-checklist September 5, 2024 16:45
Base automatically changed from feature/2492-remove-pre-launch-checklist to feature/2458-streamline-onboarding September 6, 2024 00:55
@ankitguptaindia
Copy link
Member

QA/Test Report-

Testing Environment -

  • WordPress: 6.6.1
  • Theme active on store: Twenty Twenty-Four Version: 1.2
  • WooCommerce - Version 9.2.3
  • PHP: 8.3
  • Web Server: Nginx
  • Browser: Chrome - Version 127
  • OS: macOS Sonoma 14.6.1

Test Results -

  • Onboarding flow works as expected. ✅
  • Free Listing works as expected and functions properly. ✅
  • Free listing updates/edits work as expected. ✅
  • No fatal/console errors occurred while performing the above tests. ✅

Functional Demo / Screencast -.

Recording.817.mp4

Copy link
Member

@eason9487 eason9487 left a comment

Choose a reason for hiding this comment

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

The same cleanup in #2532 has been made and merged into the feature/2458-streamline-onboarding branch. Ref: d78738a

I also tested the merge of this PR locally and it resulted in an empty commit.

git checkout feature/2458-streamline-onboarding
git merge origin/feature/remove-redundant-settings
git diff HEAD~1 HEAD

This PR can be closed.

@kt-12
Copy link
Collaborator Author

kt-12 commented Sep 9, 2024

Closing this as it's already handled in #2532

Check comment #2584 (review)

@kt-12 kt-12 closed this Sep 9, 2024
@eason9487 eason9487 deleted the feature/remove-redundant-settings branch September 10, 2024 02:05
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.

Onboarding: Clean up unused MC settings data
4 participants