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
Feat: detect language of the message content #6660
Conversation
✅ Deploy Preview for chatwoot-storybook ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
aa6a233
to
7c72994
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- added some comments
… into feat/detect-lang
… into feat/detect-lang
442722d
to
13ee2fc
Compare
e733509
to
4d5bf2e
Compare
@tejaswini, the code looks good to me. But not super sure about the behaviour here. The more I look into this PR, it looks like an extension of the existing translation integration. Rather than mixing up the configs between the integration hook and accounts.
Note: the language attribute could also be a part of the conversation object so that we can avoid API calls on every message. But rather do it on the first user-generated message and update the conversation. I would wait on the product decision before merging this. @pranavrajs thoughts? |
@sojan-official We can tie this over automation rule and report spam or resolve it. I am removing the settings; in this PR we can just have detectLanguageJob. We can make it available only for |
… into feat/detect-lang
|
||
event = Events::Base.new('conversation_updated', Time.zone.now, { conversation: conversation }) | ||
|
||
AutomationRuleListener.instance.conversation_updated(event) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sojan-official I Had to add this manually as we don't support the 'conversation_updated' event in additional_attributes.
def notify_conversation_updation
return unless previous_changes.keys.present? && (previous_changes.keys & %w[team_id assignee_id status snoozed_until custom_attributes label_list first_reply_created_at]).present?
dispatcher_dispatch(CONVERSATION_UPDATED, previous_changes)
end
I didn't want to call it for all other listeners and grab the impact on this change.
Without this, it will work for other attributes on the conversation when you update something manually on the dashboard, not when you detect and update the language and then want to run the automation based on that. So for this case, I added the line.
Let me know wdyt?
… into feat/detect-lang
… into feat/detect-lang
Note to self: ref: #6660 (comment) It feels wrong to trigger the update event directly from the integration processor. Lets do that from the conversation model itself by expanding update event to chatwoot/app/models/conversation.rb Line 214 in 21da03f
chatwoot/spec/models/conversation_spec.rb Line 157 in 21da03f
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes: https://linear.app/chatwoot/issue/CW-46/reduce-spam
Screen.Recording.2023-03-14.at.2.28.10.PM.mov