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

refactor: use sharing NG for share listings #10392

Merged
merged 2 commits into from
Jan 31, 2024
Merged

refactor: use sharing NG for share listings #10392

merged 2 commits into from
Jan 31, 2024

Conversation

JammingBen
Copy link
Contributor

@JammingBen JammingBen commented Jan 23, 2024 •
edited
Loading

Description

Use the new sharing NG API for the share listings (Shared with/by me, Shared via link).

I also removed the shareType property on the ShareResource because it can have multiple different share types (e.g. when it's shared with another user as well as via link). The already existing shareTypes property now holds all the types. In addition to that, each item in the sharedWith array now holds information about the share type.

Related Issue

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
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@JammingBen JammingBen self-assigned this Jan 23, 2024
@update-docs Update Docs
Copy link

update-docs bot commented Jan 23, 2024

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.

@JammingBen JammingBen force-pushed the sharing-ng-shared-with-me branch from 6962f2f to 65ce1b0 Compare January 23, 2024 07:56
@JammingBen JammingBen changed the title refactor: use sharing NG for the shared views refactor: use sharing NG for "Shared with me" listing Jan 23, 2024
@JammingBen JammingBen force-pushed the sharing-ng-shared-with-me branch 13 times, most recently from 916477f to 9a3ca73 Compare January 24, 2024 14:20
@fschade fschade changed the title refactor: use sharing NG for "Shared with me" listing refactor: use sharing NG for share listings Jan 26, 2024
@fschade fschade mentioned this pull request Jan 26, 2024
Merged
4 tasks
@JammingBen JammingBen force-pushed the sharing-ng-shared-with-me branch 3 times, most recently from db5e4a3 to 3920f86 Compare January 29, 2024 09:33
@fschade fschade mentioned this pull request Jan 29, 2024
Merged
@JammingBen JammingBen force-pushed the sharing-ng-shared-with-me branch 4 times, most recently from a7dbb6a to 84436e1 Compare January 29, 2024 12:56
@JammingBen JammingBen mentioned this pull request Jan 29, 2024
Closed
40 tasks
@JammingBen JammingBen force-pushed the sharing-ng-shared-with-me branch 2 times, most recently from 9cb53b2 to acc960e Compare January 30, 2024 06:32
@JammingBen JammingBen force-pushed the sharing-ng-shared-with-me branch from acc960e to d65c03b Compare January 30, 2024 06:33
JammingBen
JammingBen commented Jan 30, 2024
View reviewed changes
tests/acceptance/features/webUISharingAcceptShares/acceptShares.feature
@@ -11,21 +11,6 @@ Feature: accept/decline shares coming from internal users
| Brian |
And user "Brian" has logged in using the webUI

@issue-ocis-1950
Scenario: reject a share that you received as user and as group member
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test doesn't work in that form anymore because oCIS now sends 2 shares when one resource is being shared with a user directly as well as via group.

Could be refactored, but since it's an acceptance test and we won't have the concept of "declining" shares in the future I guess we can just let the test die.

@JammingBen JammingBen marked this pull request as ready for review January 30, 2024 08:40
@JammingBen JammingBen mentioned this pull request Jan 30, 2024
Merged
7 tasks
kulmann
kulmann requested changes Jan 30, 2024
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Share names in Shared with others and Shared via link are broken, see screenshot. Folder names from the personal space become a single . instead in the outgoing share lists.

Screenshot 2024-01-30 at 14 34 57 Screenshot 2024-01-30 at 14 35 02

packages/web-app-files/src/views/shares/SharedWithMe.vue Outdated Show resolved Hide resolved
@JammingBen
Copy link
Contributor Author

Share names in Shared with others and Shared via link are broken, see screenshot. Folder names from the personal space become a single . instead in the outgoing share lists.

Yep that was due to some of the server changes (owncloud/ocis#8307) I was mentioning earlier 😄 The server now correctly sends the parent path without the resource name, hence we need to join both on our own.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

26 New issues
0 Security Hotspots
32.2% Coverage on New Code
3.0% Duplication on New Code

See analysis details on SonarCloud

@JammingBen JammingBen requested a review from kulmann January 30, 2024 14:22
kulmann
kulmann approved these changes Jan 30, 2024
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Don't know if that's an issue of this PR, but whenever I use row actions like hide/unhide/sync/unsync in the shared with me list, the avatar in the shared with column disappears. Everything else seems to work fine 💪 😍

@JammingBen
Copy link
Contributor Author

JammingBen commented Jan 30, 2024 •
edited
Loading

Don't know if that's an issue of this PR, but whenever I use row actions like hide/unhide/sync/unsync in the shared with me list, the avatar in the shared with column disappears. Everything else seems to work fine 💪 😍

That's because changing the status/hidden state re-fetches the resource from the old ocs api, resulting in slightly different data. This is not really needed though and will be fixed via the follow-up #10421 👍

@JammingBen JammingBen merged commit 7d66c11 into master Jan 31, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the sharing-ng-shared-with-me branch January 31, 2024 07:05
This was referenced Feb 27, 2024
Closed
Merged
saw-jan added a commit that referenced this pull request Mar 4, 2024
@saw-jan
test: port acceptance test codes from #10392
46302e1
saw-jan added a commit that referenced this pull request Mar 4, 2024
@S-Panta @saw-jan
test: port acceptance test codes from #10392 (#10521)
40018f2 
Co-authored-by: Saw-jan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann approved these changes

Assignees

@JammingBen JammingBen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[web] Make use of /me/drives/sharedwithme endpoint
2 participants
@JammingBen @kulmann