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

[full-ci] Sharing jail #6593

Merged
merged 45 commits into from
May 6, 2022
Merged

[full-ci] Sharing jail #6593

merged 45 commits into from
May 6, 2022

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Mar 14, 2022 •
edited
Loading

Switches over to the new webdav endpoints for a) personal files, i.e. personal space and b) shares, i.e. share jail.

To be tested with oCIS from owncloud/ocis#3626 (or later on with master when it's merged) because of a new capability that enables the share jail.

needs clarification

open tasks

  • finish resource loader
  • hack webDavPath
  • breadcrumbs for new SharedResource view/route
  • fix file/folder creation inside share
  • make files app file actions available in SharedResource route/view
  • include existing url query params in breadcrumbs (otherwise navigation to parent in a share fails with a Resource not found because there is no resourceId available anymore)
  • contextRouteQuery for app defaults (SharedResource view needs share id in the url query because share name in the path is non-unique and doesn't work for propfind)
  • fix path and webdav path after accept share action
  • left sidebar active route highlighting
  • check back with CERN if their shares still work as expected not needed because we hide the share jail behind a new capability. if they set that to false they have the old behaviour.
  • switch to new personal space webdav endpoint for location picker
  • repair that quick actions and sharing context menu items are not available anymore (broke it in this PR)
  • Shared with me doesn't show an accepted share in the right sidebar anymore, because it has a path of / and that is treated as a root element (which are not shown in the right sidebar).
  • fix unit tests
  • fix acceptance tests 🤯
  • fix e2e tests
  • adapt packages/web-app-files/src/mixins/actions/createQuicklink.js regarding share capabilities after [full-ci] Introduce QuickLinks #6820 is merged
  • adapt new hasSpaces occurrences after [full-ci] Search all files with ocis backend #6841 is merged
  • fix uploads in shares (needs different endpoint, still points to /dav/files/<user>)

followup after merge (create issues once this PR is merged or close to it)

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Mar 14, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kulmann kulmann force-pushed the sharing-jail branch 4 times, most recently from f80f4af to 6da3154 Compare March 17, 2022 09:16
@kulmann kulmann mentioned this pull request Mar 24, 2022
Closed
@kulmann kulmann force-pushed the sharing-jail branch 2 times, most recently from 7373302 to fc1409b Compare March 29, 2022 09:10
@kulmann kulmann force-pushed the sharing-jail branch 2 times, most recently from f19786f to 3e99942 Compare April 7, 2022 18:42
@kulmann kulmann force-pushed the sharing-jail branch 3 times, most recently from 862b9cf to f93ff5e Compare April 26, 2022 20:45
@micbar micbar mentioned this pull request Apr 27, 2022
Merged
@kulmann kulmann mentioned this pull request Apr 27, 2022
Closed
@micbar micbar mentioned this pull request Apr 27, 2022
Merged
9 tasks
@kulmann kulmann mentioned this pull request Apr 28, 2022
Closed
@JammingBen
Copy link
Collaborator

At a first glance, renaming files as a share recipient does not work for me most of the time. Strangely it works when declining a share, then accepting that share again and renaming it. But when I freshly browse to the page with accepted shares, renaming fails for me.

@kulmann kulmann mentioned this pull request Apr 29, 2022
Merged
9 tasks
@kulmann kulmann self-assigned this Apr 29, 2022
@kulmann kulmann mentioned this pull request Apr 29, 2022
Closed
25 tasks
@kulmann
Copy link
Member Author

kulmann commented Apr 29, 2022

At a first glance, renaming files as a share recipient does not work for me most of the time. Strangely it works when declining a share, then accepting that share again and renaming it. But when I freshly browse to the page with accepted shares, renaming fails for me.

Renaming a share or renaming a file within a share?

@JammingBen
Copy link
Collaborator

Renaming a share or renaming a file within a share?

Renaming the share itself. E.g. sharing a file and then trying to rename that file as share recipient.

@kulmann kulmann changed the title Sharing jail [full-ci] Sharing jail Apr 29, 2022
@ownclouders
Copy link
Contributor

ownclouders commented Apr 29, 2022 •
edited
Loading

Results for oCISSharingInternal3 https://drone.owncloud.com/owncloud/web/25389/62/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUIResharing1-reshareUsers_feature-L198.png

webUIResharing1-reshareUsers_feature-L198.png

This was referenced May 6, 2022
Closed
Open
@kulmann
Copy link
Member Author

kulmann commented May 6, 2022

Commented about the upcoming changes of this PR in owncloud/owncloud-test-middleware#116

This was referenced May 6, 2022
Closed
Closed
Closed
@kulmann kulmann marked this pull request as ready for review May 6, 2022 10:47
@kulmann kulmann requested a review from fschade as a code owner May 6, 2022 10:47
fschade and others added 3 commits May 6, 2022 13:58
@fschade
bring back ci e2e tests
87e2199 
fix failing e2e tests
fix clickResource e2e test helper
@fschade
fix starlark
9fbab06
@pascalwengerter
Skip last resharing test in oCIS CI
ba47836
@sonarcloud
Copy link

sonarcloud bot commented May 6, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 21 Code Smells

30.3% 30.3% Coverage
15.0% 15.0% Duplication

pascalwengerter
pascalwengerter reviewed May 6, 2022
View reviewed changes
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Just figured I have a pending review, let's tackle this in a follow-up 🤦🏽

packages/web-app-files/src/views/shares/SharedResource.vue
icon="folder"
>
<template #message>
<span v-translate>There are no resources in this folder</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could/should potentially be a <p> tag

packages/web-app-files/src/views/shares/SharedResource.vue
<span v-translate>There are no resources in this folder</span>
</template>
<template #callToAction>
<span v-translate>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could/should potentially be a <p> tag

@pascalwengerter pascalwengerter merged commit b747d29 into master May 6, 2022
@kulmann kulmann mentioned this pull request May 6, 2022
Closed
7 tasks
@pascalwengerter pascalwengerter mentioned this pull request May 6, 2022
Closed
@kulmann kulmann mentioned this pull request May 6, 2022
Closed
@JammingBen JammingBen mentioned this pull request May 17, 2022
Merged
5 tasks
@pascalwengerter pascalwengerter deleted the sharing-jail branch June 29, 2022 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dschmidt dschmidt dschmidt left review comments

@pascalwengerter pascalwengerter pascalwengerter left review comments

@JammingBen JammingBen JammingBen approved these changes

@fschade fschade Awaiting requested review from fschade

Assignees

@kulmann kulmann

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Shared with me-view Trying to create a (public) link share of the Shares folder throws an error
6 participants
@kulmann @JammingBen @ownclouders @pascalwengerter @dschmidt @fschade