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

web-app-external: view mode #9879

Merged
merged 11 commits into from
Nov 10, 2023
Merged

web-app-external: view mode #9879

merged 11 commits into from
Nov 10, 2023

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Oct 27, 2023

Description

Open non-personal files in "view" (readonly) mode when enabled via config: editor.openAsPreview.

Based on the logic implemented by CERN this currently is the case for all files in shares and public links.

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

@update-docs
Copy link

update-docs bot commented Oct 27, 2023

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.

@dschmidt dschmidt force-pushed the web-app-external-view-mode branch 2 times, most recently from 92554e5 to 052a40b Compare October 27, 2023 15:58
packages/web-app-external/src/App.vue Outdated Show resolved Hide resolved
packages/web-app-external/src/App.vue Show resolved Hide resolved
packages/web-app-external/src/App.vue Outdated Show resolved Hide resolved
@micbar
Copy link
Contributor

micbar commented Oct 27, 2023

Our product supports editing in a public link share. We must do this based on the share permissions

@dschmidt
Copy link
Member Author

dschmidt commented Oct 27, 2023

Our product supports editing in a public link share. We must do this based on the share permissions

This is not about forbidding it completely, it's just about opening files in shares/public links in read only mode by default - the user can then click edit and switch to edit mode given they have the permission to do edit the file

@micbar
Copy link
Contributor

micbar commented Oct 27, 2023

Hmm i would expect only one smart open action which always uses the highest possible permission.

@dschmidt
Copy link
Member Author

Yeah, so config option to make it opt in.

Why I pinged you tho: is Office365 as wopi app name fixed or is that configurable?

@micbar
Copy link
Contributor

micbar commented Oct 27, 2023

Configurable in the app provider

@dschmidt dschmidt changed the title web-app-external: view mode wip: web-app-external: view mode Oct 28, 2023
@dschmidt dschmidt force-pushed the web-app-external-view-mode branch from b083464 to ea802d5 Compare November 8, 2023 18:11
@dschmidt dschmidt changed the title wip: web-app-external: view mode web-app-external: view mode Nov 9, 2023
@JammingBen JammingBen requested a review from kulmann November 9, 2023 13:52
try {
if (props.isReadOnly && viewMode === 'write') {
Copy link
Contributor

@lookacat lookacat Nov 9, 2023

Choose a reason for hiding this comment

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

would it make sense to create an enum for the viewMode since its used at multiple locations?

Copy link
Member Author

Choose a reason for hiding this comment

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

seems overkill to me tbh. In theory we could also support more modes which are defined by the backend... this is just some specific handling for an edge case

@dschmidt dschmidt force-pushed the web-app-external-view-mode branch from e2a6b61 to 05ed300 Compare November 9, 2023 18:23
packages/web-pkg/src/configuration/manager.ts Outdated Show resolved Hide resolved
packages/web-app-external/src/App.vue Outdated Show resolved Hide resolved
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

62.5% 62.5% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@JammingBen JammingBen merged commit a2da2bf into master Nov 10, 2023
3 of 4 checks passed
@delete-merged-branch delete-merged-branch bot deleted the web-app-external-view-mode branch November 10, 2023 11:22
ownclouders pushed a commit that referenced this pull request Nov 10, 2023
* web-app-external: open files in shares and public links in view mode by default

* Simplify logic by not handling everything in a watcher

* Show error popup when trying to switch to edit mode for read-only files

* Simplify logic some more

* Remove obsolete unref

* Remove console output

* Implement feature flag

* run linter

* test: fix unit tests

* add config to dev docs

* fixup! add config to dev docs

---------

Co-authored-by: Jannik Stehle <[email protected]>
return
}

debugger
Copy link
Contributor

Choose a reason for hiding this comment

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

@dschmidt you forgot to delete it
Screenshot 2023-11-16 at 15 55 50

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll drop it in my PR

Copy link
Member Author

Choose a reason for hiding this comment

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

doh 😂

AlexAndBear pushed a commit that referenced this pull request Dec 13, 2023
* web-app-external: open files in shares and public links in view mode by default

* Simplify logic by not handling everything in a watcher

* Show error popup when trying to switch to edit mode for read-only files

* Simplify logic some more

* Remove obsolete unref

* Remove console output

* Implement feature flag

* run linter

* test: fix unit tests

* add config to dev docs

* fixup! add config to dev docs

---------

Co-authored-by: Jannik Stehle <[email protected]>
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.

O365: Open Non "Personal" Documents in View Mode per Default
7 participants