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

Change connection test page capabilities to manage_woocommerce #2443

Merged

Conversation

jorgemd24
Copy link
Contributor

@jorgemd24 jorgemd24 commented Jun 25, 2024

Changes proposed in this Pull Request:

Part of #2146

Currently, the connection test page is only available to the administrator because it is registered with the manage_options capability. However, all our endpoints are checking the manage_woocommerce capability.

protected function can_manage(): bool {
return current_user_can( 'manage_woocommerce' );
}

This makes it harder for a Shop Manager to debug any issues with the new approach as the connection test page is not available. Therefore, this PR updates the capability to manage_woocommerce. Since all actions available on the connection test page are also accessible through the UI for users with manage_woocommerce capability I don't think we are giving new privileges to the Shop Manager role.

Screenshots:

Detailed test instructions:

  1. Log in to wp-admin with a Shop Manager user role.
  2. Add this filter add_filter('woocommerce_gla_enable_connection_test','__return_true'); or navigate to wp-admin/admin.php?page=connection-test-admin-page
  3. Check that you can access the Connection Test Page.

Additional details:

Not sure why but the phpcs were failing for the account services test, so I fixed it in this PR.

Changelog entry

@jorgemd24 jorgemd24 self-assigned this Jun 25, 2024
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jun 25, 2024
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 64.5%. Comparing base (aec6d57) to head (e048937).
Report is 477 commits behind head on feature/google-api-project.

Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                       @@
##             feature/google-api-project   #2443     +/-   ##
==============================================================
+ Coverage                          64.2%   64.5%   +0.3%     
- Complexity                         4484    4541     +57     
==============================================================
  Files                               471     472      +1     
  Lines                             18862   17707   -1155     
==============================================================
- Hits                              12117   11425    -692     
+ Misses                             6745    6282    -463     
Flag Coverage Δ
php-unit-tests 64.5% <0.0%> (+0.3%) ⬆️

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

Files Coverage Δ
src/ConnectionTest.php 0.0% <0.0%> (ø)

... and 202 files with indirect coverage changes

@jorgemd24 jorgemd24 marked this pull request as ready for review June 25, 2024 20:47
@jorgemd24 jorgemd24 requested a review from a team June 25, 2024 20:47
Copy link
Member

@ianlin ianlin 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 the change. I can confirm that the connection page is accessible when using the shop manager account.

@jorgemd24 jorgemd24 merged commit d20d93f into feature/google-api-project Jun 26, 2024
12 checks passed
@jorgemd24 jorgemd24 deleted the tweak/connection-test-page-capability branch June 26, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants