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] personal spaceId in URL #7053

Merged
merged 12 commits into from
Jun 13, 2022
Merged

[full-ci] personal spaceId in URL #7053

merged 12 commits into from
Jun 13, 2022

Conversation

AlexAndBear
Copy link
Contributor

@AlexAndBear AlexAndBear commented May 24, 2022 •
edited by kulmann
Loading

Description

We now include the personal space id in the URL instead of using a magic "home" alias. When using the old "home" alias the user gets redirected to the URL with their personal space id.

Related Issue

Motivation and Context

How Has This Been Tested?

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

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 May 24, 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 changed the title Start implementation of spaceId in personal home [full-ci] Start implementation of spaceId in personal home Jun 3, 2022
@kulmann kulmann changed the title [full-ci] Start implementation of spaceId in personal home [full-ci] personal spaceId in URL Jun 3, 2022
@kulmann kulmann mentioned this pull request Jun 3, 2022
Closed
25 tasks
@ownclouders
Copy link
Contributor

ownclouders commented Jun 3, 2022 •
edited
Loading

Results for oCISSharingAndUpload https://drone.owncloud.com/owncloud/web/26146/67/1

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

webUIUpload-upload_feature-L188.png

webUIUpload-upload_feature-L188.png

webUIUpload-upload_feature-L55.png

webUIUpload-upload_feature-L55.png

webUIUpload-upload_feature-L69.png

webUIUpload-upload_feature-L69.png

dschmidt
dschmidt approved these changes Jun 3, 2022
View reviewed changes
Copy link
Member

@dschmidt dschmidt left a comment

Choose a reason for hiding this comment

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

I think we should move the quota setting somewhere else, apart from that ok IMHO.

packages/web-app-files/src/services/folder/legacy/loaderPersonal.ts
@@ -52,7 +55,7 @@ export class FolderLoaderLegacyPersonal implements FolderLoader {

// fetch user quota
;(async () => {
const user = await client.users.getUser(ref.user.id)
const user = await client.users.getUser(router.currentRoute.params.storageId)
store.commit('SET_QUOTA', user.quota)
})()
Copy link
Member

Choose a reason for hiding this comment

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

This seems wrong, afaict this would update the user specific quota when viewing someone else's personal space.

What happens, when we use a bookmark to enter oC Web and skipping the personal view? Does the quota get loaded at all (it's visible from the user menu)? This seems to the wrong place after all.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, will create a followup issue to query quota from the backend in some other place (maybe quota component in the topbar, maybe something that polls with low frequency, let's see...)

Copy link
Member

Choose a reason for hiding this comment

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

created #7093 to follow up on this

packages/web-app-files/src/views/LocationPicker.vue
buildWebDavFilesPath(this.user.id, resource),
targetPath
)
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we reduce the code duplication here? This block is almost exactly the same as for the move operation.

Copy link
Member

Choose a reason for hiding this comment

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

Won't spend time on it since the location picker will most likely go away. 😅

@kulmann kulmann marked this pull request as ready for review June 3, 2022 10:25
@kulmann
Copy link
Member

kulmann commented Jun 7, 2022

found an issue in upload-only public links, which were also the/one reason why CI failed. fixing it now...

@kulmann kulmann mentioned this pull request Jun 7, 2022
Closed
@kulmann kulmann force-pushed the issues/7024 branch 2 times, most recently from 2bb5003 to 8944526 Compare June 9, 2022 14:01
@kulmann kulmann mentioned this pull request Jun 10, 2022
Closed
@kulmann kulmann force-pushed the issues/7024 branch 3 times, most recently from 87b8130 to 1eb252b Compare June 10, 2022 21:23
@kulmann
Copy link
Member

kulmann commented Jun 10, 2022

Somehow we can't get a working ocis in the e2e tests... fails with:

+ mc mirror s3/owncloud/web/ocis-build/$OCIS_COMMITID /var/www/owncloud/ocis-build/
5s
9	mc: <ERROR> Unable to stat source `s3/owncloud/web/ocis-build/123ddc6c9141ab634594d0fb531c72bf90ff6b58`. Object does not exist.

@sonarcloud
Copy link

sonarcloud bot commented Jun 13, 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 3 Code Smells

41.4% 41.4% Coverage
8.2% 8.2% Duplication

@kulmann kulmann merged commit b75b56d into master Jun 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the issues/7024 branch June 13, 2022 11:36
ownclouders pushed a commit that referenced this pull request Jun 13, 2022
commit b75b56d
e271d2c 
Author: Jan <jackermann@owncloud.com>
Date:   Mon Jun 13 13:36:07 2022 +0200

    [full-ci] personal spaceId in URL (#7053)

    Use personal storage id in url

    Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz>
This was referenced Jun 15, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann left review comments

@dschmidt dschmidt dschmidt approved these changes

@pascalwengerter pascalwengerter Awaiting requested review from pascalwengerter

@fschade fschade Awaiting requested review from fschade

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Personal space ids in URL
4 participants
@AlexAndBear @ownclouders @kulmann @dschmidt