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.

Sign up for GitHub

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

Jump to bottom

Port files trashbin events to IEventDispatcher/IEventListener #32018

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Port files trashbin events to IEventDispatcher/IEventListener #32018

merged 4 commits into from
Sep 24, 2024
+147 −73

Conversation

CarlSchwan
Copy link
Member

oc_hooks is deprecated and will at some point be removed

@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Apr 19, 2022
@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone Apr 19, 2022
@CarlSchwan CarlSchwan self-assigned this Apr 19, 2022
@CarlSchwan CarlSchwan force-pushed the cleanup/event/trashbin branch 3 times, most recently from 9af5d5f to 7079039 Compare April 19, 2022 18:49
artonge
artonge reviewed Apr 20, 2022
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Still a bunch of lint errors/warnings :)

@CarlSchwan CarlSchwan force-pushed the cleanup/event/trashbin branch 3 times, most recently from cba371e to ccad4d3 Compare April 22, 2022 14:25
@CarlSchwan CarlSchwan mentioned this pull request Apr 25, 2022
Open
6 tasks
This was referenced Aug 12, 2022
Merged
Merged
This was referenced Aug 24, 2022
Merged
Merged
This was referenced Sep 6, 2022
Merged
Merged
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
This was referenced Sep 20, 2022
Closed
Merged
@blizzz blizzz removed this from the Nextcloud 25 milestone Sep 22, 2022
@blizzz blizzz mentioned this pull request Nov 20, 2023
Merged
5 tasks
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv
Copy link
Member

As there is no feedback since a while I will close this ticket.
If you will decide to work on this feature again and if it hasn't been fixed or implemented already, feel free to re-open and solve the various conflicts.

@skjnldsv skjnldsv closed this Feb 27, 2024
@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Feb 27, 2024
@skjnldsv skjnldsv deleted the cleanup/event/trashbin branch February 27, 2024 17:34
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
@come-nc come-nc restored the cleanup/event/trashbin branch September 23, 2024 13:58
@come-nc come-nc reopened this Sep 23, 2024
@come-nc come-nc assigned come-nc and unassigned CarlSchwan Sep 23, 2024
@come-nc come-nc added the 2. developing Work in progress label Sep 23, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 23, 2024
View reviewed changes
apps/files_trashbin/lib/Listener/EventListener.php
public function handle(Event $event): void {
if ($event instanceof NodeWrittenEvent) {
// Resize trash
if (!empty($this->userId)) {

Check notice

Code scanning / Psalm

RiskyTruthyFalsyComparison Note

Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.
apps/files_trashbin/lib/Trashbin.php
@@ -1170,4 +1172,10 @@
return new NonExistingFile($rootFolder, $view, $fullPath);
}
}

public function handle(Event $event): void {
if ($event instanceof BeforeNodeDeletedEvent) {

Check notice

Code scanning / Psalm

RedundantConditionGivenDocblockType Note

Docblock-defined type OCP\Files\Events\Node\BeforeNodeDeletedEvent for $event is always OCP\Files\Events\Node\BeforeNodeDeletedEvent
CarlSchwan and others added 3 commits September 23, 2024 18:13
@CarlSchwan @come-nc
feat(trashbin): Port files trashbin events to IEventDispatcher/IEvent…
77821f8 
…Listener

oc_hooks is deprecated and will at some point be removed

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
chore: Use SPDX headers and property promotion in new files
b80bc81 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
fix(tests): Fix tests now that trashbin listens to events properly
af0b8fb 
Hooks are cleared in test bootstrap so switching to events activates
 them in tests.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 23, 2024
@come-nc
fix(tests): Pause trashbin for lock tests in ViewTest using rmdir and…
4adf5fd 
… unlink

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Copy link
Contributor

come-nc commented Sep 24, 2024

The 4 ViewTest failures came from the fact that the trashbin wrapper is now registered and intercepts the rmdir and unlink calls to applies trashbin logic instead of actually deleting stuff.
I added a call to pause trash in those tests and it passes now.

@susnux susnux added this to the Nextcloud 31 milestone Sep 24, 2024
@come-nc come-nc merged commit 280f6df into master Sep 24, 2024
173 checks passed
@come-nc come-nc deleted the cleanup/event/trashbin branch September 24, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcitworld tcitworld tcitworld left review comments

@artonge artonge artonge left review comments

@come-nc come-nc come-nc approved these changes

@susnux susnux susnux approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@provokateurin provokateurin Awaiting requested review from provokateurin

Assignees

@come-nc come-nc

Labels
3. to review Waiting for reviews technical debt
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.
7 participants
@CarlSchwan @come-nc @tcitworld @skjnldsv @susnux @artonge @blizzz