Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: chat.react api not accepting previous emojis #10290

Merged
merged 3 commits into from
Apr 3, 2018
Merged

Conversation

graywolf336
Copy link
Contributor

@RocketChat/core

Closes #10286

This happened because verification was added which checked if the emoji passed actually is one. However, this verification did not account for missing colons which has a very high chance to break existing systems.

@graywolf336 graywolf336 added this to the 0.63.0 milestone Apr 1, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10290 April 1, 2018 20:17 Inactive
geekgonecrazy
geekgonecrazy previously approved these changes Apr 1, 2018
@geekgonecrazy geekgonecrazy changed the title Fix: chat.react api not accepting previous emojis [Fix] chat.react api not accepting previous emojis Apr 1, 2018
@geekgonecrazy
Copy link
Contributor

@sampaiodiego @rodrigok important to get this one in. Otherwise we break mobile apps, and aren't compatible with anything using the reaction api.

@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-10290 April 2, 2018 16:31 Inactive
sampaiodiego
sampaiodiego previously approved these changes Apr 2, 2018
@sampaiodiego
Copy link
Member

added tests to cover this regression.. pls review

Copy link
Contributor

@geekgonecrazy geekgonecrazy left a comment

Choose a reason for hiding this comment

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

If we wanted to also test that it doesn't accept invalid emoji that might not be bad either. But at least this will keep from breaking expected functionality again

@@ -39,8 +39,6 @@ Meteor.methods({
return false;
}

reaction = `:${ reaction.replace(/:/g, '') }:`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh I knew we had this code in place somewhere, but I just didn't open my eyes enough to see that it was a few lines below.

@rodrigok rodrigok changed the title [Fix] chat.react api not accepting previous emojis Fix: chat.react api not accepting previous emojis Apr 3, 2018
@rodrigok rodrigok merged commit 0afab32 into develop Apr 3, 2018
@rodrigok rodrigok deleted the fix-react-api branch April 3, 2018 14:07
@rodrigok rodrigok mentioned this pull request Apr 4, 2018
trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request Apr 6, 2018
* develop: (91 commits)
  [FIX] Audio Message UI fixes (RocketChat#10303)
  [NEW] Improve history generation (RocketChat#10319)
  Strip the colons when searching (RocketChat#10323)
  Bump snap version to include security fix (RocketChat#10313)
  Update Meteor to 1.6.1.1 (RocketChat#10314)
  [FIX] Unable to mention after newline in message (RocketChat#10078)
  [FIX] Wrong pagination information on /api/v1/channels.members (RocketChat#10224)
  [FIX] Inline code following a url leads to autolinking of code with url (RocketChat#10163)
  Fixed issue on incoming webhooks that caused the content raw to not be available (RocketChat#10258)
  [FIX] Missing Translation Key on Reactions (RocketChat#10270)
  Fix: inputs for rocketchat apps (RocketChat#10274)
  Fix: chat.react api not accepting previous emojis (RocketChat#10290)
  Fix: Scroll on content page (RocketChat#10300)
  [FIX] File had redirect delay when using external storage services and no option to proxy only avatars
  Add quotes in the endpoint deprecation warning
  Add deprecation warning to user.roles endpoint
  Improve pt-BR translations
  Fix caddy download link to pull from github
  Renaming channels.notifications Get/Post endpoints to subscriptions.getOne and rooms.saveNotifications
  Revert the removed endpoint, user.roles
  ...

# Conflicts:
#	package-lock.json
trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request Apr 6, 2018
* goalify: (200 commits)
  Add S3 upload script
  Remove root-priviledged command
  switch gitlab ci config to use shell executor
  Try another build
  fix change language not reload
  temporary disable readonly
  [FIX] Audio Message UI fixes (RocketChat#10303)
  [NEW] Improve history generation (RocketChat#10319)
  Strip the colons when searching (RocketChat#10323)
  Bump snap version to include security fix (RocketChat#10313)
  Update Meteor to 1.6.1.1 (RocketChat#10314)
  [FIX] Unable to mention after newline in message (RocketChat#10078)
  [FIX] Wrong pagination information on /api/v1/channels.members (RocketChat#10224)
  [FIX] Inline code following a url leads to autolinking of code with url (RocketChat#10163)
  Fixed issue on incoming webhooks that caused the content raw to not be available (RocketChat#10258)
  [FIX] Missing Translation Key on Reactions (RocketChat#10270)
  Fix: inputs for rocketchat apps (RocketChat#10274)
  Fix: chat.react api not accepting previous emojis (RocketChat#10290)
  Fix: Scroll on content page (RocketChat#10300)
  change weather city to location
  ...

# Conflicts:
#	.docker/Dockerfile
#	.sandstorm/sandstorm-pkgdef.capnp
#	.travis/snap.sh
#	HISTORY.md
#	package.json
#	packages/rocketchat-lib/rocketchat.info
#	packages/rocketchat-lib/server/functions/deleteUser.js
#	packages/rocketchat-theme/client/imports/components/main-content.css
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] API chat.react is not working anymore
6 participants