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/165: check if dependencies are loaded before showing onboarding process #167

Merged
merged 12 commits into from
Jul 26, 2024

Conversation

Sidsector9
Copy link
Member

@Sidsector9 Sidsector9 commented Jul 3, 2024

All Submissions:

  • Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

Closes #165

This PR does the following if the dependencies are unmet:

  1. Does not display the onboarding wizard.
  2. Prevents direct access to the onboarding wizard via URL.
  3. Disables the "Connect to Square" button on the Settings page.
  4. Does not show the "Setup Wizard" link on the plugins page.

Steps to test the changes in this Pull Request:

Changelog entry

Fix - Check if dependencies are loaded before showing onboarding process.

@Sidsector9 Sidsector9 self-assigned this Jul 3, 2024
@Sidsector9 Sidsector9 marked this pull request as draft July 3, 2024 14:08
@Sidsector9 Sidsector9 changed the title fix/165: check if cURL is loaded fix/165: check if dependencies are loaded before showing onboarding process Jul 3, 2024
@Sidsector9 Sidsector9 force-pushed the fix/165 branch 3 times, most recently from fc6be49 to 25936d5 Compare July 3, 2024 15:12
@Sidsector9 Sidsector9 force-pushed the fix/165 branch 2 times, most recently from f18e021 to 08feadd Compare July 3, 2024 16:06
Copy link
Contributor

@dkotter dkotter left a comment

Choose a reason for hiding this comment

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

Tested and seems to work great, though we do still get a couple admin notices that mention connecting to Square and another one that provides a link to the onboarding:

Admin notices

Not critical but would be great to remove these if possible

@Sidsector9 Sidsector9 requested a review from dkotter July 3, 2024 16:59
Copy link
Contributor

@dkotter dkotter left a comment

Choose a reason for hiding this comment

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

Sorry for all the back and forth, this works great but in testing one last thing, you can still set up a sandbox connection. Once that's done, it causes the same fatal error to occur. If we can easily prevent that connection as well, that would be great

@vikrampm1 vikrampm1 added this to the 4.7.1 milestone Jul 7, 2024
@qasumitbagthariya
Copy link
Contributor

qasumitbagthariya commented Jul 15, 2024

QA Update ✅


I have checked this issue in the fix/165 branch and the related issue is resolved and working as expected.

Testing Environment

  • WordPress: 6.5.5
  • Theme: Storefront 4.6.0
  • Theme: Twenty Twenty-Four 1.1
  • WooCommerce - 9.1.2
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS Ventura 13.3

Before Fix

image

After Fix

image image image image

Steps to Test- As mentioned in the PR description.
Test Results - It is working as expected.
Functional Demo / Screencast -
Special Notes - Ready for code review (Woo)
Testing Document status:
Cases related to this Issue/PR are added to the Critical Flow Wiki pages:

  • Yes
  • Not Required/Applicable for this PR

@vikrampm1 vikrampm1 requested review from a team and james-allan and removed request for a team July 17, 2024 00:23
@vikrampm1
Copy link
Contributor

@james-allan this PR is ready for your review.

@diegocurbelo diegocurbelo requested review from diegocurbelo and removed request for james-allan July 19, 2024 20:37
Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

Changes look good, and tests well.. including the notices fix:

Screenshot 2024-07-19 at 6 02 10 PM

@vikrampm1 vikrampm1 modified the milestones: 4.7.1, 4.7.2 Jul 22, 2024
@qasumitbagthariya
Copy link
Contributor

Regression / Smoke Test Report ✅

Tested with Archive File created via "php woorelease.phar build repo_URL" (Composer version 2.5.5, npm version 10.7.0, node version 20.15.1)

Status- Working expected with Plugin Archive/Zip file same as fix specific branch.

Testing Environment

  • WordPress: 6.6.1
  • Theme: Twenty Twenty-Four 1.2
  • WooCommerce - 9.1.2
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS Ventura 13.3

Next Step- Ready to Merge 🚀

@vikrampm1 vikrampm1 marked this pull request as ready for review July 26, 2024 17:24
@vikrampm1 vikrampm1 merged commit b54bb14 into trunk Jul 26, 2024
6 checks passed
@vikrampm1 vikrampm1 deleted the fix/165 branch July 26, 2024 17:25
@vikrampm1 vikrampm1 mentioned this pull request Jul 26, 2024
16 tasks
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.

If cURL is not installed on the server, WooCommerce Square gives a fatal, instead of failing gracefully.
5 participants