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: mobile app connection owner bug #37170

Merged
merged 1 commit into from Mar 16, 2023
Merged

Conversation

rjchow
Copy link
Contributor

@rjchow rjchow commented Mar 10, 2023

All Submissions:

Changes proposed in this Pull Request:

A bug was discovered where users who had changed their wpcom display names would be shown the "not owner of connection" modal for the mobile app feature modal.

The cause of this was the assumption that the .connectionOwner field represents the user's wpcom username, but it turns out that it actually holds the user's display name. This went unnoticed in testing as the default display name is the user's username.

How to test the changes in this Pull Request:

  1. Set up the WooCommerce site using Jurassic Ninja, live branch or some other setup where you can have a Jetpack connection running. To test this thoroughly, check that your wpcom account's display name != your username.
  2. After going through the onboarding wizard and connecting your wordpress.com account, observe that you can see the mobile app task in the task list and in the help menu.
  3. Create another user on the wordpress site and log in as that user. You should not be able to see the mobile app task or help menu entry.

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 created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?
  • Have you included testing instructions?

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.

@github-actions github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Mar 10, 2023
@rjchow rjchow requested review from a team, chihsuan and adrianduffell March 10, 2023 12:42
@rjchow rjchow marked this pull request as ready for review March 10, 2023 12:42
@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #37170 (4127eca) into trunk (7c76118) will increase coverage by 0.0%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             trunk   #37170   +/-   ##
========================================
  Coverage     46.7%    46.7%           
- Complexity   17188    17191    +3     
========================================
  Files          429      429           
  Lines        64820    64845   +25     
========================================
+ Hits         30253    30276   +23     
- Misses       34567    34569    +2     

see 13 files with indirect coverage changes

@github-actions
Copy link
Contributor

Test Results Summary

Commit SHA: 4127eca

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 7s
E2E Tests189006019517m 41s

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.

Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it! Looks good and worked as described. 👍

Copy link
Contributor

@adrianduffell adrianduffell left a comment

Choose a reason for hiding this comment

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

LGTM, nice catch @rjchow ! 🚀

@rjchow rjchow merged commit 1c9b3a5 into trunk Mar 16, 2023
30 checks passed
@rjchow rjchow deleted the fix/mobile-app-connection-owner branch March 16, 2023 08:20
@github-actions github-actions bot added this to the 7.6.0 milestone Mar 16, 2023
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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants