feat(Omnichannel): System messages in transcripts

[yash-rajpal]

Proposed changes (including videos or screenshots)

Currently we don't handle system messages in our transcripts, the system messages are shown as empty space in our transcripts.

This PR adds this support to handle system generated messages for both, email and PDF transcripts.

Issue(s)

Steps to test or reproduce

Further comments

CORE-494

[changeset-bot]

🦋 Changeset detected

Latest commit: aeabfa9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 34 packages

Name	Type
@rocket.chat/omnichannel-services	Minor
@rocket.chat/pdf-worker	Minor
@rocket.chat/core-services	Minor
@rocket.chat/model-typings	Minor
@rocket.chat/i18n	Minor
@rocket.chat/meteor	Minor
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/stream-hub-service	Patch
@rocket.chat/presence	Patch
rocketchat-services	Patch
@rocket.chat/apps	Patch
@rocket.chat/models	Patch
@rocket.chat/mock-providers	Patch
@rocket.chat/ui-contexts	Major
@rocket.chat/web-ui-registration	Major
@rocket.chat/cron	Patch
@rocket.chat/instance-status	Patch
@rocket.chat/fuselage-ui-kit	Major
@rocket.chat/ui-client	Major
@rocket.chat/gazzodown	Major
@rocket.chat/livechat	Patch
@rocket.chat/ui-avatar	Major
@rocket.chat/ui-video-conf	Major
@rocket.chat/uikit-playground	Patch
@rocket.chat/core-typings	Minor
@rocket.chat/rest-typings	Minor
@rocket.chat/api-client	Patch
@rocket.chat/license	Patch
@rocket.chat/ddp-client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dionisio-bot]

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 55.47%. Comparing base (b4bbcbf) to head (aeabfa9).
Report is 2 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #32752   +/-   ##
========================================
  Coverage    55.46%   55.47%           
========================================
  Files         2635     2635           
  Lines        57343    57343           
  Branches     11874    11876    +2     
========================================
+ Hits         31807    31811    +4     
- Misses       22842    22853   +11     
+ Partials      2694     2679   -15     

Flag	Coverage Δ
e2e	54.02% <ø> (-0.03%)	⬇️
unit	72.27% <33.33%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Fxied

[ggazzo]

As your pr has changes related with performance, I don't think we need a service to deal with translations, I think everything could be moved to a lib and all services could use it, that would be much faster. what do you think?

[yash-rajpal]

@ggazzo Do you mean moving all i18n logic to the package itself, so all packages can use?

If so, then that's a great improvement and we should definitely do that, but I think it is outside the scope of this PR and should be handled separately. Currently this PR is important for this release. Maybe this can be a future overall improvement. What do you think?

[ggazzo]

@ggazzo Do you mean moving all i18n logic to the package itself, so all packages can use?

yep, its not necessarily huge code. I imagine this service was created before we removed meteor dependencies, nowadays, it would make much more sense to use this as a lib

but I think it is outside the scope of this PR and should be handled separately.

you just explained exactly why I would keep your performance part in another PR, separate from the feature ;)

[KevLehman]

Vote 3 for removing service. It was created when translations were handled by meteor. Now that the files are free of meteor we could move the actual i18n thing onto it.

Also vote of doing that on a separate PR 👀

[yash-rajpal]

you just explained exactly why I would keep your performance part in another PR, separate from the feature ;)

You are right, but earlier I implemented the feature without the performance part, but the implementation created performance issues as we were having to do a lot of i18n calls for each and every message, making entire process async (as getting translations was async). The performance part is just making this process synchronous using some tricks.

So, it is kinda needed, can't remove it for now to do in future.

But for sure this translation service should be removed and should be handled by a lib, when doing this we can improve transcripts as well. What do you think?

[kodiakhq]

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.