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.

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

[full-ci] Fix several file handling issues for apps #6456

Merged
merged 7 commits into from
Mar 4, 2022

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Feb 19, 2022

Description

This PR fixes a bunch of scenarios where mediaviewer (and possibly other apps) were broken:

  • getUrlForResource was using an ugly hack to retrieve a webdav url before webDavPath was introduced in Add space front page & bump SDK to v2.0.0 #6287, we could now get rid of that hack and make this function work for spaces and a bunch of other contexts work as well (Kudos for that PR!!)
  • mediaviewer: clicking next/previous updated the url with the resource path instead of the webDavPath: that broke reloading
  • router.parseQuery: our qs.parse invocation was missing the allowDots option, so nested objects in query were not correctly parsed and closing the apps, at least on public links was broken

Related Issues

There are two oCIS bugs left, which cause the mediaviewer to fail

With oC 10 everything seems to work just fine.

Biggest remaining issue is, that reloading the mediaviewer will only load the correct context folder for personal files or public links - i.e. if you open the mediaviewer in the shares overview you can cycle through your shared pictures, if you reload the page you will cycle through the folder from which the file was shared.
Fixing that is way beyond the scope of this PR and needs to be handled in a follow up PR.

Motivation and Context

It's annoying when the mediaviewer or other apps don't work in all contexts :-P

How Has This Been Tested?

Apart from the before mentioned issues, I'm not aware of any remaining ones.
With these changes the mediaviewer works in Personal, Favorites, Shared With Others, Shared Via Link and Spaces views.

Just Shared With Me I haven't tested yet. Shared With Me was tested, works.

Query Parsing seems to behave a little weird, needs to be double checked again.
I needed the change to fix closing apps on public links, but now reloading seems broken in Personal. Marking the PR as WIP for now.
Seems to have been a hiccup, cannot reproduce anymore.

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:

Open tasks:

  • ...

@update-docs
Copy link

update-docs bot commented Feb 19, 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.

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.

Looks really promising already! 😍

@ownclouders
Copy link
Contributor

Results for oCISSharingAndUpload https://drone.owncloud.com/owncloud/web/23237/66/1

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

webUIUpload-upload_feature-L94.png

webUIUpload-upload_feature-L94.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

@dschmidt dschmidt changed the title WIP: Fix several file handling issues for apps [full-ci] Fix several file handling issues for apps Mar 3, 2022
@ownclouders
Copy link
Contributor

Results for oCISFiles3 https://drone.owncloud.com/owncloud/web/23243/57/1

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

webUILogin-openidLogin_feature-L22.png

webUILogin-openidLogin_feature-L22.png

webUILogin-openidLogin_feature-L29.png

webUILogin-openidLogin_feature-L29.png

@ownclouders
Copy link
Contributor

Results for oCISSharingAndUpload https://drone.owncloud.com/owncloud/web/23243/66/1
The following scenarios passed on retry:

  • webUIUpload/upload.feature:188

@ownclouders
Copy link
Contributor

Results for oCISSharingPublic1 https://drone.owncloud.com/owncloud/web/23243/67/1
The following scenarios passed on retry:

  • webUISharingPublicBasic/publicLinkCreate.feature:190

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.

Looking good so far, two minor comments on the changelog and waiting for more incoming changes (?)

@@ -0,0 +1,6 @@
Bugfix: File handling in apps

We fixed loading and saving files in apps in all contexts. It's now to open files in apps in personal files, favorites, share views and spaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
We fixed loading and saving files in apps in all contexts. It's now to open files in apps in personal files, favorites, share views and spaces.
We fixed loading and saving files in apps in all contexts. It's now possible to open files in apps in personal files, favorites, share views and spaces.


We fixed loading and saving files in apps in all contexts. It's now to open files in apps in personal files, favorites, share views and spaces.

https://github.com/owncloud/web/pull/6456/files
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
https://github.com/owncloud/web/pull/6456/files
https://github.com/owncloud/web/pull/6456

@sonarcloud
Copy link

sonarcloud bot commented Mar 4, 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 2 Code Smells

20.0% 20.0% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

Results for oCISSharingPublic1 https://drone.owncloud.com/owncloud/web/23267/67/1
The following scenarios passed on retry:

  • webUISharingPublicBasic/publicLinkCreate.feature:190

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.

LGTM

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.

4 participants