Skip to content

Conversation

@rachelmcr
Copy link
Contributor

Part of: #8046

Description

This PR includes the support @ealeksandrov added for a site's frame_nonce option in the Networking, Yosemite, and Storage layers:

  • Gets the frame_nonce site option when the user's sites are requested from the me/sites endpoint.
  • Stores the value as the frameNonce attribute on the Site entity in Core Data (adding a new Core Data model).

Testing

The frameNonce attribute is not yet used in the app; confirm all unit tests pass.

Submitter Checklist

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@rachelmcr rachelmcr added the feature: products onboarding Related to onboarding new users to manage products label Nov 7, 2022
@rachelmcr rachelmcr added this to the 11.2 milestone Nov 7, 2022
@peril-woocommerce
Copy link

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@rachelmcr rachelmcr marked this pull request as ready for review November 7, 2022 15:48
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 7, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8052-78be945 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@rachelmcr rachelmcr requested a review from a team as a code owner November 7, 2022 20:29
Copy link
Contributor

@pachlava pachlava left a comment

Choose a reason for hiding this comment

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

I was automatically added as a reviewer as a part of "mobile-ui-testing-squad" because the PR changes a UI tests related file: namely, adds (78be945) frame_nonce param to rest_v11_me_sites.json.

I can see that all UI tests started to pass after that, so it's good to go from our side. Kudos for doing this @rachelmcr! 🙌

(I did not do a code review of the rest of the files).

@rachelmcr
Copy link
Contributor Author

Just a note that I'm waiting to merge this PR until we've decided whether to move forward with storing frame_nonce in Core Data or another approach. (Internal ref: pe5pgL-1ea-p2)

@rachelmcr rachelmcr added the status: blocked Blocked from progressing somehow, waiting for another piece of work to be done. label Nov 8, 2022
Copy link
Contributor

@ealeksandrov ealeksandrov left a comment

Choose a reason for hiding this comment

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

LGTM!
As we discussed in post linked above, I consider it safe to merge :shipit:
Thanks for taking another look around frame-nonce logic in Jetpack!

@rachelmcr rachelmcr removed the status: blocked Blocked from progressing somehow, waiting for another piece of work to be done. label Nov 8, 2022
@rachelmcr rachelmcr merged commit f624c72 into trunk Nov 8, 2022
@rachelmcr rachelmcr deleted the issue/8046-add-frame-nonce branch November 8, 2022 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: products onboarding Related to onboarding new users to manage products

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants