Skip to content

Conversation

@SebastianKrupinski
Copy link
Contributor

Summary

  • Initial sync no longer returns deleted items
  • fixed double processing of results when doing a follow up sync

@SebastianKrupinski SebastianKrupinski self-assigned this Jul 19, 2024
@SebastianKrupinski SebastianKrupinski added the 3. to review Waiting for reviews label Jul 19, 2024
@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-44127 branch 2 times, most recently from d86d377 to bf6bb68 Compare July 19, 2024 03:08
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.

  • Always use prepared statements with parameters
  • Add a test to \OCA\DAV\Tests\unit\CalDAV\CalDavBackendTest

@susnux susnux added the bug label Jul 25, 2024
@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-44127 branch 3 times, most recently from 250f3a6 to e450139 Compare July 27, 2024 23:05
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

This is also a (minor) performance optimization as we do $qb->func()->max('operation') instead of a busy loop now 👍

I'm gonna give this a test but looks good so far.

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

LGTM! Refactoring of the incremental sync case is a (minor) performance improvement. Additional unit tests are always welcome.

@SebastianKrupinski SebastianKrupinski merged commit 4c4e414 into master Aug 7, 2024
@SebastianKrupinski SebastianKrupinski deleted the fix/issue-44127 branch August 7, 2024 12:49
@Altahrim Altahrim mentioned this pull request Aug 8, 2024
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 bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Deleted calendar objects included in initial sync

6 participants