Skip to content

Conversation

@TemaSM
Copy link

@TemaSM TemaSM commented Jun 5, 2020

Proposed changes

Probably fix for slack's mpims importer behavior introduced by f6d11e2

Issue(s)

Related issue's comment: #17793 (comment)

How to test or reproduce

Start importing Slack history, which includes mpims and it will fail with:

Slack Importer  info 130 messages imported
server.js:204 Slack Importer  error TypeError: Cannot read property 'get' of undefined
    at SlackImporter._importMpims (app/importer-slack/server/importer.js:637:29)
    at app/importer-slack/server/importer.js:838:10
    at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
    at packages/meteor.js:550:25
    at runWithEnvironment (packages/meteor.js:1286:24)
Setting default file store to GridFS
Rocket.Chat Version: 3.3.0
NodeJS Version: 12.16.1 - x64
MongoDB Version: 4.0.18
MongoDB Engine: wiredTiger
Platform: linux
ReplicaSet OpLog: Enabled
Commit Hash: a987295718
Commit Branch: HEAD

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)

Checklist

Changelog

Fix slack's mpims import: Cannot read property 'get' of undefined

Further comments

@TemaSM
Copy link
Author

TemaSM commented Jun 5, 2020

Okay, my PR is duplicate of: #17776, but it still not merged 😞
UPD: Tested, works fine

@lgtm-com
Copy link

lgtm-com bot commented Jun 5, 2020

This pull request fixes 1 alert when merging d876542 into 2abe15d - view on LGTM.com

fixed alerts:

  • 1 for Property access on null or undefined

@lgtm-com
Copy link

lgtm-com bot commented Jun 6, 2020

This pull request fixes 1 alert when merging cc2da73 into e01297c - view on LGTM.com

fixed alerts:

  • 1 for Property access on null or undefined

@TemaSM
Copy link
Author

TemaSM commented Jun 8, 2020

Another Slack's importer issue: #17855

@sampaiodiego
Copy link
Member

thanks @TemaSM , I'll merge the other PR.. thanks for pointing it out

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.

2 participants