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(dav): Migrate from hooks to user events #50689

Merged
merged 4 commits into from
Feb 13, 2025
Merged

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Feb 6, 2025

Part of #32128

Summary

Migrate from hooks to user events in dav application.
Also avoid issues if several users are deleted in the same request/process.

Checklist

@come-nc come-nc added the 2. developing Work in progress label Feb 6, 2025
@come-nc come-nc self-assigned this Feb 6, 2025
@come-nc come-nc force-pushed the fix/migrate-dav-to-events branch 2 times, most recently from e3a17b1 to a113b66 Compare February 6, 2025 11:38
@come-nc come-nc mentioned this pull request Feb 6, 2025
4 tasks
@come-nc come-nc marked this pull request as ready for review February 6, 2025 15:17
@come-nc come-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 6, 2025
@come-nc
Copy link
Contributor Author

come-nc commented Feb 6, 2025

This error in oracle tests seems random:

There was 1 failure:

1) OCA\DAV\Tests\unit\CardDAV\CardDavBackendTest::testUpdateProperties
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'UID'
+'FN'

/home/runner/actions-runner/_work/server/server/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php:537

I suppose depending on the run the order of UID and FN in the card array is different?
I’m not sure how it could be related to the changes in this PR, but I do not think I’ve seen the same failure on other PRs, has anyone encounter that before?

@come-nc come-nc force-pushed the fix/migrate-dav-to-events branch from 0a00af5 to a4d7623 Compare February 10, 2025 13:52
@tcitworld
Copy link
Member

I suppose depending on the run the order of UID and FN in the card array is different?
I’m not sure how it could be related to the changes in this PR, but I do not think I’ve seen the same failure on other PRs, has anyone encounter that before?

Not sure what's causing this either, but as an easy fix I'd simply try to order the select from cards_properties above by name to see if that helps.

@come-nc
Copy link
Contributor Author

come-nc commented Feb 10, 2025

I suppose depending on the run the order of UID and FN in the card array is different?
I’m not sure how it could be related to the changes in this PR, but I do not think I’ve seen the same failure on other PRs, has anyone encounter that before?

Not sure what's causing this either, but as an easy fix I'd simply try to order the select from cards_properties above by name to see if that helps.

Did that, it works, thanks.

@nickvergessen nickvergessen removed their request for review February 10, 2025 19:44
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Squash would be nice

@come-nc come-nc merged commit ee48caf into master Feb 13, 2025
189 checks passed
@come-nc come-nc deleted the fix/migrate-dav-to-events branch February 13, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants