Skip to content

refactor: Use @nextcloud/files and @nextcloud/sharing instead of custom logic #2392

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
Merged
skjnldsv merged 1 commit into from
Aug 9, 2024
Merged

refactor: Use @nextcloud/files and @nextcloud/sharing instead of custom logic #2392

skjnldsv merged 1 commit into from
Aug 9, 2024

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Aug 6, 2024 •
edited
Loading

Cleanup logic that we can share between server and apps through the files library.

@susnux susnux added the 2. developing Work in progress label Aug 6, 2024
@susnux susnux marked this pull request as ready for review August 7, 2024 16:52
@susnux susnux added 3. to review Waiting for reviews ♻️ refactor Pull request that is neither a fix nor a feature and removed 2. developing Work in progress labels Aug 7, 2024
@skjnldsv
Copy link
Member

skjnldsv commented Aug 9, 2024

/compile amend /

@susnux @nextcloud-command
refactor: Use @nextcloud/files and @nextcloud/sharing instead of …
1dea2c8 
…custom logic

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the refactor/use-files-functions branch from 163cba2 to 1dea2c8 Compare August 9, 2024 08:16
@skjnldsv skjnldsv merged commit 7417482 into master Aug 9, 2024
@skjnldsv skjnldsv deleted the refactor/use-files-functions branch August 9, 2024 11:55
@juliusknorr
Copy link
Member

juliusknorr commented Aug 12, 2024 •
edited
Loading

This seems to have broken a few things for me (and the text cypress tests):

  • Open regular images as logged in user
Screenshot 2024-08-12 at 12 41 46
  • Opening files if a folder was shared as a public share link
    • The relative path prop on the viewer component seems to contain /files/{shareToken} now instead of just being relative to the share
    • Note in text we use the relativePath and assume it is relative to the share root, but seems this also affects other file types
  • Sidebar action is missing

@juliusknorr
Copy link
Member

juliusknorr commented Aug 12, 2024

@susnux @skjnldsv In case this is not a quick fix, could we revert and bring back after the branch-off?

@juliusknorr
Copy link
Member

juliusknorr commented Aug 12, 2024

Cypress tests passing here also seem to be odd as every runner executed the following spec without any tests:

https://github.com/nextcloud/viewer/actions/runs/10315988707/job/28557340785?pr=2392
Screenshot 2024-08-12 at 12 59 43

@susnux susnux mentioned this pull request Aug 12, 2024
Merged
@juliusknorr juliusknorr mentioned this pull request Aug 13, 2024
Merged
3 tasks
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
Merged
@joshtrichards joshtrichards mentioned this pull request Oct 1, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@artonge artonge Awaiting requested review from artonge

@SystemKeeper SystemKeeper Awaiting requested review from SystemKeeper

Assignees

No one assigned

Labels

3. to review Waiting for reviews ♻️ refactor Pull request that is neither a fix nor a feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@susnux @skjnldsv @juliusknorr