Skip to content

[JP Plugin] Track error reason when installation fails#20436

Merged
dvdchr merged 2 commits intotrunkfrom
feature/jp-plugin-track-install-error-message
Mar 31, 2023
Merged

[JP Plugin] Track error reason when installation fails#20436
dvdchr merged 2 commits intotrunkfrom
feature/jp-plugin-track-install-error-message

Conversation

@dvdchr
Copy link
Copy Markdown
Contributor

@dvdchr dvdchr commented Mar 30, 2023

Refs p1679941373337129-slack-C01CW1VMLAF

This is a small improvement that tracks the error's localizedDescription for .unknown error types.

To test

  • Prepare an individual site that's connected to your WordPress.com account.
  • Install the full Jetpack plugin from wp-admin, and then deactivate it.
  • Launch the Jetpack app, and switch to your individual site.
  • Go through the Jetpack plugin installation flow. The installation flow should fail.
  • Verify that this is tracked: 🔵 Tracked: jp_install_full_plugin_onboarding_modal_viewed <description: Destination folder already exists., status: error>

Regression Notes

  1. Potential unintended areas of impact
    Self-hosted Jetpack plugin installation may be impacted when the error is also unknown.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manually tested the changes.

  3. What automated tests I added (or what prevented me from doing so)
    N/A.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@dvdchr dvdchr added [Type] Enhancement Jetpack App Jetpack Install Anything related to the Jetpack Install process labels Mar 30, 2023
@dvdchr dvdchr added this to the 22.1 milestone Mar 30, 2023
@dvdchr dvdchr self-assigned this Mar 30, 2023
@dvdchr dvdchr requested a review from wargcm March 30, 2023 17:05
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Mar 30, 2023 •
edited
Loading

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr20436-8f8ee45
Version22.0
Bundle IDorg.wordpress.alpha
Commit8f8ee45
App Center BuildWPiOS - One-Offs #5379
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Mar 30, 2023 •
edited
Loading

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr20436-8f8ee45
Version22.0
Bundle IDcom.jetpack.alpha
Commit8f8ee45
App Center Buildjetpack-installable-builds #4407
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wargcm
wargcm approved these changes Mar 30, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@wargcm wargcm left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@wargcm
Copy link
Copy Markdown
Contributor

wargcm commented Mar 30, 2023

I retested without checking the failures but it looks like this change may break some existing unit tests:

Failing tests 
WordPressTest.WPComJetpackRemoteInstallViewModelTests
--
  | test_installJetpack_givenValidSite_AndRequestFails_shouldUpdateStateToFailure, XCTAssertTrue failed
  | WPComJetpackRemoteInstallViewModelTests.swift:106
  | ```
  | // assert
  | XCTAssertTrue(viewModel.state == .failure(.unknown))
  | }
  | ```
  |  
  | test_installJetpack_givenValidSite_AndRequestFails_shouldUpdateStateToFailure, XCTAssertTrue failed
  | WPComJetpackRemoteInstallViewModelTests.swift:106
  | ```
  | // assert
  | XCTAssertTrue(viewModel.state == .failure(.unknown))
  | }
  | ```
  |  
  | test_installJetpack_givenValidSite_AndRequestFails_shouldUpdateStateToFailure, XCTAssertTrue failed
  | WPComJetpackRemoteInstallViewModelTests.swift:106
  | ```
  | // assert
  | XCTAssertTrue(viewModel.state == .failure(.unknown))
  | }
  | ```

There were some other tests failing but they looked unrelated to this change.

@dvdchr
Copy link
Copy Markdown
Contributor Author

dvdchr commented Mar 31, 2023

Good catch @wargcm. I didn't run the tests locally 😅. This is now fixed in 8f8ee45.

@dvdchr dvdchr enabled auto-merge March 31, 2023 06:16
@dvdchr dvdchr merged commit 7f685f6 into trunk Mar 31, 2023
@dvdchr dvdchr deleted the feature/jp-plugin-track-install-error-message branch March 31, 2023 06:38
@thomashorta thomashorta mentioned this pull request Apr 3, 2023
Closed
@dvdchr dvdchr mentioned this pull request Apr 14, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@wargcm wargcm wargcm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dvdchr dvdchr

Labels

Jetpack App Jetpack Install Anything related to the Jetpack Install process

Projects

None yet

Milestone

22.1

Development

Successfully merging this pull request may close these issues.

3 participants

@dvdchr @wpmobilebot @wargcm