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] Check attachments is defined before accessing first element #8295

Merged

Conversation

Darkneon
Copy link
Contributor

@RocketChat/core

Prevents an error if attachments is undefined. It's not possible to create an empty message with no attachments in Rocket.Chat at the moment so this is mainly for those that are building on top of Rocket.Chat and are extending the message model.

@Darkneon Darkneon changed the title Fix: check attachments is defined before accessing first element [FIX] check attachments is defined before accessing first element Sep 25, 2017
@Darkneon Darkneon changed the title [FIX] check attachments is defined before accessing first element [FIX] Check attachments is defined before accessing first element Sep 25, 2017
Copy link
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

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

This was done in a branch of my own, but glad to see it here.

@graywolf336
Copy link
Contributor

@rodrigok can we get this in the next pre-release, as it is a bug that can cause data loss.

@Darkneon
Copy link
Contributor Author

@graywolf336 what's the name of your branch?

I just noticed https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-integrations/server/processWebhookMessage.js#L44, maybe it would be better to assign it to an empty array?

@graywolf336
Copy link
Contributor

@Darkneon it's intermixed with a new feature, so don't worry about it.

I would keep it like it is where you're checking it at. This way the attachments property isn't saved to the database if there are none that exist.

@rodrigok
Copy link
Member

@graywolf336 Sure

@rodrigok rodrigok added this to the 0.59.0-rc.9 milestone Sep 25, 2017
@graywolf336
Copy link
Contributor

@rodrigok thanks!

@rodrigok rodrigok merged commit b16ff09 into RocketChat:develop Sep 25, 2017
rodrigok added a commit that referenced this pull request Sep 26, 2017
…ments

[FIX] Check attachments is defined before accessing first element
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.

3 participants