PLT-7238 Migration of create_post.jsx to redux #210

sudheerDev · 2017-10-27T15:00:56Z

Summary

WIP: Migration of create_post.jsx to redux

Ticket Link

mattermost/mattermost#7238

Checklist

Added or updated unit tests (required for all new features)
Has redux changes (please link)
Touches critical sections of the codebase (auth, posting, etc.)

saturninoabril · 2017-11-06T19:16:35Z

components/create_post/create_post.jsx

+
+ /**
+ * Data used in notifyiing user for @all and @channel
+ */


just minor correction on spelling - "notifying"

saturninoabril · 2017-11-06T19:25:42Z

components/create_post/create_post.jsx

- this.showPostDeletedModal = this.showPostDeletedModal.bind(this);
- this.hidePostDeletedModal = this.hidePostDeletedModal.bind(this);
- this.showShortcuts = this.showShortcuts.bind(this);
- this.handleEmojiClick = this.handleEmojiClick.bind(this);
 this.handlePostError = this.handlePostError.bind(this);


Better if these remaining bind functions are changed into its equivalent arrow functions.

Yes of course.

saturninoabril · 2017-11-06T19:33:20Z

components/create_post/index.js

+ fullWidthTextBox: get(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.CHANNEL_DISPLAY_MODE, Preferences.CHANNEL_DISPLAY_MODE_DEFAULT) === Preferences.CHANNEL_DISPLAY_MODE_FULL_SCREEN,
+ showTutorialTip: get(state, Preferences.TUTORIAL_STEP, getCurrentUserId(state), 999),
+ messageInHistoryItem: makeGetMessageInHistoryItem('post')(state)
+ };


Same constant here.

saturninoabril · 2017-11-06T19:36:51Z

components/create_post/create_post.jsx

@@ -796,7 +846,3 @@ export default class CreatePost extends React.Component {
 );
 }
 }


Could you please add tests to CreatePost component?

Yes, I was hoping to add test cases was out on a trip. Just catching up with things. I will add test cases first chance I get.

saturninoabril · 2017-11-06T19:38:26Z

components/create_comment/create_comment.jsx

@@ -46,8 +45,6 @@ export default class CreateComment extends React.Component {
 constructor(props) {
 super(props);

- this.lastTime = 0;


Thanks for cleaning up!

saturninoabril · 2017-11-06T19:53:47Z

components/create_post/create_post.jsx

@@ -415,35 +468,42 @@ export default class CreatePost extends React.Component {
 }

 componentWillMount() {


I suggests to have these lifecycle methods to move at top location, just after the constructor and before other functions.

saturninoabril · 2017-11-06T20:00:08Z

components/create_post/create_post.jsx

- });
+ e.preventDefault();
+ if (e.keyCode === Constants.KeyCodes.UP) {
+ this.props.actions.moveHistoryIndexBack('post').then(() => this.fillMessageFromHistory());


Might be better to use constant here:

import {Posts} from 'mattermost-redux/constants'; ... ... moveHistoryIndexBack(Posts.MESSAGE_TYPES.POST)

saturninoabril · 2017-11-06T20:00:28Z

components/create_post/create_post.jsx

+ if (e.keyCode === Constants.KeyCodes.UP) {
+ this.props.actions.moveHistoryIndexBack('post').then(() => this.fillMessageFromHistory());
+ } else {
+ this.props.actions.moveHistoryIndexForward('post').then(() => this.fillMessageFromHistory());


Same constants here.

sudheerDev · 2017-11-10T07:42:37Z

components/create_comment/create_comment.jsx

@@ -46,8 +45,6 @@ export default class CreateComment extends React.Component {
 constructor(props) {
 super(props);

- this.lastTime = 0;
-
 PostStore.clearCommentDraftUploads();


Oo i could change this to use redux actions

jwilander · 2017-11-17T18:59:28Z

Hey @sudheerDev, just checking in on how this is going. Let us know if you need any help and when you're ready for review :)

sudheerDev · 2017-11-17T19:13:41Z

@jwilander I need to make few changes and add test cases as well. I will have a look at it over the weekend and let you know if I need any help.

jespino · 2017-11-21T07:27:14Z

Hi, I have changed a thing in create_post.jsx to fix the arrow up to edit. the PR is #339.

sudheerDev · 2017-11-21T07:48:50Z

@jespino Aah thanks. I came across this the other day and I thought I broke something.

sudheerDev · 2017-11-22T13:54:03Z

@saturninoabril @jwilander Still working on this. Need to add some more test cases and will be done. While writing test i am finding things that could be fixed :).

sudheerDev · 2017-11-23T18:03:42Z

This PR is a dependency for this PR.
And this PR is good to have so i can remove the dependency of postStore for the component.

* Fix typo in audit table component * Updating i18n strings as well, as requested

* Added PostMarkdown component * Fixed failing tests * Fixing styles

…most#420)

this blocks users from clicking messages directly behind the NewMessageIndicator compnent

lindalumitchell · 2017-12-13T22:44:54Z

@jwilander @sudheerDev sorry for the delay on testing this; it looks good to me!

Did a bunch of testing, including having multiple tabs open. Tested posts, replies, editing, deleting, long messages, formatted messages, pinning, flagging, autocomplete for emoji, users, channel names, emoji reactions, mentions, DMs, permalinks, all in combinations between two users and in center and RHS. No issues found!

* Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** * Fix Tests * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** * Fix Tests * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** * Fix Tests * Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display * Code Review Fixes * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** * Fix Tests * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** * Fix Tests * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** * Fix Tests * Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display * Code Review Fixes * WIP: Changes to testing Work in Progress, last two tests returning errors. * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display Code Review Fixes Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** WIP: Changes to testing Work in Progress, last two tests returning errors. Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display WIP: Changes to testing Work in Progress, last two tests returning errors. Code Review Fixes Merge remote-tracking branch 'avasconcelos114/PLT-7784' into PLT-7784 Fixed test errors Fix test name Merge remote-tracking branch 'avasconcelos114/PLT-7784' into PLT-7784 * Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display Code Review Fixes Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** WIP: Changes to testing Work in Progress, last two tests returning errors. Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display Code Review Fixes Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** WIP: Changes to testing Work in Progress, last two tests returning errors. Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Migrate do_verify_email to Redux **Should wait until enzyme & react version issues are resolved** Fix Tests Finalized component tests * Snapshot test * verifyEmail function test * Test mattermost-redux action & error display WIP: Changes to testing Work in Progress, last two tests returning errors. Code Review Fixes Merge remote-tracking branch 'avasconcelos114/PLT-7784' into PLT-7784 Fixed test errors Fix test name Merge remote-tracking branch 'avasconcelos114/PLT-7784' into PLT-7784 Remove useless Adapter import in test Merge remote-tracking branch 'avasconcelos114/PLT-7784' into PLT-7784 Merge remote-tracking branch 'avasconcelos114/PLT-7784' into PLT-7784 * Fix merge issue

saturninoabril · 2017-12-14T10:04:39Z

@sudheerDev Please give this a rebase and we're good to merge this. Thanks!

Remove channelId and channel from state as it is avaialable via props. Remove Team store dependency. Remove User store dependency Remove Preferences store dependency. * Remove preferences from store as props change trigger render. * Add selector for preferences Move REACTION_PATTERN to utils. Remove message_history_store dependency. Add redux post actions instead of flux actions. Change bind functions to class prop arrow functions. Add proptype comments. Remove couple of unused variables. * Remove all bind functions. * Add MESSAGE_TYPES constants. Remove few postStore dependent functions. Add getCurrentUsersLatestPostId redux state selector. Add Test cases for create_post Remove uploadInprogress and filesInfo from state Remove unused variable from file_preview which causes mutation on props. Fix sort on filesInfo for file_preview component. Fix draft constant Remove postStore dependecy. Add edit dispatch from redux. Add test cases Fix failing snapshots.

Change currentChannelMembersCount default value to 1 as user is part of the channel.

…ad creation it is undefined.

…ebapp into PLT-7238

This reverts commit 8292b1c.

mattermod · 2017-12-22T00:28:20Z

Spinmint test server destroyed

…channel (#210)

coreyhulen added 2: Dev Review Requires review by a core commiter Work in Progress Not yet ready for review labels Oct 27, 2017

coreyhulen requested review from jwilander and saturninoabril October 27, 2017 15:05

coreyhulen added the Hacktoberfest label Oct 27, 2017

coreyhulen changed the title ~~PLT 7238~~ PLT-7238 Migration of create_post.jsx to redux Oct 27, 2017

sudheerDev force-pushed the PLT-7238 branch 6 times, most recently from b465460 to cc63407 Compare November 3, 2017 12:32

saturninoabril suggested changes Nov 6, 2017

View reviewed changes

sudheerDev commented Nov 10, 2017

View reviewed changes

sudheerDev force-pushed the PLT-7238 branch 8 times, most recently from 726f8f8 to 1e6d361 Compare November 17, 2017 03:22

sudheerDev force-pushed the PLT-7238 branch from 1e6d361 to 8b4abf2 Compare November 20, 2017 20:50

sudheerDev force-pushed the PLT-7238 branch from b6d5afe to cd3364e Compare November 23, 2017 18:08

crspeller and others added 9 commits December 11, 2017 15:49

Merge branch 'release-4.5'

59b6321

add test to AccountSettingsDisplay modal (mattermost#440)

735eb23

change inline arrow function in ref (mattermost#449)

d9ac963

Fix jenkinsfile s3 push (mattermost#452)

a9e713a

Fix typo in audit table component (mattermost#427)

11dcf7a

* Fix typo in audit table component * Updating i18n strings as well, as requested

Added PostMarkdown component (mattermost#404)

97cb44a

* Added PostMarkdown component * Fixed failing tests * Fixing styles

add unit test to AddUsersToTeam and fix function/unused codes (matter…

191a102

…most#420)

Remove div container taking up width (mattermost#425)

b28ae6e

this blocks users from clicking messages directly behind the NewMessageIndicator compnent

Fix mutation on draft file remove

e651617

sudheer added 7 commits December 14, 2017 15:38

Fix review comments

dbc4b53

Fix member count in render for notifying user on @ALL

1c59809

Fix test cases

9ef2cdc

Change currentChannelMembersCount default value to 1 as user is part of the channel.

Fix for the value getCurrentChannelStats as on team creation and thre…

1de3bd3

…ad creation it is undefined.

Fix mutation on draft file remove

6caf4a7

Merge branch 'PLT-7238' of https://github.com/sudheerDev/mattermost-w…

ccbb95b

…ebapp into PLT-7238

saturninoabril merged commit 8292b1c into mattermost:master Dec 14, 2017

saturninoabril added a commit that referenced this pull request Dec 14, 2017

Revert "PLT-7238 Migration of create_post.jsx to redux (#210)"

67251f3

This reverts commit 8292b1c.

saturninoabril mentioned this pull request Dec 14, 2017

[PLT-7238] Migrate create_post component to redux #470

Closed

5 tasks

lindalumitchell added the Tests/Done Release tests have been written label Dec 15, 2017

saturninoabril added a commit that referenced this pull request Dec 18, 2017

Revert "PLT-7238 Migration of create_post.jsx to redux (#210)"

77af514

This reverts commit 8292b1c.

hmhealey removed the Setup Old Test Server Triggers the creation of a test server label Dec 22, 2017

jasonblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 4, 2018

hmhealey pushed a commit that referenced this pull request Aug 28, 2020

Fix thread selector when postsInChannel does not have post Ids for a …

a57820d

…channel (#210)

hmhealey pushed a commit that referenced this pull request Mar 17, 2021

Fix thread selector when postsInChannel does not have post Ids for a …

8f2f978

…channel (#210)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PLT-7238 Migration of create_post.jsx to redux #210

PLT-7238 Migration of create_post.jsx to redux #210

sudheerDev commented Oct 27, 2017 •

edited

Loading

saturninoabril Nov 6, 2017

saturninoabril Nov 6, 2017

sudheerDev Nov 7, 2017

saturninoabril Nov 6, 2017 •

edited

Loading

saturninoabril Nov 6, 2017

sudheerDev Nov 7, 2017

saturninoabril Nov 6, 2017

saturninoabril Nov 6, 2017

saturninoabril Nov 6, 2017 •

edited

Loading

saturninoabril Nov 6, 2017

sudheerDev Nov 10, 2017 •

edited

Loading

jwilander commented Nov 17, 2017

sudheerDev commented Nov 17, 2017

jespino commented Nov 21, 2017

sudheerDev commented Nov 21, 2017

sudheerDev commented Nov 22, 2017

sudheerDev commented Nov 23, 2017

lindalumitchell commented Dec 13, 2017

saturninoabril commented Dec 14, 2017

mattermod commented Dec 22, 2017

@@ @@ -796,7 +846,3 @@ export default class CreatePost extends React.Component { @@
  );
  }
 }

		@@ -415,35 +468,42 @@ export default class CreatePost extends React.Component {
		}

		componentWillMount() {

PLT-7238 Migration of create_post.jsx to redux #210

PLT-7238 Migration of create_post.jsx to redux #210

Conversation

sudheerDev commented Oct 27, 2017 • edited Loading

Summary

Ticket Link

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saturninoabril Nov 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saturninoabril Nov 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudheerDev Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

jwilander commented Nov 17, 2017

sudheerDev commented Nov 17, 2017

jespino commented Nov 21, 2017

sudheerDev commented Nov 21, 2017

sudheerDev commented Nov 22, 2017

sudheerDev commented Nov 23, 2017

lindalumitchell commented Dec 13, 2017

saturninoabril commented Dec 14, 2017

mattermod commented Dec 22, 2017

sudheerDev commented Oct 27, 2017 •

edited

Loading

saturninoabril Nov 6, 2017 •

edited

Loading

saturninoabril Nov 6, 2017 •

edited

Loading

sudheerDev Nov 10, 2017 •

edited

Loading