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

Reordering of the attendees should not be a signitifcant change (#540) #541

Merged
merged 1 commit into from
Nov 14, 2021

Conversation

floerke
Copy link
Contributor

@floerke floerke commented Sep 10, 2021

#540

If a server (using sabre) receives a VEVENT update it will send mails to all attendees. Not for all modified events, but for events with a "significant change". The calculation of "signifcant change" is done by vobject.

I have trouble with to many "Invitation: ..."-Mails for events that has not been changed.

Investigating this I have seen:

  • the events were modified by the client and sent to the server (eg. to save the acknowledgment of an alarm).
  • the client has changed the order (not the attendees itself) of the attendees in the modified event
  • vobject calculated a "significant change"
  • the server sent the email

I think everything is correct, except, the reordering is not a significant change. Why the client desires to reorder the attendees? I don't know, but I think this should not be a significant change.

@staabm
Copy link
Member

staabm commented Sep 10, 2021

Sounds reasonable. Could you add a unit test?

@floerke
Copy link
Contributor Author

floerke commented Sep 10, 2021

Hi Markus, I added a new unittest...

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Rebased and squashed to get fresh CI. LGTM.

@codecov
Copy link

codecov bot commented Nov 14, 2021

Codecov Report

Merging #541 (5f81c0f) into master (6ef9328) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #541   +/-   ##
=========================================
  Coverage     98.69%   98.69%           
- Complexity     1816     1817    +1     
=========================================
  Files            66       66           
  Lines          4445     4448    +3     
=========================================
+ Hits           4387     4390    +3     
  Misses           58       58           
Impacted Files Coverage Δ
lib/ITip/Broker.php 99.51% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ef9328...5f81c0f. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants