Skip to content

Port files trashbin events to IEventDispatcher/IEventListener - #32018

Merged
come-nc merged 4 commits into
masterfrom
cleanup/event/trashbin
Sep 24, 2024
Merged

Port files trashbin events to IEventDispatcher/IEventListener#32018
come-nc merged 4 commits into
masterfrom
cleanup/event/trashbin

Conversation

@CarlSchwan

@CarlSchwanCarlSchwan commented Apr 19, 2022

Copy link
Copy Markdown
Member

oc_hooks is deprecated and will at some point be removed

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

@artongeartonge left a comment

Copy link
Copy Markdown
Collaborator

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
CarlSchwanforce-pushed the cleanup/event/trashbin branch 3 times, most recently from cba371e to ccad4d3CompareApril 22, 2022 14:25
come-nc
come-nc reviewed Apr 25, 2022
View reviewed changes
Comment threadapps/files_trashbin/lib/AppInfo/Application.php Outdated
come-nc
come-nc reviewed Apr 25, 2022
View reviewed changes
Comment threadapps/files_trashbin/lib/Storage.php Outdated
come-nc
come-nc reviewed Apr 25, 2022
View reviewed changes
Comment threadapps/files_trashbin/lib/Trashbin.php Outdated
@CarlSchwanCarlSchwan mentioned this pull request Apr 25, 2022
Open
7 tasks
@come-nccome-nc added the technical debt 🧱 🤔🚀 label Apr 26, 2022
@CarlSchwan
CarlSchwanforce-pushed the cleanup/event/trashbin branch from ccad4d3 to 0766816CompareMay 5, 2022 14:27
This was referenced Aug 12, 2022
Merged
Merged
This was referenced Aug 24, 2022
Merged
Merged
This was referenced Sep 6, 2022
Merged
Merged
@skjnldsvskjnldsv mentioned this pull request Sep 15, 2022
Merged
This was referenced Sep 20, 2022
Closed
Merged
@blizzzblizzz removed this from the Nextcloud 25 milestone Sep 22, 2022
@blizzzblizzz modified the milestones: Nextcloud 28, Nextcloud 29Nov 23, 2023
@skjnldsv

skjnldsv commented Feb 27, 2024

Copy link
Copy Markdown
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.

@skjnldsvskjnldsv 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
@skjnldsvskjnldsv 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-nccome-nc reopened this Sep 23, 2024
@come-nccome-nc assigned come-nc and unassigned CarlSchwanSep 23, 2024
@come-nc
come-ncforce-pushed the cleanup/event/trashbin branch from 2ceeac1 to fa2936cCompareSeptember 23, 2024 14:33
@come-nccome-nc added the 2. developing Work in progress label Sep 23, 2024
@come-nc
come-ncforce-pushed the cleanup/event/trashbin branch from ad3e703 to 8a6761aCompareSeptember 23, 2024 14:46
github-advanced-security[bot]
github-advanced-securityAI found potential problems Sep 23, 2024
View reviewed changes
Comment threadapps/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

Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.
Comment threadapps/files_trashbin/lib/Trashbin.php
}

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

Check notice

Code scanning / Psalm

RedundantConditionGivenDocblockType

Docblock-defined type OCP\Files\Events\Node\BeforeNodeDeletedEvent for $event is always OCP\Files\Events\Node\BeforeNodeDeletedEvent
CarlSchwanand 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-ncforce-pushed the cleanup/event/trashbin branch from 98986b7 to af0b8fbCompareSeptember 23, 2024 16:13
@come-nccome-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

come-nc commented Sep 24, 2024

Copy link
Copy Markdown
Contributor

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.

@susnuxsusnux added this to the Nextcloud 31 milestone Sep 24, 2024
@come-nc
come-nc merged commit 280f6df into masterSep 24, 2024
@come-nc
come-nc deleted the cleanup/event/trashbin branch September 24, 2024 15:15
@skjnldsvskjnldsv mentioned this pull request Jan 7, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tcitworldtcitworldtcitworld left review comments
@artongeartongeartonge left review comments
@come-nccome-nccome-nc approved these changes
@susnuxsusnuxsusnux approved these changes
@icewind1991icewind1991Awaiting requested review from icewind1991
@provokateurinprovokateurinAwaiting requested review from provokateurin

Assignees

@come-nccome-nc

Labels

3. to reviewWaiting for reviewstechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 31

Development

Successfully merging this pull request may close these issues.

8 participants

@CarlSchwan@come-nc@tcitworld@skjnldsv@susnux@artonge@github-advanced-security@blizzz