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.

Sign up for GitHub

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

Jump to bottom

Chore: Move voip's Wrap-up and On-hold functionality to EE (Backend) #25160

Merged
merged 16 commits into from
Jun 9, 2022
Merged

Chore: Move voip's Wrap-up and On-hold functionality to EE (Backend) #25160

merged 16 commits into from
Jun 9, 2022

Conversation

murtaza98
Copy link
Contributor

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@murtaza98 murtaza98 requested a review from a team April 12, 2022 09:47
@KevLehman

This comment was marked as resolved.

Sign in to view
@murtaza98 murtaza98 requested a review from a team as a code owner April 21, 2022 12:01
@KevLehman

This comment was marked as resolved.

Sign in to view
KevLehman
KevLehman reviewed May 4, 2022
View reviewed changes
Copy link
Contributor

@KevLehman KevLehman left a comment

Choose a reason for hiding this comment

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

Currently on CE, it shows you the modal to close the conversation but doesn't store the data. Would be better to not to show the modal at all 👀

The other thing: when looking at the calls panel, the call has both values as "Not available", would be good to not to show them if there's no valid license, maybe @MartinSchoeler can help you with how to achieve that 🤗

ggazzo
ggazzo previously requested changes May 4, 2022
View reviewed changes
Copy link
Member

@ggazzo ggazzo left a comment

Choose a reason for hiding this comment

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

could you check my comment? thanks

apps/meteor/app/api/server/v1/voip/rooms.ts Outdated Show resolved Hide resolved
packages/core-typings/src/IMessage/IMessage.ts Outdated Show resolved Hide resolved
@murtaza98
Copy link
Contributor Author

Currently on CE, it shows you the modal to close the conversation but doesn't store the data. Would be better to not to show the modal at all eyes

The other thing: when looking at the calls panel, the call has both values as "Not available", would be good to not to show them if there's no valid license, maybe @MartinSchoeler can help you with how to achieve that hugs

Well, both these behaviours were actually intentional since I was planning to keep this PR isolated to backend only change. However, I don't mind adding the frontend changes too in here if the frontend guys are okay with it. @MartinSchoeler WDYT?

@murtaza98 murtaza98 requested a review from a team as a code owner May 24, 2022 11:45
@murtaza98
Copy link
Contributor Author

murtaza98 commented May 26, 2022 •
edited
Loading

Currently on CE, it shows you the modal to close the conversation but doesn't store the data. Would be better to not to show the modal at all eyes

The other thing: when looking at the calls panel, the call has both values as "Not available", would be good to not to show them if there's no valid license, maybe @MartinSchoeler can help you with how to achieve that hugs

Hi @KevLehman The Frontend guys would like to keep this PR isolated from any frontend changes so it is easier to review. So what do you say about that? Should we merge this after this month's release by keeping it isolated to just backend changes?

@murtaza98 murtaza98 marked this pull request as draft May 27, 2022 10:38
@murtaza98

This comment was marked as outdated.

Sign in to view
@murtaza98 murtaza98 changed the title Chore: Move voip's Wrap-up modal functionality to EE (Backend) Chore: Move voip's Wrap-up and On-hold functionality to EE (Backend) May 30, 2022
@murtaza98 murtaza98 marked this pull request as ready for review May 30, 2022 07:57
@murtaza98
Copy link
Contributor Author

moving this to draft since some changes are also required for On-hold feature migration

Done with this change. This PR is again ready for review 😄

@murtaza98 murtaza98 requested a review from ggazzo May 30, 2022 10:01
@murtaza98 murtaza98 dismissed ggazzo’s stale review May 30, 2022 10:02

Changes done, so dismissing it

@KevLehman
Copy link
Contributor

Noticed you moved some files to livechat-enterprise, could you please create a voip-enterprise instead? 🙈 since the functionality you moved was not from livechat but from voip 👀

@murtaza98
Copy link
Contributor Author

Noticed you moved some files to livechat-enterprise, could you please create a voip-enterprise instead? see_no_evil since the functionality you moved was not from livechat but from voip eyes

That's a good idea. I've moved them into their own folder now 👍

PS: Seems like we're facing the same build issue on this PR too :(

@murtaza98 murtaza98 requested a review from KevLehman June 3, 2022 15:05
@murtaza98
Copy link
Contributor Author

Resynced PR with develop for QA team as agent's page wasn't working on this branch. Hence lost the approval here 🙈 @KevLehman Mind doing the honours again? 😃

KevLehman
KevLehman previously approved these changes Jun 9, 2022
View reviewed changes
@KevLehman
Copy link
Contributor

Let's wait for the other revs :)

@KevLehman KevLehman added the stat: ready to merge PR tested and approved waiting for merge label Jun 9, 2022
@kodiakhq kodiakhq bot removed the stat: ready to merge PR tested and approved waiting for merge label Jun 9, 2022
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Jun 9, 2022

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

@murtaza98 murtaza98 added the stat: ready to merge PR tested and approved waiting for merge label Jun 9, 2022
@kodiakhq kodiakhq bot merged commit a741aec into develop Jun 9, 2022
@kodiakhq kodiakhq bot deleted the voip/wrap-up branch June 9, 2022 18:05
gabriellsh added a commit that referenced this pull request Jun 10, 2022
@gabriellsh
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into chor…
d17602f 
…e/split-useUserInfoActions

* 'develop' of github.com:RocketChat/Rocket.Chat: (41 commits)
  Regression: Fix apps wrong typing (#25824)
  Chore: Remove compose from main repo (#23426)
  [FIX] `You and @Yourusername reacted with`title on reactions (#25733)
  [FIX] AgentsPage pagination (#25820)
  Chore: Move voip's Wrap-up and On-hold functionality to EE (Backend) (#25160)
  [FIX] Access issue on chat.getThreadsList (#25750)
  Chore: Remove snap files from Houston config (#25819)
  [FIX] Voip endpoint permissions (#25783)
  [FIX] allow only livechat-agents to be contact manager for any omnichannel contact  (#25451)
  Chore: use params instead of URL building on livechat endpoints (#25810)
  Regression: fix apps path (#25809)
  [BREAK] Remove RDStation integration (#25774)
  Chore: RestApiClient as Package (#25469)
  [FIX] Wrong argument name preventing Omnichannel Chat Forward to User  (#25723)
  [FIX] AccountBox checks for condition (#25708)
  Chore: Fix CI (#25797)
  [FIX] Fix prom-client new promise usage (#25781)
  [FIX] Discussion alphabetical  ordering (#25788)
  Update .kodiak.toml
  Update .kodiak.toml
  ...
gabriellsh added a commit that referenced this pull request Jun 13, 2022
@gabriellsh
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into new/…
be7ff5f 
…homepage

* 'develop' of github.com:RocketChat/Rocket.Chat: (21 commits)
  [FIX] Client-generated sort parameters in channel directory  (#25768)
  Chore: Add tests for agents screens (#25637)
  Chore: Notification Preferences to TS (#25827)
  Chore: Convert MemoizedSetting, Setting, Section (#25572)
  Regression: Fix users.create call (#25834)
  Chore: Add auto label and improve Kodiak configuration (#25829)
  Regression: Fix apps wrong typing (#25824)
  Chore: Remove compose from main repo (#23426)
  [FIX] `You and @Yourusername reacted with`title on reactions (#25733)
  [FIX] AgentsPage pagination (#25820)
  Chore: Move voip's Wrap-up and On-hold functionality to EE (Backend) (#25160)
  [FIX] Access issue on chat.getThreadsList (#25750)
  Chore: Remove snap files from Houston config (#25819)
  [FIX] Voip endpoint permissions (#25783)
  [FIX] allow only livechat-agents to be contact manager for any omnichannel contact  (#25451)
  Chore: use params instead of URL building on livechat endpoints (#25810)
  Regression: fix apps path (#25809)
  [BREAK] Remove RDStation integration (#25774)
  Chore: RestApiClient as Package (#25469)
  [FIX] Wrong argument name preventing Omnichannel Chat Forward to User  (#25723)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggazzo ggazzo ggazzo approved these changes

@KevLehman KevLehman KevLehman approved these changes

Assignees
No one assigned
Labels
stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@murtaza98 @KevLehman @ggazzo