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

[feature/shortcut-uploads] Shortcut uploads and error handling improvements #930

Merged
merged 10 commits into from
Apr 9, 2021

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Mar 12, 2021

Description

Update to Shortcuts support:

  • Save File (Uploads)
    • add a "wait for completion" option
    • delegate uploads to the FileProvider, so they can upload in the background
  • Create Folder
    • add a "wait for completion" option
  • improved error handling - and now also reporting authentication errors
  • improved behaviour by
    • waiting for the connection to the server to complete for up to 5 seconds
    • keeping managed OCCores alive while scheduling work
    • ensuring intent completion handler can never be called twice
  • refactor/rewrite of OCItemTracker
  • typo fixed error messages
  • implement new iOS 14 only methods required by Xcode 12.4
  • File Provider tunneling support for file replacements (report content modification)

Screenshots

Error when authentication fails

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)

QA

QA Checks: #930 (comment)

- add "Wait for upload to finish" option to SaveFileIntentHandler (implementation of the functionality still pending)
- FileProviderServiceSource:
	- add additional logging
	- add new method for reporting local modifications
- OCFileProviderServiceSession:
	- plug error handling hole
	- add support for new method for reporting local modifications
- SaveFileIntentHandler:
	- refactor and partial rewrite to support new option to wait for completion
	- add support for import and update through the FP for almost immediate return with the file being uploaded in the background
	- fix provideAccountOptionsCollection implementation
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@felix-schwarz felix-schwarz changed the title [feature/shortcut-uploads] Shortcut Upo [feature/shortcut-uploads] Shortcut Upload improvements Mar 12, 2021
@felix-schwarz felix-schwarz changed the title [feature/shortcut-uploads] Shortcut Upload improvements [feature/shortcut-uploads] Shortcut Upload improvements [WIP] Mar 12, 2021
- rewrite OCItemTracker helper to support additional features:
	- report core errors (withErrorHandler, on by default)
	- delay item tracking until the core's connection status is online or a timeout is met (waitOnlineTimeout, off by default)
- OCIssue extension to simplify identification and access to authentication errors from OCIssues and Error

Intents: big refactoring and expansion
- CreateFolder: add "wait for completion" option, report authentication errors, wait (with timeout) for core to become online
- DeletePath: report authentication errors, wait (with timeout) for core to become online
- GetDirectoryListing: add missing return of requested core, report authentication errors
- GetFileInfo: report authentication errors, wait (with timeout) for core to become online
- GetFile: allow time for the core to find newly added files, report authentication errors, wait (with timeout) for core to become online
- PathExists: report authentication errors, wait (with timeout) for core to become online
- SaveFile: report authentication errors, wait (with timeout) for core to become online (pending cleanup)
- fix typo in localization: "The given path does not exists" -> "The given path does not exist."

ownCloud:
- adopt improvements from ownCloudAppShared
…- do not wait for timeout before starting item tracking in that case

- SaveFile Intent: clean up code, streamline error reporting, report authentication errors
@felix-schwarz felix-schwarz changed the title [feature/shortcut-uploads] Shortcut Upload improvements [WIP] [feature/shortcut-uploads] Shortcut uploads and error handling improvements Mar 13, 2021
@felix-schwarz felix-schwarz added this to the 11.6.0-Current milestone Mar 13, 2021
… is not online and a download therefore not possible
ownCloud Intents/SaveFileIntentHandler.swift Show resolved Hide resolved
ownCloud Intents/SaveFileIntentHandler.swift Show resolved Hide resolved
ownCloud Intents/GetFileIntentHandler.swift Show resolved Hide resolved
ownCloud Intents/GetFileIntentHandler.swift Show resolved Hide resolved
@jesmrec
Copy link
Contributor

jesmrec commented Apr 5, 2021

QA checks:

  • Execute shortcut "Create Folder" with Wait for folder creation to complete
  • Execute shortcut "Create Folder" without Wait for folder creation to complete)
  • Execute shortcut "Create Folder" in non-root with Wait for folder creation to complete
  • Execute shortcut "Create Folder" in non-root without Wait for folder creation to complete
  • Execute shortcut "Create Folder" with OAuth2 session expired -> [feature/shortcut-uploads] Shortcut uploads and error handling improvements #930 (comment)
  • Execute shortcut "Save File" with Wait for folder creation to complete
  • Execute shortcut "Save File" without Wait for folder creation to complete
  • Execute shortcut "Save File" with OAuth2 session expired

@jesmrec
Copy link
Contributor

jesmrec commented Apr 5, 2021

(1)

  1. Create a simple shortcut that creates a folder in the account, like:

Screenshot 2021-04-05 at 10 24 08

  1. In web dashboard, revoke token
  2. Execute the shortcut

Current:

Screenshot 2021-04-05 at 10 25 13

Expected:

Screenshot 2021-04-05 at 10 27 17

Commit c087c6aa
iPhoneXR v14.4

@jesmrec
Copy link
Contributor

jesmrec commented Apr 5, 2021

(2)

Kind of regression

Checking the Select Photos action, it has a sub-option called Select Multiple.

In case you select that option and then, a Save File ownCloud action, the shortcut will allow you to select multiple Photos, but, at the end, only one will be uploaded to ownCloud.

Is this a new behaviour? should we address to a new ticket?

@jesmrec
Copy link
Contributor

jesmrec commented Apr 5, 2021

(3)

Also, kind of regression.

  1. Remove device connection
  2. Execute a "Create folder" shortcut

The following error message is displayed. Is this the expected behaviour?:

Screenshot 2021-04-05 at 10 25 13

In case the executed shortcut is a "Save File", the displayed error is An unknown error occurred

Expected: a correct no-connection message (feasible?)

Commit c087c6aa
iPhoneXR v14.4

- fix finding (1) from #930: wrong error message trying to create a folder in an account with invalid token
…ing, remove multiple implementations in extensions/categories
@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented Apr 8, 2021

@jesmrec Regarding the findings:

(1)

Thanks! I could reproduce and fix this in acf2f3d.

(2)

Save File has only accepted a single file in the past, too, because it is defined that way:

Bildschirmfoto 2021-04-08 um 12 17 43

We could expand it to accept and upload more than one file, but this would be outside the scope of this PR IMO. Please file an issue.

- add support for differentiated network unavailable error reporting
- add .isNetworkConnectionError NSError/Error glue code
@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented Apr 8, 2021

@jesmrec

(3)

Thanks! Networking errors weren't handled in a differentiated way. Fixed that with ec1a0bc .

@jesmrec
Copy link
Contributor

jesmrec commented Apr 9, 2021

(1) and (3) are fixed with 451a35b8

(2) addressed to #947

@jesmrec
Copy link
Contributor

jesmrec commented Apr 9, 2021

This is approved on my side. You can check above the QA checks

@jesmrec jesmrec added the Approved by QA Approved by QA label Apr 9, 2021
@hosy hosy merged commit 13f35b4 into milestone/11.6 Apr 9, 2021
@delete-merged-branch delete-merged-branch bot deleted the feature/shortcut-uploads branch April 9, 2021 08:19
@felix-schwarz
Copy link
Contributor Author

@jesmrec Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved by QA Approved by QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Shortcut actions needs to throw an error when access token is expired
4 participants