Skip to content

SSO: do not display JITM when not in wp-admin #14622

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

Merged
merged 4 commits into from
Feb 13, 2020
Merged

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Feb 10, 2020

Fixes #14547

Changes proposed in this Pull Request:

This PR introduces a change to the filter that can be used to customize the list of JITMs returned by a site for a specific query. We now pass the message path to that filter, so folks customizing the list of messages can choose to only do so on specific pages, for example.

We then use that new parameter to ensure that the custom JITM currently displayed the first time you log in to your site using SSO only appears when you access the main dashboard, where the message should be displayed.

Testing instructions:

  • Create a test site using this branch.
  • Setup Jetpack and Connect to WordPress.com
  • Turn on the following setting in Jetpack Settings:

Screen Shot 2020-01-31 at 1 43 35 PM

- Visit WordPress.com/stats/day/siteslug - You should not see a JITM telling you more about the SSO feature. - Go back to wp-admin and log out. - Log back in using SSO. - You should see the JITM then.

Proposed changelog entry for your changes:

  • Secure Sign On: do not display feature message when logging in to WordPress.com's central dashboard.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] SSO [Status] Needs Review This PR is ready for review. labels Feb 10, 2020
@jeherve jeherve added this to the 8.3 milestone Feb 10, 2020
@jeherve jeherve requested a review from a team February 10, 2020 14:13
@jeherve jeherve self-assigned this Feb 10, 2020
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Feb 10, 2020
@jetpackbot
Copy link
Collaborator

jetpackbot commented Feb 10, 2020

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: March 3, 2020.
Scheduled code freeze: February 25, 2020

Generated by 🚫 dangerJS against d89edb2

kwight
kwight previously approved these changes Feb 10, 2020
Copy link
Contributor

@kwight kwight left a comment

Choose a reason for hiding this comment

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

Works wonderfully 👍

gwwar
gwwar previously approved these changes Feb 11, 2020
Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the fix @jeherve

I verified that this only displays in wp-admin:
Screen Shot 2020-02-11 at 3 11 25 PM

And without any Calypso notices for SSO:
Screen Shot 2020-02-11 at 3 12 05 PM
Screen Shot 2020-02-11 at 3 11 58 PM

@jeherve jeherve dismissed stale reviews from gwwar and kwight via 3f85104 February 12, 2020 13:45
zinigor
zinigor previously approved these changes Feb 12, 2020
Copy link
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

The change looks good to me, with one minor comment. If you decide it's fine to go like this, let's go like this.

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 12, 2020
@kraftbj kraftbj merged commit c85a654 into master Feb 13, 2020
@kraftbj kraftbj deleted the fix/sso-jitm-calypso branch February 13, 2020 05:41
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 13, 2020
@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Feb 14, 2020
jeherve added a commit that referenced this pull request Feb 25, 2020
jeherve added a commit that referenced this pull request Feb 25, 2020
* 8.3 release: changelog

* Changelog: add #14516

* Changelog: add #14574

* Bring in changes from 8.2.1 and 8.2.2

* Update stable version

* Bring in 8.2.3 changes

* Changelog: add #14714

* Changelog: add #14639

* Changelog: add #14678

* Changelog: add #14673

* Changelog: add #14687

* Changelog: add #14704

* Changelog: add #14702

* Changelog: add #14541

* Changelog: add #14657

* Changelog: add #14622

* Changelog: add #14582

* Changelog: add #14638

* Changelog: add #14633

* Changelog: add #14571

* Changelog: add #14592

* Changelog: add #14539

* Changelog: add #14514

* Changelog: add #14643

* Changelog: add #14494

* Changelog: add #13739

* Changelog: add #14707

* Changelog: add #14736

* Changelog: add #14706

* Changelog: add #14730

* Changelog: add #14685

* Changelog: add #14727

* Changelog: add #14711

* Changelog: add #14742

* Changelog: add #14746

* Changelog: add #14725

* Changelog: add #13999

* Changelog: add #14740

* Changelog: add #14759

* Changelog: add #14703

* Changelog: add #14753

* Changelog: add #14754

* Changelog: add #14645

* Cahngelog: add #14599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] SSO [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSO JITM appears in Calypso Sidebar
7 participants