Skip to content
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

fix: Application crashes when clicking on Analytics option in Omnichannel view page #29286

Merged
merged 6 commits into from
May 24, 2023

Conversation

anikdhabal
Copy link
Contributor

@anikdhabal anikdhabal commented May 18, 2023

Proposed changes (including videos or screenshots)

Issue(s)

fix #29272
OC-953

Steps to test or reproduce

Further comments

@anikdhabal anikdhabal requested a review from a team as a code owner May 18, 2023 16:28
@changeset-bot
Copy link

changeset-bot bot commented May 18, 2023

🦋 Changeset detected

Latest commit: 21cd09d

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

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

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #29286 (11f399f) into develop (89e2c26) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #29286      +/-   ##
===========================================
+ Coverage    46.77%   46.82%   +0.04%     
===========================================
  Files          707      707              
  Lines        13239    13239              
  Branches      2221     2221              
===========================================
+ Hits          6193     6199       +6     
+ Misses        6732     6722      -10     
- Partials       314      318       +4     
Flag Coverage Δ
e2e 46.79% <ø> (+0.05%) ⬆️

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

@yash-rajpal yash-rajpal requested a review from a team May 18, 2023 19:29
yash-rajpal
yash-rajpal previously approved these changes May 18, 2023
@KevLehman
Copy link
Member

Hey! Thanks for the contribution. The PR indeed solves the crash problem, but it also removes the translations for the server-returned strings:
image

In this case, %_of_conversations should translate to % of conversations (and other languages too) but the string is left as it is. Would you mind checking? 🙌🏽 thanks!

@jayesh-jain252
Copy link
Contributor

The presence of the '%' symbol in the "% of Conversations" at "apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json" file is probably causing the problem.

image

Escaping the "%" like this solves the issue

image

Reference: martinandert/counterpart#37

image

@KevLehman is this solution accurate and valid?

@anikdhabal
Copy link
Contributor Author

Hey! Thanks for the contribution. The PR indeed solves the crash problem, but it also removes the translations for the server-returned strings: image

In this case, %_of_conversations should translate to % of conversations (and other languages too) but the string is left as it is. Would you mind checking? 🙌🏽 thanks!

sure sir!

@anikdhabal
Copy link
Contributor Author

anikdhabal commented May 19, 2023

Yes, percentage signs need to be escaped with another percentage sign in the json file. Sorry for my mistake.

@murtaza98
Copy link
Contributor

Yes, percentage signs need to be escaped with another percentage sign in the json file. Sorry for my mistake.

Hey @anikdhabal No need to apologize, we're all learning here. Thanks for letting us know about the percentage sign issue in the json file. Could you please inform us when you're done with the suggestion? We appreciate your help! 😊

@anikdhabal
Copy link
Contributor Author

Yes, percentage signs need to be escaped with another percentage sign in the json file. Sorry for my mistake.

Hey @anikdhabal No need to apologize, we're all learning here. Thanks for letting us know about the percentage sign issue in the json file. Could you please inform us when you're done with the suggestion? We appreciate your help! 😊

Yes sir ☺️,I will fix it by today.

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels May 23, 2023
@murtaza98 murtaza98 added this to the 6.3.0 milestone May 24, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels May 24, 2023
@kodiakhq kodiakhq bot merged commit 7e00009 into RocketChat:develop May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad: omnichannel stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Application crashes when clicking on Analytics option in Omnichannel view page
5 participants