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

Migrate createPost to redux #2005

Merged
merged 3 commits into from
Nov 6, 2018
Merged

Migrate createPost to redux #2005

merged 3 commits into from
Nov 6, 2018

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Nov 5, 2018

Summary

Affected areas:

  • Post a message from Center channel
  • Post a message from the RHS

Ticket Link

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

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Added or updated unit tests (required for all new features)

@enahum enahum added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Nov 5, 2018
@enahum enahum added this to the v5.6.0 milestone Nov 5, 2018
@@ -51,7 +51,7 @@ jest.mock('actions/global_actions.jsx', () => ({
}));

jest.mock('actions/post_actions.jsx', () => ({
createPost: jest.fn(),
createPost: jest.fn(() => ({type: 'MOCK_CREATE_POST'})),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
createPost: jest.fn(() => ({type: 'MOCK_CREATE_POST'})),
createPost: jest.fn((...args) => ({type: 'MOCK_CREATE_POST', args})),

I think it should have args so we can validate the params passed into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the problem here is that the args are being generated in submitPost and is adding a pending_post_id and a timestamp that I don't really know who to mock

for (const emoji of emojis) {
const trimmed = emoji.substring(1, emoji.length - 1);
emitEmojiPosted(trimmed);
export function createPost(post, files) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add test to createPost?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

if (emojis) {
for (const emoji of emojis) {
const trimmed = emoji.substring(1, emoji.length - 1);
dispatch(addRecentEmoji(trimmed));
Copy link
Member

Choose a reason for hiding this comment

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

This should be doDispatch right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually I'm going to get rid of the global dispatch in this file

@enahum
Copy link
Contributor Author

enahum commented Nov 6, 2018

@jwilander changes made

@enahum enahum added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Nov 6, 2018
@enahum enahum removed the request for review from DHaussermann November 6, 2018 16:08
@enahum enahum merged commit cd1ec53 into master Nov 6, 2018
@enahum enahum deleted the MM-12711 branch November 6, 2018 16:08
@lindy65 lindy65 removed the 4: Reviews Complete All reviewers have approved the pull request label Nov 7, 2018
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Nov 16, 2018
@DHaussermann DHaussermann added the Tests/Done Release tests have been written label Nov 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation Tests/Done Release tests have been written
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants