owncloud / web

:dragon_face: Next generation frontend for ownCloud Infinite Scale
https://owncloud.dev/clients/web/
GNU Affero General Public License v3.0
443 stars 157 forks source link

Versions link opens web office #9868

Closed michaelstingl closed 12 months ago

michaelstingl commented 1 year ago

Describe the bug

"Show file versions in web browser" opens online office, not the file list with the versions open in the sidebar.

Steps to reproduce

Steps to reproduce the behavior:

  1. setup desktop client with einstein:relativity@ocis.ocis-wopi.latest.owncloud.works
  2. click "Show file versions in web browser" action for New file.odt in desktop file explorer

CleanShot 2023-10-25 at 18 46 33

Expected behavior

File list with the versions should open in the sidebar.

CleanShot 2023-10-25 at 18 51 09

Actual behavior

Web office opens

CleanShot 2023-10-25 at 18 51 54

Setup

[Log]  ownCloud Web UI 8.0.0-alpha.4  (index.html-3c2aed60.mjs, line 1)
[Log]  Infinite Scale 4.0.0+337dca17e Community  (index.html-3c2aed60.mjs, line 1)

Additional context

@TheOneRing does the desktop client compose the versions URL ? Would it be more robust if the desktop client would request the full URL from the backend?

michaelstingl commented 1 year ago

Actual behavior

Web office opens

Same for Share…, Copy private link to clipboard and Show in web browser right-click actions: https://ocis.ocis-wopi.latest.owncloud.works/f/0ad6c97c-f0ac-4c2d-b140-29e9ca52618c$4c510ada-c86b-4815-8820-42cdf82c3d51%2175478407-899c-447b-9131-8ec1e8cf0c05

kulmann commented 1 year ago

This is a regression from when we implemented auto-forwarding into editor applications.

JammingBen commented 1 year ago

Am I assuming correctly that the version URL is e.g. https://host.docker.internal:9201/f/e71d3e4f-3caa-4bc4-8e90-27ea80a45bd3%243966ad4f-ad72-41b2-a23e-b8ff73144d8e%218e0c2070-e9a6-4146-aa15-2d51404f084d?details=versions? Which means we should disable the auto-forwarding in case the details param is present in the URL.

kulmann commented 1 year ago

Am I assuming correctly that the version URL is e.g. https://host.docker.internal:9201/f/e71d3e4f-3caa-4bc4-8e90-27ea80a45bd3%243966ad4f-ad72-41b2-a23e-b8ff73144d8e%218e0c2070-e9a6-4146-aa15-2d51404f084d?details=versions? Which means we should disable the auto-forwarding in case the details param is present in the URL.

yes, correct. that solution would work: no auto-forwarding if details url param is present.

kulmann commented 1 year ago

Elevated the priority to p2. We want this fixed in the stable-7.1 branch and release it for ocis stable-4.0. If it was broken in stable-7.0 already then worth fixing it there as well.

To whoever fixes this, a unit test for the auto-forwarding being disabled if the details url param is present is appreciated. An e2e test as well but I will write a separate story for some url related e2e test scenarios.

kulmann commented 1 year ago

Probably makes sense to merge https://github.com/owncloud/web/pull/9821 first - but how do you feel about backporting that to stable-7.1 @JammingBen ? 🤔

JammingBen commented 1 year ago

Probably makes sense to merge #9821 first - but how do you feel about backporting that to stable-7.1 @JammingBen ? 🤔

Yep we should do that, although I'd like to get it merged to master first and then cherry-pick it to stable-7.1.

By the way, IMO this is the way we should handle our stable branches in the future. Until now, we merged all bug fixes to stable-7.1 and are now in that ugly situation where we don't want to release all of them in one hotfix.

Elevated the priority to p2. We want this fixed in the stable-7.1 branch and release it for ocis stable-4.0. If it was broken in stable-7.0 already then worth fixing it there as well.

Do you know the timeline for the ocis stable-4.0 release? Because this means there is quite a bit of work to do on our end 😅

kulmann commented 1 year ago

By the way, IMO this is the way we should handle our stable branches in the future. Until now, we merged all bug fixes to stable-7.1 and are now in that ugly situation where we don't want to release all of them in one hotfix.

Well, that's what the four eye principle (2 reviews required on stable branches) was intended for... having certainty that a patch release can happen at any time. In addition to that always possible to add new tests on the stable branch / in the PR with the code fix... but not a big deal, I'm also fine to have fixes on master and backport some of them to the respective stable branch. Downside: This reduces the chance of having frequent bugfixes in stable branches, because people will rarely do this.

Do you know the timeline for the ocis stable-4.0 release? Because this means there is quite a bit of work to do on our end 😅

No particular time line. Customers are happy about (reliable) patch releases though, so "as soon as possible but not prio 1".