Skip to content

Admin Page: ensure user caps are available when picking active tab#13689

Merged
jeherve merged 1 commit intomasterfrom
revert/settings-change
Oct 11, 2019
Merged

Admin Page: ensure user caps are available when picking active tab#13689
jeherve merged 1 commit intomasterfrom
revert/settings-change

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented Oct 8, 2019

Follow-up from #13541

Changes proposed in this Pull Request:

We need to make sure this.props.userCanManageModules is available before we use it.

I introduced a bug in #13541: when an admin was loading Jetpack > Settings, the Writing tab was loading by default.

Testing instructions:

  • Go to Jetpack > Settings as an admin on the site.
  • The Security tab and its contents should load by default.
  • Go to Users > Add New, and add a new editor to your site.
  • Log to your site with that new user.
  • Go to Jetpack > Settings.
  • The Writing tab and its contents should load instead of the Security tab.

Proposed changelog entry for your changes:

  • None

@jeherve
Admin Page: ensure user caps are available when picking active tab
ba67662 
Follow-up from #13541

We need to make sure this.props.userCanManageModules is available before we use it.
@jeherve jeherve added Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu [Pri] High labels Oct 8, 2019
@jeherve jeherve added this to the 7.9 milestone Oct 8, 2019
@jeherve jeherve requested a review from a team October 8, 2019 12:19
@jeherve jeherve self-assigned this Oct 8, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Oct 8, 2019

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: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against ba67662

@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Oct 10, 2019 •
edited
Loading

I think this is tangental. If the secondary user is not connected, it looks kinda broken to me. It may be better to include the CTA to create an account where the settings would go and/or tease what they would get once they connect. I'll open a separate issue for that.

2019-10-10 at 10 58 AM

@kraftbj kraftbj 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 Oct 10, 2019
@jeherve jeherve merged commit 70ba68d into master Oct 11, 2019
@jeherve jeherve deleted the revert/settings-change branch October 11, 2019 12:04
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kraftbj kraftbj kraftbj approved these changes

Assignees

@jeherve jeherve

Labels

Admin Page React-powered dashboard under the Jetpack menu Bug When a feature is broken and / or not performing as intended [Pri] High

Projects

None yet

Milestone

7.9

Development

Successfully merging this pull request may close these issues.

4 participants

@jeherve @jetpackbot @kraftbj @matticbot