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

MM 18912 Migrate 'utils/filter_users.js' and associated tests to TypeScript #4890

Merged
merged 6 commits into from
Sep 28, 2020

Conversation

MathewtheCoder
Copy link
Contributor

@MathewtheCoder MathewtheCoder commented Feb 12, 2020

Summary

Migrated utils/filter_users.jsx and associate test files to type script

Ticket Link

Fixes mattermost/mattermost#12491
JIRA Link: https://mattermost.atlassian.net/browse/MM-18912

@MathewtheCoder MathewtheCoder changed the title Mm 18912 Migrate 'utils/filter_users.js' and associated tests to TypeScript MM 18912 Migrate 'utils/filter_users.js' and associated tests to TypeScript Feb 12, 2020
@@ -2,19 +2,22 @@
// See LICENSE.txt for license information.
import {UserSearchOptions, UserListOptions, UserFilters} from 'utils/constants';

export type FilterOptions = {
[key: string]: string | boolean;
}
Copy link
Member

@M-ZubairAhmed M-ZubairAhmed Feb 13, 2020

Choose a reason for hiding this comment

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

Can you please include a new-line here before your comment for linting test to pass

@hanzei hanzei added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Feb 20, 2020
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Looking good, just need to fix the lint error.
I've also added a non-blocking comment.
Thanks @MathewtheCoder!

@@ -2,19 +2,22 @@
// See LICENSE.txt for license information.
import {UserSearchOptions, UserListOptions, UserFilters} from 'utils/constants';

export type FilterOptions = {
Copy link
Member

Choose a reason for hiding this comment

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

There's actually a TypeScript provided type you can use here instead of defining your own dictionary:
Record<string, string | boolean>

@mattermod
Copy link
Contributor

This issue has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @hanzei

@jasonblais
Copy link
Contributor

Hey @MathewtheCoder, let us know if you have any questions about feedback from @devinbinnie? We'd be happy to clarify. Thanks for your contribution! :)

@MathewtheCoder
Copy link
Contributor Author

Sorry for not following up with the PR. I will look into the comment and fix the lint error.

@hanzei hanzei added Awaiting Submitter Action Blocked on the author and removed 2: Dev Review Requires review by a core commiter labels Mar 24, 2020
@hanzei
Copy link
Contributor

hanzei commented Sep 22, 2020

@MathewtheCoder Gentle reminder to fix the merge conflict.

@hanzei hanzei self-assigned this Sep 22, 2020
@hanzei hanzei removed the request for review from jgilliam17 September 22, 2020 10:08
# Conflicts:
#	utils/filter_users.test.ts
#	utils/filter_users.ts
@hanzei hanzei removed their assignment Sep 25, 2020
@MathewtheCoder
Copy link
Contributor Author

@hanzei Sorry for the slow response. I have fixed the merge conflicts and pushed the changes.

@devinbinnie
Copy link
Member

/update-branch

@devinbinnie devinbinnie added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Sep 28, 2020
@devinbinnie devinbinnie merged commit b3f8607 into mattermost:master Sep 28, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Sep 28, 2020
@hanzei hanzei added this to the v5.30.0 milestone Sep 28, 2020
@hanzei
Copy link
Contributor

hanzei commented Sep 28, 2020

Thanks for returning back to this PR @MathewtheCoder 🎉

Tak-Iwamoto pushed a commit to Tak-Iwamoto/mattermost-webapp that referenced this pull request Oct 1, 2020
…o MM-20457

* 'master' of github.com:Tak-Iwamoto/mattermost-webapp:
  Added legal text to upgrade button, and showing again the Edition section in team edition (mattermost#6470)
  add uiClickPostDropdownMenu (mattermost#6581)
  Fix nested a tags in Marketplace labels (mattermost#6520)
  [MM-27277] Enable stream mode for file uploads (mattermost#6568)
  move cypress plugin api (mattermost#6496)
  [MM-28361] User limit reached and overage banners for Mattermost Cloud (mattermost#6548)
  MM-28409 Add e2e for testing archive channel header (mattermost#6522)
  channel cannot be required (because it's often not there), so check for it (mattermost#6475)
  Cypress test for MM-T2368 'Fixed width center' setting does not cause scroll pop and breaks post display (mattermost#6443)
  MM 18912 Migrate 'utils/filter_users.js' and associated tests to TypeScript (mattermost#4890)
  fix getType is not a function (mattermost#6566)
  MM-29028 Remove findDOMNode from Slack import (mattermost#6554)
  MM-28997 Remove references to old context API (mattermost#6545)
  Add custom slash command tests (mattermost#6391)
  Cypress/E2E: Fix guest experience ui spec (mattermost#6563)
  [MM-28784] Migrate string refs to functional ones (mattermost#6494)
jfrerich pushed a commit that referenced this pull request Oct 23, 2020
…Script (#4890)

* MM-18912 Migrate filter_users utility file to ts

* Revert "MM-18912 Migrate filter_users utility file to ts"

This reverts commit 890f587.

* MM-18912 Migrated filter_users utility file to ts

* Added types for isActive function

Co-authored-by: Mattermod <[email protected]>
calebroseland pushed a commit that referenced this pull request Oct 27, 2020
…Script (#4890)

* MM-18912 Migrate filter_users utility file to ts

* Revert "MM-18912 Migrate filter_users utility file to ts"

This reverts commit 890f587.

* MM-18912 Migrated filter_users utility file to ts

* Added types for isActive function

Co-authored-by: Mattermod <[email protected]>
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
Projects
None yet
9 participants