search

mattermost / mattermost-plugin-mscalendar

Mattermost plugin for Microsoft Office365
Apache License 2.0
13 stars 21 forks source link

[MM-228] Add feature to update custom status and status of the user during meeting #359

Closed raghavaggarwal2308 closed 1 month ago

raghavaggarwal2308 commented 7 months ago

Recreating this PR with the changes of https://github.com/mattermost/mattermost-plugin-mscalendar/pull/355 because of a lot of merge conflicts on that PR

Summary

Ticket Link

228

Screenshots

image image

What to test?

How to test?

codecov-commenter commented 7 months ago

Codecov Report

Attention: Patch coverage is 46.55172% with 62 lines in your changes are missing coverage. Please review.

Project coverage is 21.92%. Comparing base (7f1bbcd) to head (642b212).

Files Patch % Lines
calendar/engine/availability.go 57.44% 30 Missing and 10 partials :warning:
calendar/engine/welcome_flow.go 0.00% 11 Missing :warning:
calendar/engine/settings.go 0.00% 9 Missing :warning:
calendar/engine/welcomer.go 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #359 +/- ## ========================================== + Coverage 20.94% 21.92% +0.98% ========================================== Files 67 67 Lines 3442 3525 +83 ========================================== + Hits 721 773 +52 - Misses 2628 2652 +24 - Partials 93 100 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hanzei commented 7 months ago

I'm not too knowldeage with this plugin. @mickmister @fmartingr would you mind reviewing the PR?

mickmister commented 4 months ago

@AayushChaudhary0001 Can you please take a look at this PR when you have the chance? Thanks

mickmister commented 4 months ago

From the discussion https://github.com/mattermost/mattermost-plugin-mscalendar/issues/228#issuecomment-2123280460, it seems we should still change the user's status when there are overlapping meetings. We want to emulate real life as much as possible. If there is an event where I'm a (confirmed?) attendee, and there are other attendees, then the user's status should change

mickmister commented 3 months ago

Hi @raghavaggarwal2308 just checking in. Do you or another dev plan to continue work on this PR?

raghavaggarwal2308 commented 3 months ago

Hi @raghavaggarwal2308 just checking in. Do you or another dev plan to continue work on this PR?

@mickmister Yeah, we will pick this up asap. Apologies for the delay.

mickmister commented 3 months ago

Hi @raghavaggarwal2308, just checking in on this

raghavaggarwal2308 commented 3 months ago

Hi @raghavaggarwal2308, just checking in on this

@mickmister @ayusht2810 Has replied to few of the comments above. Can you please look at those. After that we can proceed further with the fixes.

Comments: https://github.com/mattermost/mattermost-plugin-mscalendar/pull/359#discussion_r1650908946 https://github.com/mattermost/mattermost-plugin-mscalendar/pull/359#discussion_r1650895056

ayusht2810 commented 3 months ago

@mickmister Fixed the review comment. Please re-review

mickmister commented 2 months ago

Hi @AayushChaudhary0001, can you please review this when you have a chance?

ayusht2810 commented 2 months ago

@mickmister Fixed the review fixes. Please re-review.

arush-vashishtha commented 2 months ago

@raghavaggarwal2308 I found two issue while testing this PR

Issue 01: Wrong text is shown for the status change.

Steps to reproduce:

  1. Enable the MS calendar plugin
  2. Open the ms calendar settings for Mattermost server via slash command
  3. Then set 'Setting: Get Confirmation' to Yes
  4. Also set 'Setting: Set Custom Status' to Yes
  5. Now schedule a meeting from outlook calendar
  6. Check the text shown for status change message.

Expected: Appropriate and accurate text should be shown. Actual: Wrong text is shown for the status change.

SS for the reference: image

Issue 02: Custom status is not getting updated properly

Steps to reproduce:

  1. Enable the MS calendar plugin
  2. Create two overlapping meeting from outlook calendar like - one from 10:00 am to 10:15 am and second from 10:15 am to 10:30 am
  3. Check the custom status for second meeting

Expected: No delay should be there in updating the custom status of the second meeting Actual: Around 4 min delay is found for the updation of custom status for the second meeting

wiggin77 commented 1 month ago

@raghavaggarwal2308 I didn't realize this PR was needed for the custom status in Google Calendar plugin as well. We have a customer waiting on the GCal part. We'll need to test and merge this despite wanting to avoid big changes in MSCal, unless there is a better solution.

raghavaggarwal2308 commented 1 month ago

@wiggin77 Sure, we will work on this PR on priority. Also, the issue in mscalendar plugin has been fixed now. We have also got the confirmation from the customer who was facing the issue earliar.

raghavaggarwal2308 commented 1 month ago

@AayushChaudhary0001 @arush-vashishtha The issues reported above are very rare edge cases. These are happening because our job to update status is running in every 5 mins. So, when two meetings are very close there might be some inconsistencies rarely. I think we should create a new issue for this particular case and spend time on this there. As this is out of scope for this PR.

AayushChaudhary0001 commented 1 month ago

Approved this PR because the bug found are possibly out of the scope of this PR, these might be due to the time interval for updating the status by the job. Rest everything looks working fine in this PR(updating the custom status). Created a new issue regarding for the above #393