Improvements for app provider #6003

diocas · 2021-11-09T08:26:01Z

This PR fixes 2 bugs for the app-provider extension and adds error messages to the UI.

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Manually

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 · 2021-11-09T08:26:05Z

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.

packages/web-app-external/src/App.vue

packages/web-app-external/src/components/ErrorScreen.vue

packages/web-app-files/src/mixins/fileActions.js

pascalwengerter

Changes look pretty good (just ran them locally against the oCIS-wopi example. To go forward with this, you'll have to run yarn test:unit packages/web-app-external/ -u to update & commit the snapshot tests (and we'll need to investigate why the CI died for you before even reaching them)

pascalwengerter · 2021-11-09T19:17:13Z

Changes look pretty good (just ran them locally against the oCIS-wopi example. To go forward with this, you'll have to run yarn test:unit packages/web-app-external/ -u to update & commit the snapshot tests (and we'll need to investigate why the CI died for you before even reaching them)

Also, could you add one (or two?) bugfix changelog item(s) in the unreleased folder? Same idea/structure/process as in REVA from my understanding

ownclouders · 2021-11-09T21:54:31Z

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/20177/14/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

diocas · 2021-11-10T08:17:19Z

Is the pipeline breaking in sharing? I don't see the relation to this PR. Can you help me?

pascalwengerter · 2021-11-10T08:25:08Z

Is the pipeline breaking in sharing? I don't see the relation to this PR. Can you help me?

Yeah we'll do a code review & CI check after standup today!

ownclouders · 2021-11-10T22:10:40Z

Results for oCISFavorites https://drone.owncloud.com/owncloud/web/20223/53/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-10T22:10:41Z

Results for oCISResharing1 https://drone.owncloud.com/owncloud/web/20223/62/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-10T22:10:44Z

Results for oC10XGAPortrait2 https://drone.owncloud.com/owncloud/web/20223/45/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-10T22:10:45Z

Results for oC10XGAPortrait1 https://drone.owncloud.com/owncloud/web/20223/44/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-10T22:10:48Z

Results for oCISFiles1 https://drone.owncloud.com/owncloud/web/20223/54/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-10T22:10:49Z

Results for oC10SharingExternalRoot https://drone.owncloud.com/owncloud/web/20223/42/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-10T22:10:52Z

Results for oC10XGAPortraitNotifications https://drone.owncloud.com/owncloud/web/20223/43/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

pascalwengerter

Sorry for being this naggy, found more small stuff 😬

changelog/unreleased/enhancement-app-provider-feedback

changelog/unreleased/bugfix-no-context-menu-app-provider

pascalwengerter · 2021-11-11T14:35:26Z

packages/web-app-external/tests/unit/components/__snapshots__/ErrorScreen.spec.ts.snap

@@ -3,6 +3,7 @@
 exports[`The external app error screen component displays an icon and a paragraph 1`] = `
 <div class="uk-text-center">
  <ocicon-stub size="xxlarge" name="warning"></ocicon-stub>
-  <p data-msgid="Error when loading the application" data-current-language="en_US">Error when loading the application</p>
+  <h1 class="oc-text-lead" data-msgid="Error when loading the application" data-current-language="en_US">Error when loading the application</h1>
+  <!---->


Would like to see a scenario where an error message is given and rendered in the ErrorScreen test!

I'm not sure how to add tests... Can you explain/give some pointers?

The test lives close to the component (https://github.com/owncloud/web/blob/master/packages/web-app-external/tests/unit/components/ErrorScreen.spec.ts) and should be pretty self-explanatory:

describe steps (strings) to explain the context

it steps (strings) to assert certain behavior

components get mounted and then you can assert a certain DOM structure (snapshot tests, like in this case), check if user interactions trigger correct behavior (by e.g. mocking clicks) etc

you can mount them passing props (in this case you'd want one snapshot test with and one without the errorMessage and check if the <p> tag gets rendered if an errorMessage is given

Feel free to get inspired by the other tests, usually the files app offers more diverse test scenarios: https://github.com/owncloud/web/tree/master/packages/web-app-files/tests/unit/components

pascalwengerter · 2021-11-11T14:36:03Z

packages/web-app-external/tests/unit/__snapshots__/app.spec.js.snap

  <h1 class="oc-invisible-sr">"exampleApp" app page</h1>
-  <errorscreen-stub></errorscreen-stub>
+  <loadingscreen-stub></loadingscreen-stub>


Somehow this doesn't render the errorscreen anymore, could you investigate?

@elizavetaRa can you have a look?

pascalwengerter · 2021-11-11T14:36:08Z

packages/web-app-external/tests/unit/__snapshots__/app.spec.js.snap

  <h1 class="oc-invisible-sr">"exampleApp" app page</h1>
-  <errorscreen-stub></errorscreen-stub>
+  <loadingscreen-stub></loadingscreen-stub>


Somehow this doesn't render the errorscreen anymore, could you investigate?

packages/web-app-files/src/mixins/fileActions.js

diocas · 2021-11-11T17:19:35Z

Np! I'm super picky when reviewing PRs so I expect the same from others :)

ownclouders · 2021-11-11T17:26:06Z

Results for oC10SharingPermissionsUsers https://drone.owncloud.com/owncloud/web/20262/33/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2021-11-16T12:23:28Z

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/20315/14/1
The following scenarios passed on retry:

webUISharingAcceptSharesToRoot/acceptShares.feature:108

diocas · 2021-12-02T11:29:58Z

Updated the branch with latest master. @elizavetaRa can you check @pascalwengerter message?

Somehow this doesn't render the errorscreen anymore, could you investigate?

@pascalwengerter sorry, didn't have time to add tests. Could your QA team do it?

Avoid scrollbars on apps

sonarcloud · 2021-12-09T09:49:11Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

diocas · 2021-12-13T14:56:51Z

Can this be merged?
I saw that @dschmidt is handling #6002 but that is already fixed in this PR.
I guess the only thing missing are some tests.

pascalwengerter · 2021-12-13T15:29:54Z

I'll merge this PR into a temporary branch in web in order to being able to fix the tests myself. Thanks @diocas for the contribution!

diocas requested review from fschade, kulmann and pascalwengerter as code owners November 9, 2021 08:26

pascalwengerter requested changes Nov 9, 2021

View reviewed changes

diocas force-pushed the up_app_provider branch from 782a2f0 to 47c81fa Compare November 9, 2021 12:56

diocas added the Early-Adopter:CERN label Nov 9, 2021

pascalwengerter reviewed Nov 9, 2021

View reviewed changes

diocas force-pushed the up_app_provider branch from 47c81fa to 199a4ed Compare November 9, 2021 21:16

diocas force-pushed the up_app_provider branch from 9e15e52 to 67a9ed7 Compare November 10, 2021 21:49

pascalwengerter reviewed Nov 11, 2021

View reviewed changes

diocas force-pushed the up_app_provider branch 2 times, most recently from 82c1ea1 to 8d9b664 Compare November 16, 2021 11:56

diocas force-pushed the up_app_provider branch from 8d9b664 to e35cd69 Compare December 2, 2021 11:28

Fix app provider mimetype resolution

5a0f12f

diocas added 3 commits December 9, 2021 10:41

App provider: show errors

b68b982

Avoid scrollbars on apps

Update snapshot tests

40e0e7d

Changelog

834dd67

diocas force-pushed the up_app_provider branch from e35cd69 to 834dd67 Compare December 9, 2021 09:41

pascalwengerter changed the base branch from master to external-apps-errorhandling December 13, 2021 15:29

pascalwengerter merged commit 72cacbd into owncloud:external-apps-errorhandling Dec 13, 2021

pascalwengerter mentioned this pull request Dec 13, 2021

External apps errorhandling #6145

Merged

diocas deleted the up_app_provider branch December 15, 2021 11:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements for app provider #6003

Improvements for app provider #6003

diocas commented Nov 9, 2021

update-docs bot commented Nov 9, 2021

pascalwengerter left a comment

pascalwengerter commented Nov 9, 2021 •

edited

Loading

ownclouders commented Nov 9, 2021

diocas commented Nov 10, 2021

pascalwengerter commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

pascalwengerter left a comment

pascalwengerter Nov 11, 2021

diocas Nov 11, 2021

pascalwengerter Nov 11, 2021

pascalwengerter Nov 11, 2021

diocas Nov 11, 2021

pascalwengerter Nov 11, 2021

diocas commented Nov 11, 2021

ownclouders commented Nov 11, 2021

ownclouders commented Nov 16, 2021

diocas commented Dec 2, 2021

sonarcloud bot commented Dec 9, 2021

diocas commented Dec 13, 2021 •

edited

Loading

pascalwengerter commented Dec 13, 2021

Improvements for app provider #6003

Improvements for app provider #6003

Conversation

diocas commented Nov 9, 2021

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Open tasks:

update-docs bot commented Nov 9, 2021

pascalwengerter left a comment

Choose a reason for hiding this comment

pascalwengerter commented Nov 9, 2021 • edited Loading

ownclouders commented Nov 9, 2021

diocas commented Nov 10, 2021

pascalwengerter commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

ownclouders commented Nov 10, 2021

pascalwengerter left a comment

Choose a reason for hiding this comment

pascalwengerter Nov 11, 2021

Choose a reason for hiding this comment

diocas Nov 11, 2021

Choose a reason for hiding this comment

pascalwengerter Nov 11, 2021

Choose a reason for hiding this comment

pascalwengerter Nov 11, 2021

Choose a reason for hiding this comment

diocas Nov 11, 2021

Choose a reason for hiding this comment

pascalwengerter Nov 11, 2021

Choose a reason for hiding this comment

diocas commented Nov 11, 2021

ownclouders commented Nov 11, 2021

ownclouders commented Nov 16, 2021

diocas commented Dec 2, 2021

sonarcloud bot commented Dec 9, 2021

diocas commented Dec 13, 2021 • edited Loading

pascalwengerter commented Dec 13, 2021

pascalwengerter commented Nov 9, 2021 •

edited

Loading

diocas commented Dec 13, 2021 •

edited

Loading