Skip to content

Conversation

@max-nextcloud
Copy link
Collaborator

Allow opening attachments in public collective shares
as well as in folder descriptions of public shares.

@max-nextcloud max-nextcloud requested a review from mejo- as a code owner July 24, 2025 12:58
@max-nextcloud
Copy link
Collaborator Author

I tested this in collectives and in shared folders with a folder description. However I don't know if I am missing some corner cases. All in all the approach seems sound though.

Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this ❤️

Does this also work with subpage shares in Collectives? I can imagine it does, as internalPath might be relative to the share root? 🤔

@max-nextcloud
Copy link
Collaborator Author

Does this also work with subpage shares in Collectives? I can imagine it does, as internalPath might be relative to the share root? 🤔

Sadly not.... it's relative to the collective root. But maybe we can also resolve it relatively to the share root.

@max-nextcloud max-nextcloud force-pushed the fix/public-attachments branch 4 times, most recently from f053c71 to 697564c Compare July 26, 2025 12:35
@max-nextcloud max-nextcloud requested a review from mejo- July 26, 2025 13:15
@max-nextcloud
Copy link
Collaborator Author

@mejo- I think I got it now. Could you double check the getShareFolder function?
It's a bit unfortunate to repeat all this code from the getTextFilePublic function. I tried to refactor it with a function that returns both the file and the folder as a list{File, Folder | null} - but I never managed to make psalm happy and it also introduces a fair bit of complexity.

@max-nextcloud max-nextcloud changed the title fix(share): use internal path as davPath for attachments fix(share): use relative path in share as davPath for attachments Jul 26, 2025
Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Nice, great that you found a generic solution that works for all kinds of public file shares ❤️

I didn't test myself, but the code looks sensible to me.

@max-nextcloud max-nextcloud force-pushed the fix/public-attachments branch 2 times, most recently from 9b6ed49 to 6e1fa86 Compare July 26, 2025 19:19
@max-nextcloud
Copy link
Collaborator Author

/backport to stable31

@max-nextcloud
Copy link
Collaborator Author

/backport to stable30

@max-nextcloud max-nextcloud force-pushed the fix/public-attachments branch from 6e1fa86 to 2b1271d Compare July 26, 2025 19:37
@max-nextcloud max-nextcloud enabled auto-merge July 26, 2025 19:37
Provide relative path to attachments in public shares.

The path needs to be relative to the shared folder.

Open attachments in public collective shares
as well as in folder descriptions of public shares.

Signed-off-by: Max <[email protected]>
Only in folder description though as we do not allow it from the viewer.

Signed-off-by: Max <[email protected]>
@max-nextcloud max-nextcloud force-pushed the fix/public-attachments branch from 2b1271d to 8b04564 Compare July 28, 2025 06:17
@codecov
Copy link

codecov bot commented Jul 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.65%. Comparing base (aff1125) to head (8b04564).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7477   +/-   ##
=======================================
  Coverage   59.65%   59.65%           
=======================================
  Files         495      495           
  Lines       37866    37866           
  Branches     1090     1090           
=======================================
  Hits        22589    22589           
  Misses      15170    15170           
  Partials      107      107           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@max-nextcloud max-nextcloud merged commit 1261285 into main Jul 28, 2025
70 checks passed
@max-nextcloud max-nextcloud deleted the fix/public-attachments branch July 28, 2025 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants