Skip to content

fix(files_sharing): Fix getting shares by path#16988

Open
artonge wants to merge 5 commits intomainfrom
artonge/fix/share_path_matching
Open

fix(files_sharing): Fix getting shares by path#16988
artonge wants to merge 5 commits intomainfrom
artonge/fix/share_path_matching

Conversation

@artonge
Copy link
Contributor

@artonge artonge commented Feb 5, 2026

  • Match both parent and child share
  • Improve path matching with /_%.
  • Filter share by path in resolveSharesForRecipient

@artonge artonge self-assigned this Feb 5, 2026
@artonge artonge added bug feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings php labels Feb 5, 2026
@artonge
Copy link
Contributor Author

artonge commented Feb 5, 2026

/backport to stable33

@artonge artonge force-pushed the artonge/fix/share_path_matching branch from 809cca1 to e6ae3fa Compare February 5, 2026 14:16
@artonge artonge force-pushed the artonge/fix/share_path_matching branch from e6ae3fa to 641ce97 Compare February 5, 2026 16:06
- Match both parent and child share
- Improve path matching with `/_%`.
- Filter share by path in `resolveSharesForRecipient`

Signed-off-by: Louis Chmn <louis@chmn.me>
@artonge artonge force-pushed the artonge/fix/share_path_matching branch from 641ce97 to 6115ef8 Compare February 6, 2026 11:11
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Anything specific we can/should add test-wise? The PR description and commit message (fix) seem to indicate a bug, but I assume its only the upcoming performance improvement?

@artonge
Copy link
Contributor Author

artonge commented Feb 6, 2026

Anything specific we can/should add test-wise? The PR description and commit message (fix) seem to indicate a bug, but I assume its only the upcoming performance improvement?

My only question would be whether ROOMUSER share are expected to always exists. Because if one of them do not exist, then a share will be returned with a target set as {TALK_PLACEHOLDER}.

This might be linked to the failing tests on the PR.

@nickvergessen
Copy link
Member

Because if one of them do not exist, then a share will be returned with a target set as {TALK_PLACEHOLDER}.

I assume the events BeforeShareCreatedEvent and VerifyMountPointEvent I mentioned in the chat took care of that in the past:

BeforeShareCreatedEvent::class => $this->overwriteShareTarget($event),
VerifyMountPointEvent::class => $this->overwriteMountPoint($event),

So if one of them is no longer triggered when loading a specific share for a user, that's possible.

@nickvergessen
Copy link
Member

The sharing-1 and features/sharing-4/update.feature Seems to be fixed by work from Robins PR in #16987 but it's breaking a new one.

So yeah, from talk's pov we always expect the Talk/ folder being used and that should result in the USERROM share, so we don't have to replace all the time and it's not moved when you change your directory later (e.g. in case you sort by Talk/$Year/ )

@artonge
Copy link
Contributor Author

artonge commented Feb 6, 2026

Backporting to see the result of CI on 33

@artonge
Copy link
Contributor Author

artonge commented Feb 6, 2026

/backport! to stable33

Signed-off-by: Joas Schilling <coding@schilljs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings php

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants