fix(autotranslate): always remove existing callback before re-registering#41330
fix(autotranslate): always remove existing callback before re-registering#41330prathamesh04 wants to merge 1 commit into
Conversation
…ring When switching AutoTranslate providers, the old callback was not being removed because callbacks.add() with an existing ID silently returns a removal function instead of replacing. This caused translations to continue using the old provider until server restart. Fixes RocketChat#41197
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
WalkthroughAuto-translate callback registration now removes the existing callback before evaluating whether auto-translate is enabled, allowing provider changes to replace stale message translation callbacks. ChangesAuto-translate callback lifecycle
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Proposed changes
When switching AutoTranslate providers while enabled, translations continue using the old provider until the server is restarted.
Root cause: In
TranslationProviderRegistry.registerCallbacks(), the oldafterSaveMessagecallback was only removed when auto-translate was disabled. When the provider changed,callbacks.add()was called with the existing idautotranslate, butcallbacks.add()silently returns a removal function instead of replacing when the id already exists. So the old callback (capturing the previous provider in its closure) was never replaced.Fix: Always call
callbacks.remove('afterSaveMessage', 'autotranslate')before adding the new callback, regardless of whether auto-translate is being enabled or the provider is being changed.Issue(s)
Fixes #41197
Steps to test or reproduce
Further comments
apps/meteor/app/autotranslate/server/autotranslate.tscallbacks.remove()call to always execute beforecallbacks.add(), instead of only when disablingSummary by CodeRabbit