Skip to content

Conversation

@aleksandernsilva
Copy link
Contributor

@aleksandernsilva aleksandernsilva commented Sep 20, 2022

OC-209

This pull request fixes:

  • Fallbacks from a few livechat methods to take into account null values and fallback to the correct default value instead of breaking the widget.
  • defaultAgent not being cleared from cache when a new department is set
  • default guest information not being displayed

@codecov
Copy link

codecov bot commented Sep 20, 2022

Codecov Report

Merging #26909 (c2e9a29) into develop (74cd22b) will increase coverage by 1.08%.
The diff coverage is n/a.

❗ Current head c2e9a29 differs from pull request most recent head a65fa80. Consider uploading reports for the commit a65fa80 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #26909      +/-   ##
===========================================
+ Coverage    39.90%   40.98%   +1.08%     
===========================================
  Files          826      801      -25     
  Lines        18373    17920     -453     
  Branches      2019     1960      -59     
===========================================
+ Hits          7332     7345      +13     
+ Misses       10742    10279     -463     
+ Partials       299      296       -3     
Flag Coverage Δ
e2e 40.98% <ø> (+1.08%) ⬆️

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

@aleksandernsilva aleksandernsilva marked this pull request as ready for review September 21, 2022 11:40
@aleksandernsilva aleksandernsilva requested review from a team as code owners September 21, 2022 11:40
@aleksandernsilva aleksandernsilva added this to the 5.2.0 milestone Sep 21, 2022
tiagoevanp
tiagoevanp previously approved these changes Sep 21, 2022
ggazzo
ggazzo previously approved these changes Sep 21, 2022
tassoevan
tassoevan previously approved these changes Sep 21, 2022
Copy link
Contributor

@tassoevan tassoevan left a comment

Choose a reason for hiding this comment

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

Nullish coalescing (e.g. a ?? b) could be better, but || is fine as we're expecting objects. Just keep it in mind when dealing with falsy primitives.

@aleksandernsilva aleksandernsilva removed this from the 5.2.0 milestone Sep 22, 2022
@aleksandernsilva aleksandernsilva added this to the 5.2.0 milestone Sep 23, 2022
MartinSchoeler
MartinSchoeler previously approved these changes Sep 23, 2022
@tassoevan tassoevan modified the milestones: 5.2.0, 5.2.1 Sep 23, 2022
@aleksandernsilva aleksandernsilva modified the milestone: 5.2.1 Sep 23, 2022
@aleksandernsilva aleksandernsilva removed this from the 5.2.1 milestone Sep 26, 2022
@lgtm-com
Copy link

lgtm-com bot commented Sep 26, 2022

This pull request introduces 1 alert when merging 05dcba1 into ca43434 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

MartinSchoeler
MartinSchoeler previously approved these changes Sep 26, 2022
@ggazzo ggazzo added the stat: ready to merge PR tested and approved waiting for merge label Sep 27, 2022
@aleksandernsilva aleksandernsilva modified the milestones: 5.2.0, 5.1.4 Sep 27, 2022
@tassoevan tassoevan merged commit 3a2b6da into develop Sep 27, 2022
@tassoevan tassoevan deleted the fix/livechat-null-room branch September 27, 2022 18:51
tassoevan added a commit that referenced this pull request Sep 27, 2022
…26909)

* [FIX] Adjusted livechat fallbacks to take null values into account

* [FIX] Adjusted to clear defaultAgent when department is changed

* [FIX] Fixed default guest info not being displayed

* [FIX] Adjusted widget default values logic

* [FIX] Removing unnecessary check

* [FIX] Changed default values logic again to account for changes via api

Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
@tassoevan tassoevan mentioned this pull request Sep 27, 2022
@tassoevan tassoevan mentioned this pull request Oct 13, 2022
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.

5 participants