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: override iTip Broker to fix several issues #48583

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Oct 6, 2024

Summary

This over rides the iTip broker and the main iTip message generation routine to fix several issues.

Checklist

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good

Do you have instructions for us for some manual testing? Would it make sense to cover some of this with unit tests? parseEventForOrganizer appears to be free of side effects - event info in, messages out - something that's naturally nice to write tests for.

apps/dav/lib/CalDAV/TipBroker.php Outdated Show resolved Hide resolved
@SebastianKrupinski
Copy link
Contributor Author

Code looks good

Do you have instructions for us for some manual testing? Would it make sense to cover some of this with unit tests? parseEventForOrganizer appears to be free of side effects - event info in, messages out - something that's naturally nice to write tests for.

Morning, Sure I can write some test, for it, I originally skipped the tests because iTip broker message generation is already covered by most of our unit tests, but I'll write some test specifically for this, this week.

@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Oct 14, 2024

Do you have instructions for us for some manual testing?

For this, basically just run through the common actions a user would do while inviting an attendee,

  1. create an event with attendees
  2. update an event (location, description)
  3. cancel an event using status in the side bar

And basically see if the attendees received and email.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Merci for the test coverage 🙏

Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Oct 21, 2024

@ChristophWurst backport to 30 or further? We would need to backport the sabre patch to go further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
Status: ☑️ Done
Development

Successfully merging this pull request may close these issues.

3 participants