MM-41955 Reduce useless changes to user state #9833

hmhealey · 2022-02-17T18:46:03Z

As discussed, we want to reduce accidental state changes caused by reducers returning new state objects which are identical to the the previous values. This does that for all of the most used parts of users state (profiles, statuses, profilesInChannel). I didn't bother doing it with a lot of the other ones like profilesInTeam or profilesNotInGroup since they're only really used in the System Console and modals for managing users in channels/teams/groups, so they won't impact general performance as much. Ideally, I'd actually like to get those ones out of redux entirely, but that's future work.

Ticket Link

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

Release Note

Improved performance of code for storing users in web app

hmhealey · 2022-02-17T18:48:33Z

components/root/root.jsx

@@ -181,10 +180,6 @@ export default class Root extends React.PureComponent {
 }

 onConfigLoaded = () => {
- if (isDevMode()) {
- enableDevModeFeatures();


This added a warning when trying to access a non-existent .length property of a Set or Map which lodash does in isEqual to try to identify the type of object. Since we have TypeScript now that will warn us if we do that by mistake, that warning isn't needed any more anyway.

M-ZubairAhmed · 2022-02-17T19:43:08Z

packages/mattermost-redux/src/reducers/entities/users.ts

@@ -458,25 +454,31 @@ function profilesNotInGroup(state: RelationOneToMany<Group, UserProfile> = {}, a
 }
 }

+function saveToState<T>(state: Record<string, T>, key: string, value: T): Record<string, T> {


can we think of a better name, because its not actually saving but merging 0/5 🤔

Maybe addToState? I just put this helper here, but it seems like it'd be really helpful throughout all the redux state. I'm pretty sure we could use these helpers in Joram's PR too, but I don't want to change too much else right now.

hmhealey · 2022-02-17T20:02:26Z

/e2e-tests

jwilander

LGTM - one non-blocking comment

packages/mattermost-redux/src/reducers/entities/users.ts

hmhealey · 2022-02-17T21:37:27Z

/e2e-test

mattermod · 2022-02-17T21:37:59Z

Triggering e2e testing with options:
<nil>

mattermod · 2022-02-17T21:38:00Z

Successfully triggered e2e testing!
https://git.internal.mattermost.com/qa/cypress-ui-automation/-/pipelines/152432

jgilliam17 · 2022-02-25T03:38:28Z

E2E report shows many failures (35+) related to autocomplete
Mention autocomplete is showing a loading spinner e.g. for no results and also when match is found

hmhealey · 2022-02-28T20:40:36Z

/e2e-test

hmhealey · 2022-02-28T20:40:56Z

Those should be fixed now. Turns out the autocomplete was throwing an error which broke most of those tests

jgilliam17 · 2022-03-01T15:55:34Z

/update-branch

jgilliam17 · 2022-03-01T15:58:51Z

Updating, will restart e2es afterwards, the cycle failed to start yesterday.

jgilliam17 · 2022-03-01T17:34:50Z

/e2e-test

mattermod · 2022-03-01T17:35:22Z

Triggering e2e testing with options:
<nil>

mattermod · 2022-03-01T17:35:23Z

Successfully triggered e2e testing!
https://git.internal.mattermost.com/qa/cypress-ui-automation/-/pipelines/157495

jgilliam17

Thanks @hmhealey
Tested, looks good to merge.

E2E report looks good, no PR related failures.

mm-cloud-bot · 2022-03-01T20:42:31Z

Test server destroyed

amyblais · 2022-03-07T14:32:38Z

/cherry-pick release-6.3

mattermod · 2022-03-07T14:32:39Z

Cherry pick is scheduled.

mattermod · 2022-03-07T14:32:42Z

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Failed to add the RSA host key for IP address '140.82.114.4' to the list of known hosts (/app/.ssh/known_hosts).
From github.com:mattermost/mattermost-webapp
   2255783b2..773c6da47  master      -> upstream/master
   2bd0eb01e..be55b969e  release-6.4 -> upstream/release-6.4
   df6d2aefd..e4224d8eb  release-6.5 -> upstream/release-6.5
Fetching origin
Failed to add the RSA host key for IP address '140.82.113.4' to the list of known hosts (/app/.ssh/known_hosts).
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-mattermost-webapp-#9833-upstream-release-6.3-1646663560
Switched to a new branch 'automated-cherry-pick-of-mattermost-webapp-#9833-upstream-release-6.3-1646663560'
Branch 'automated-cherry-pick-of-mattermost-webapp-#9833-upstream-release-6.3-1646663560' set up to track remote branch 'release-6.3' from 'upstream'.

+++ About to attempt cherry pick of PR #9833 with merge commit 707676cc3b860e53e2a2134cb4ecfab0e93fefd5.

error: could not apply 707676cc3... MM-41955 Reduce useless changes to user state (#9833)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

+++ Conflicts detected:

UU components/root/root.jsx
UU components/root/root.test.jsx
UU packages/mattermost-redux/src/reducers/entities/users.ts
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

amyblais · 2022-03-07T14:35:45Z

@hmhealey Needs a manual cherry-pick.

* MM-41955 Reduce useless changes to user profile state * MM-41955 Reduce useless changes to user statuses state * Remove helper that prints errors when using lodash's isEqual * Remove tests involving enableDevModeFeatures * Handle test that uses an array instead of a Set * Rename saveToState to addToState * Fix autocomplete Co-authored-by: Mattermod <[email protected]>

* MM-41955 Reduce useless changes to user profile state * MM-41955 Reduce useless changes to user statuses state * Remove helper that prints errors when using lodash's isEqual * Remove tests involving enableDevModeFeatures * Handle test that uses an array instead of a Set * Rename saveToState to addToState * Fix autocomplete Co-authored-by: Mattermod <[email protected]> Co-authored-by: Mattermod <[email protected]>

hmhealey added 3 commits February 17, 2022 13:41

MM-41955 Reduce useless changes to user profile state

ecbe517

MM-41955 Reduce useless changes to user statuses state

a4e139e

Remove helper that prints errors when using lodash's isEqual

a5428be

hmhealey 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 Feb 17, 2022

hmhealey added this to the v6.3.0 milestone Feb 17, 2022

hmhealey requested review from jwilander, M-ZubairAhmed and jgilliam17 February 17, 2022 18:46

mm-cloud-bot added the release-note label Feb 17, 2022

hmhealey commented Feb 17, 2022

View reviewed changes

hmhealey added 2 commits February 17, 2022 13:50

Merge branch 'master' into MM-41955_reduce-user-state-changes

b1830b3

Remove tests involving enableDevModeFeatures

a29a057

M-ZubairAhmed reviewed Feb 17, 2022

View reviewed changes

M-ZubairAhmed approved these changes Feb 17, 2022

View reviewed changes

hmhealey added 2 commits February 17, 2022 14:56

Handle test that uses an array instead of a Set

baaf72b

Rename saveToState to addToState

d9e07f2

jwilander approved these changes Feb 17, 2022

View reviewed changes

packages/mattermost-redux/src/reducers/entities/users.ts Show resolved Hide resolved

jwilander removed the 2: Dev Review Requires review by a core commiter label Feb 17, 2022

hmhealey removed the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Feb 17, 2022

hmhealey removed this from the v6.3.0 milestone Feb 17, 2022

jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Feb 25, 2022

Fix autocomplete

73925cc

Merge branch 'master' into MM-41955_reduce-user-state-changes

15f1781

jgilliam17 approved these changes Mar 1, 2022

View reviewed changes

jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Mar 1, 2022

jgilliam17 merged commit 707676c into master Mar 1, 2022

jgilliam17 deleted the MM-41955_reduce-user-state-changes branch March 1, 2022 20:42

amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Mar 2, 2022

amyblais assigned hmhealey Mar 7, 2022

amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Mar 7, 2022

hmhealey mentioned this pull request Mar 7, 2022

MM-41955 Reduce useless changes to user state (#9833) (release-6.3) #9959

Merged

amyblais 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 Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-41955 Reduce useless changes to user state #9833

MM-41955 Reduce useless changes to user state #9833

hmhealey commented Feb 17, 2022

hmhealey Feb 17, 2022

M-ZubairAhmed Feb 17, 2022

hmhealey Feb 17, 2022

hmhealey commented Feb 17, 2022

jwilander left a comment

hmhealey commented Feb 17, 2022

mattermod commented Feb 17, 2022

mattermod commented Feb 17, 2022

jgilliam17 commented Feb 25, 2022

hmhealey commented Feb 28, 2022

hmhealey commented Feb 28, 2022

jgilliam17 commented Mar 1, 2022

jgilliam17 commented Mar 1, 2022

jgilliam17 commented Mar 1, 2022

mattermod commented Mar 1, 2022

mattermod commented Mar 1, 2022

jgilliam17 left a comment

mm-cloud-bot commented Mar 1, 2022

amyblais commented Mar 7, 2022

mattermod commented Mar 7, 2022

mattermod commented Mar 7, 2022

amyblais commented Mar 7, 2022

MM-41955 Reduce useless changes to user state #9833

MM-41955 Reduce useless changes to user state #9833

Conversation

hmhealey commented Feb 17, 2022

Ticket Link

Release Note

hmhealey Feb 17, 2022

Choose a reason for hiding this comment

M-ZubairAhmed Feb 17, 2022

Choose a reason for hiding this comment

hmhealey Feb 17, 2022

Choose a reason for hiding this comment

hmhealey commented Feb 17, 2022

jwilander left a comment

Choose a reason for hiding this comment

hmhealey commented Feb 17, 2022

mattermod commented Feb 17, 2022

mattermod commented Feb 17, 2022

jgilliam17 commented Feb 25, 2022

hmhealey commented Feb 28, 2022

hmhealey commented Feb 28, 2022

jgilliam17 commented Mar 1, 2022

jgilliam17 commented Mar 1, 2022

jgilliam17 commented Mar 1, 2022

mattermod commented Mar 1, 2022

mattermod commented Mar 1, 2022

jgilliam17 left a comment

Choose a reason for hiding this comment

mm-cloud-bot commented Mar 1, 2022

amyblais commented Mar 7, 2022

mattermod commented Mar 7, 2022

mattermod commented Mar 7, 2022

amyblais commented Mar 7, 2022