Skip to content

Change Jetpack connection delegates to weak references#21052

Merged
crazytonyli merged 6 commits intotrunkfrom
weak-delegate-violation-fixes-2
Jul 19, 2023
Merged

Change Jetpack connection delegates to weak references#21052
crazytonyli merged 6 commits intotrunkfrom
weak-delegate-violation-fixes-2

Conversation

@crazytonyli
Copy link
Copy Markdown
Contributor

@crazytonyli crazytonyli commented Jul 11, 2023

Relates to #21050.

The changed delegate properties should be declared as weak references, considering how they are used:

WordPress-iOS/WordPress/Classes/ViewRelated/Jetpack/Login/JetpackLoginViewController.swift

Lines 170 to 176 in 3d8c016

fileprivate func openInstallJetpackURL() {
trackStat(.selectedInstallJetpack)
let controller = JetpackConnectionWebViewController(blog: blog)
controller.delegate = self
let navController = UINavigationController(rootViewController: controller)
present(navController, animated: true)
}

WordPress-iOS/WordPress/Classes/ViewRelated/Jetpack/Install/JetpackRemoteInstallViewController.swift

Lines 104 to 110 in 3d8c016

func openInstallJetpackURL() {
viewModel.track(AccountHelper.isLoggedIn ? .connect : .login)
let controller = JetpackConnectionWebViewController(blog: blog)
controller.delegate = self
navigationController?.pushViewController(controller, animated: true)
}

Regression Notes

  1. Potential unintended areas of impact
    None.

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

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

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.

UI Changes testing checklist: N/A

@crazytonyli crazytonyli added this to the 22.9 milestone Jul 11, 2023
@crazytonyli crazytonyli requested review from jkmassel and mokagio July 11, 2023 04:11
@crazytonyli crazytonyli self-assigned this Jul 11, 2023
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jul 11, 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 Numberpr21052-22389b1
Version22.8
Bundle IDcom.jetpack.alpha
Commit22389b1
App Center Buildjetpack-installable-builds #5426
 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 Jul 11, 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 Numberpr21052-22389b1
Version22.8
Bundle IDorg.wordpress.alpha
Commit22389b1
App Center BuildWPiOS - One-Offs #6399
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@staskus staskus mentioned this pull request Jul 14, 2023
Open
@crazytonyli crazytonyli enabled auto-merge July 19, 2023 01:08
@crazytonyli crazytonyli merged commit 1851fa4 into trunk Jul 19, 2023
@crazytonyli crazytonyli deleted the weak-delegate-violation-fixes-2 branch July 19, 2023 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mokagio mokagio mokagio approved these changes

@jkmassel jkmassel Awaiting requested review from jkmassel

Assignees

@crazytonyli crazytonyli

Labels

None yet

Projects

None yet

Milestone

22.9

Development

Successfully merging this pull request may close these issues.

3 participants

@crazytonyli @wpmobilebot @mokagio