Skip to content

Comments

[IMPROVE] Allow e-mail channel to be used without default department.#23945

Merged
cauefcr merged 2 commits intodevelopfrom
omni-email-with-no-dept
Dec 15, 2021
Merged

[IMPROVE] Allow e-mail channel to be used without default department.#23945
cauefcr merged 2 commits intodevelopfrom
omni-email-with-no-dept

Conversation

@cauefcr
Copy link
Contributor

@cauefcr cauefcr commented Dec 14, 2021

Proposed changes (including videos or screenshots)

Due to a missing condition in the e-mail input processing, Rocket.Chat was unable to receive e-mails from e-mail channels that did not have a default department.

Issue(s)

The following error is raised when an email message is sent to an email account without default department

W20211206-13:46:39.202(-3)? (STDERR) === UnHandledPromiseRejection ===
W20211206-13:46:39.204(-3)? (STDERR) errorClass [Error]: Provided department does not exists [invalid-department]
W20211206-13:46:39.204(-3)? (STDERR) at Object.setDepartmentForGuest (app/livechat/server/lib/Livechat.js:332:10)
W20211206-13:46:39.204(-3)? (STDERR) at getGuestByEmail (server/features/EmailInbox/EmailInbox_Incoming.ts:46:13)
W20211206-13:46:39.204(-3)? (STDERR) at server/features/EmailInbox/EmailInbox_Incoming.ts:134:16
W20211206-13:46:39.204(-3)? (STDERR) at /Users/renatobecker/.meteor/packages/promise/.0.11.2.q9g02k.d69vo++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40 {
W20211206-13:46:39.204(-3)? (STDERR) isClientSafe: true,
W20211206-13:46:39.204(-3)? (STDERR) error: 'invalid-department',
W20211206-13:46:39.205(-3)? (STDERR) reason: 'Provided department does not exists',
W20211206-13:46:39.205(-3)? (STDERR) details: { method: 'setDepartmentForGuest' },
W20211206-13:46:39.205(-3)? (STDERR) errorType: 'Meteor.Error'
W20211206-13:46:39.205(-3)? (STDERR) }
W20211206-13:46:39.205(-3)? (STDERR) ---------------------------------
W20211206-13:46:39.205(-3)? (STDERR) Errors like this can cause oplog processing errors.
W20211206-13:46:39.205(-3)? (STDERR) Setting EXIT_UNHANDLEDPROMISEREJECTION will cause the process to exit allowing your service to automatically restart the process
W20211206-13:46:39.205(-3)? (STDERR) Future node.js versions will automatically exit the process
W20211206-13:46:39.205(-3)? (STDERR) =================================

Steps to test or reproduce

By creating an e-mail channel and not giving it a default department the above error would occur, dropping e-mail messages from Rocket.Chat, with only a console log for an error.

Further comments

@cauefcr cauefcr requested review from a team and renatobecker December 14, 2021 18:34
Copy link
Contributor

@murtaza98 murtaza98 left a comment

Choose a reason for hiding this comment

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

@cauefcr I found an edge case with your implementation where if the email was first connected to a department, let's say sales, and then a room was closed within this department - Post this, the email was then unassigned from any department on Rocket.Chat, and if the same chat arrives back, ideally I think we shouldn't associate this new chat to the sales department since there is no department connected to the email - but in your implementation, it was still associating it to the Sales department.

Hopefully, this PR should fix this - #23950

@cauefcr
Copy link
Contributor Author

cauefcr commented Dec 15, 2021

Thanks for the PR! I didn't test this edge case.

@cauefcr cauefcr requested a review from murtaza98 December 15, 2021 13:35
@cauefcr cauefcr dismissed murtaza98’s stale review December 15, 2021 13:36

The requested changes were merged

@cauefcr cauefcr merged commit f8be788 into develop Dec 15, 2021
@cauefcr cauefcr deleted the omni-email-with-no-dept branch December 15, 2021 15:23
@sampaiodiego sampaiodiego mentioned this pull request Dec 29, 2021
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