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

[MM-21979] Fix incorrect placeholder prop for searchable user list #4712

Merged

Conversation

fmunshi
Copy link
Contributor

@fmunshi fmunshi commented Jan 22, 2020

Summary

Placeholder is expected to be of type

    placeholder: {
        id: string;
        defaultMessage: string;
        values?: {string: any};
    };

however there was a recent commit changing the type being passed down to a string causing any component referencing SearchableUserList to error on load

Ticket Link

https://mattermost.atlassian.net/browse/MM-21979

@fmunshi fmunshi force-pushed the MM-21979-Fix-incorrect-placeholder-prop branch from 96e342e to 48e38c7 Compare January 22, 2020 17:38
@fmunshi fmunshi added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jan 22, 2020
@fmunshi fmunshi requested review from sudheerDev and a team January 22, 2020 17:38
@ghost ghost requested review from jwilander and removed request for a team January 22, 2020 17:39
@fmunshi
Copy link
Contributor Author

fmunshi commented Jan 22, 2020

Requesting @jgilliam17 for review since the original PR (#4253) was also reviewed by them

QA Steps (Copied from the ticket)

  • Open manage channel members modal and ensure that it works
  • Open manage team members modal and ensure that it works

@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 22, 2020
@mattermod mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 22, 2020
@fmunshi fmunshi added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 22, 2020
@fmunshi
Copy link
Contributor Author

fmunshi commented Jan 22, 2020

@jgilliam17 not sure why the first build failed but I pushed a new commit and created a new test server for you

@jgilliam17
Copy link
Contributor

@jgilliam17 not sure why the first build failed but I created pushed a new commit and created a new test server for you

Thank you @fm2munsh

Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Tested, looks good to merge.

  • Both manage channel members modal and manage team members modal work as expected.
  • Able to search, remove from channel/team, invite/add and make admin.

@jgilliam17 jgilliam17 added QA Review Done and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jan 23, 2020
@mattermod
Copy link
Contributor

Test server destroyed

Copy link
Contributor

@sudheerDev sudheerDev left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jwilander jwilander left a comment

Choose a reason for hiding this comment

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

LGTM

@jwilander jwilander added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Jan 23, 2020
@jwilander jwilander merged commit 66f7d68 into mattermost:master Jan 23, 2020
@fmunshi fmunshi deleted the MM-21979-Fix-incorrect-placeholder-prop branch January 23, 2020 15:45
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 23, 2020
@fmunshi fmunshi added this to the v5.22.0 milestone Jan 29, 2020
@jgilliam17 jgilliam17 added the Tests/Done Release tests have been written label Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation QA Review Done Tests/Done Release tests have been written
Projects
None yet
6 participants