Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Fixing admin console links in the announcements banner #4484

Merged
merged 3 commits into from
Dec 11, 2019
Merged

Fixing admin console links in the announcements banner #4484

merged 3 commits into from
Dec 11, 2019

Conversation

jespino
Copy link
Member

@jespino jespino commented Dec 10, 2019

Summary

Fixing admin console links in the announcements banner

Ticket Link

MM-21009

@jespino jespino added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Dec 10, 2019
@jespino jespino added this to the v5.18.0 milestone Dec 10, 2019
@lindalumitchell lindalumitchell added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Dec 11, 2019
lindalumitchell
lindalumitchell approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@lindalumitchell lindalumitchell left a comment

Choose a reason for hiding this comment

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

Couldn't test with the Site URL as that field can't be edited (set by environment variable) on the test server. Tested instead by entering an invalid URL for Plugin Marketplace, which also results in a System Console link in announcement banner. It worked fine to open System Console on desktop app and Chrome, no issues.

@lindalumitchell lindalumitchell added QA Review Done Tests/Done Release tests have been written and removed 3: QA Review Requires review by a QA tester labels Dec 11, 2019
@Willyfrog
Copy link
Contributor

quick question, as I recall some conversation on Apps about having a separate tab for this on the browser due to having the user move to a really different section. @esethna can you take a look and let us know if there should be a different behaviour on desktop vs browser for this?

@Willyfrog Willyfrog added the 1: PM Review Requires review by a product manager label Dec 11, 2019
alifarooq0
alifarooq0 suggested changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@alifarooq0 alifarooq0 left a comment

Choose a reason for hiding this comment

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

Missing test case where siteURL is not present.

components/announcement_bar/text_dismissable_bar.test.jsx Outdated Show resolved Hide resolved
@mattermod
Copy link
Contributor

Mattermost test server updated with git commit 69018f1ab949d6c73f9108cee9298d17ddfa1ed1.

Access here: https://mattermost-webapp-pr-4484.test.mattermost.cloud

@esethna esethna removed the 1: PM Review Requires review by a product manager label Dec 11, 2019
esethna
esethna approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@esethna esethna left a comment

Choose a reason for hiding this comment

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

@Willyfrog I think it's fine as is

alifarooq0
alifarooq0 approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@alifarooq0 alifarooq0 left a comment

Choose a reason for hiding this comment

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

👌

@alifarooq0 alifarooq0 added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Dec 11, 2019
@jespino jespino merged commit 03a208d into mattermost:master Dec 11, 2019
@jespino jespino deleted the MM-21009 branch December 11, 2019 18:51
@mattermod
Copy link
Contributor

Test server destroyed

@mattermost-build mattermost-build mentioned this pull request Dec 11, 2019
Merged
@mattermod mattermod added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Dec 11, 2019
jespino pushed a commit that referenced this pull request Dec 11, 2019
@mattermost-build @jespino
Automated cherry pick of #4484 (#4497)
512c431 
* Fixing admin console links in the announcements banner

* Adding tests for text_dismissable_bar component

* Adding a test without siteURL
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Dec 12, 2019
@hanzei hanzei removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Dec 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Willyfrog Willyfrog Willyfrog approved these changes

@esethna esethna esethna approved these changes

@alifarooq0 alifarooq0 alifarooq0 approved these changes

@lindalumitchell lindalumitchell lindalumitchell approved these changes

Assignees

@jespino jespino

Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation QA Review Done Tests/Done Release tests have been written
Projects
None yet
Milestone
v5.18.0
Development

Successfully merging this pull request may close these issues.
8 participants
@jespino @Willyfrog @mattermod @esethna @alifarooq0 @lindalumitchell @hanzei @amyblais