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

Revert "Revert "Create Talk rooms for appointments"" #5319

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Jun 14, 2023

Reverts #5306 and brings back #4025

How to test

  1. Install and enable Calendar and Talk
  2. Create an appointment config in calendar, tick the Create talk room checkbox
  3. Open the booking URL in a private tab
  4. Book and confirm an appointment
  5. Open Talk as the organizer
  6. See the Talk conversation created for the event

@ChristophWurst ChristophWurst added the 3. to review Waiting for reviews label Jun 14, 2023
@ChristophWurst ChristophWurst self-assigned this Jun 14, 2023
@ChristophWurst ChristophWurst marked this pull request as draft June 14, 2023 10:05
@ChristophWurst ChristophWurst force-pushed the revert-5306-enhancement/create-talk-room-appointment-revert branch 2 times, most recently from 908e383 to b8d2f67 Compare July 4, 2023 12:36
@ChristophWurst ChristophWurst marked this pull request as ready for review July 4, 2023 12:37
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.16% 🎉

Comparison is base (7f39c7b) 22.66% compared to head (9ca3afc) 22.82%.
Report is 15 commits behind head on main.

❗ Current head 9ca3afc differs from pull request most recent head 8838fba. Consider uploading reports for the commit 8838fba to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5319      +/-   ##
============================================
+ Coverage     22.66%   22.82%   +0.16%     
- Complexity      372      387      +15     
============================================
  Files           237      239       +2     
  Lines         11725    11745      +20     
  Branches       2282     2274       -8     
============================================
+ Hits           2657     2681      +24     
+ Misses         9068     9064       -4     
Flag Coverage Δ
javascript 13.93% <0.00%> (+0.03%) ⬆️
php 64.94% <ø> (-0.54%) ⬇️

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

Files Changed Coverage Δ
src/components/AppointmentConfigModal.vue 0.00% <0.00%> (ø)
...s/AppointmentConfigModal/CheckedDurationSelect.vue 0.00% <ø> (ø)
...rc/components/AppointmentConfigModal/TextInput.vue 0.00% <0.00%> (ø)
src/models/appointmentConfig.js 0.00% <0.00%> (ø)
src/store/settings.js 87.71% <0.00%> (-0.88%) ⬇️

... and 17 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ChristophWurst ChristophWurst added the enhancement New feature request label Aug 4, 2023
@ChristophWurst ChristophWurst force-pushed the revert-5306-enhancement/create-talk-room-appointment-revert branch from 9ca3afc to 9f8fb93 Compare August 4, 2023 13:22
Copy link
Contributor

@hamza221 hamza221 left a comment

Choose a reason for hiding this comment

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

image
2 talk rooms are created but it's a problem with the original Pr I'm guessing. I can try to solve it when this is merged.
Also The booking page needs a loading animation, I'll create an issue
But aside from this works perfectly 👍

@ChristophWurst
Copy link
Member Author

Did you book two appointments? The room will not be shared. If person a and person b book, they get two separate rooms. If person a books two slots, there will still be two rooms.

@hamza221
Copy link
Contributor

No just one appointment with one user

@hamza221
Copy link
Contributor

image
Looks deterministic

@ChristophWurst
Copy link
Member Author

I'll test another time. This doesn't seem right

@hamza221
Copy link
Contributor

The user receives only one confirmation email, with the first room's link .

@st3iny
Copy link
Member

st3iny commented Aug 14, 2023

The AppointmentBookedEvent is emitted twice in \OCA\Calendar\Service\Appointments\BookingService::confirmBooking which leads to two rooms being created.

ChristophWurst and others added 2 commits August 14, 2023 16:30
…nfigMapper::findByToken"

This reverts commit 94c93f1.

Signed-off-by: Christoph Wurst <[email protected]>
@ChristophWurst ChristophWurst force-pushed the revert-5306-enhancement/create-talk-room-appointment-revert branch from 4fd771b to 8838fba Compare August 14, 2023 14:31
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 14, 2023
@ChristophWurst ChristophWurst merged commit a3cb83b into main Aug 14, 2023
39 of 41 checks passed
@ChristophWurst ChristophWurst deleted the revert-5306-enhancement/create-talk-room-appointment-revert branch August 14, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature request
Projects
Development

Successfully merging this pull request may close these issues.

4 participants