nextcloud / nextcloud/richdocuments

Creating files from mobile clients fails

Open
#5,839 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
453
Forks
147
Avg merge
14h 54m
Merged PRs (30d)
83

Description

So at the risk of being really verbose:

The createDirect() change is right: returning the native richdocuments.directView.show webview instead of files.DirectEditingView.edit means the URL no longer depends on the editor being registered through OCP\DirectEditing, so it renders regardless of client version. The DirectEditingMobileInterface forwarding in mobile.js matches what text already does for the shared direct-editing webview, and token handling is unchanged (64-char CSPRNG, single-use, 10-min TTL), no new exposure.

What still fails: createFromTemplate() (POST /api/v1/templates/new) wasn't given the same treatment, it still returns a files.DirectEditingView.edit URL. So a <v34 mobile client creating a new document from a template:

  1. POSTs to /api/v1/templates/new; the endpoint force-registers the editor inline, create() succeeds, and hands back a files.DirectEditingView.edit?token=… URL.
  2. Loads that URL in its webview. The GET carries the legacy iOS/Android user-agent, so RegisterDirectEditorListener::shouldExposeEditor() gates richdocuments out (major < 34), the token's editor can't be resolved, and the render fails — 404 on iOS, infinite spinner on Android.

Same symptom this PR fixes for opening, on the same entry-point family. Reachable from the shipping apps — Android hits /api/v1/templates/new via CreateFileFromTemplateOperation, iOS via NextcloudKit/ios-communication-library — so "new doc from template" on a v33 app against a v34 server still breaks after this merges.

createFromTemplate is the only endpoint still on the gated flow: createDirect, createPublic, createPublicFromInitiator already use richdocuments.directView.show.

Fix (same shape as the createDirect change here): recreate the file explicitly and carry the template id on the direct token; DirectViewController::show() already primes the template via primeTemplateSource(). As a bonus this removes the last users of directEditingManager/officeDirectEditor, so those imports and constructor deps can go too. You could fold it into here, or file a new issue and create a new PR for better tracking.

I've not checked this code, so verify it please:

1. lib/Controller/OCSController.phpcreateFromTemplate() (replaces lines 290–336)
	public function createFromTemplate($path, $template) {
		if ($path === null || $template === null) {
			throw new OCSBadRequestException('path and template must be set');
		}

		if (!$this->manager->isTemplate($template)) {
			throw new OCSBadRequestException('Invalid template provided');
		}

		try {
			$info = $this->mb_pathinfo($path);

			$userFolder = $this->rootFolder->getUserFolder($this->userId);
			$folder = isset($info['dirname']) ? $userFolder->get($info['dirname']) : $userFolder;
			$name = $folder->getNonExistingName($info['basename']);
			$file = $folder->newFile($name);

			$direct = $this->directMapper->newDirect($this->userId, $file->getId(), $template);

			return new DataResponse([
				'url' => $this->urlGenerator->linkToRouteAbsolute('richdocuments.directView.show', [
					'token' => $direct->getToken(),
				]),
			]);
		} catch (NotFoundException) {
			throw new OCSNotFoundException();
		} catch (\Exception $e) {
			$this->logger->error($e->getMessage(), ['exception' => $e]);
			throw new OCSException('Failed to create new file from template.');
		}
	}

Notes:

  • newFile($name) recreates the empty file (create() used to do this); newDirect(..., $template) carries the template id on the token.
  • Dropped the catch (OCSBadRequestException $e) { throw $e; } — nothing inside the try throws it any more (the getTemplateTypeForMime null-check went away; isTemplate() validation stays at the top).
  • Mime-based creator derivation is no longer needed — it only mattered for create()'s creator routing; the native flow resolves the type from the file/template on render.
2. lib/Controller/OCSController.php — now-dead imports / deps to remove
// remove these imports:
use OCA\Richdocuments\AppInfo\Application;                 // line 12
use OCA\Richdocuments\DirectEditing\OfficeDirectEditor;    // line 14
use OCP\DirectEditing\IManager as IDirectEditingManager;   // line 27

// remove these constructor params:
private IDirectEditingManager $directEditingManager,       // line 54
private OfficeDirectEditor $officeDirectEditor,            // line 55

(These were the only remaining users after createDirect was reverted; safe to drop — nothing external constructs this controller.)

3. cypress/e2e/direct.spec.js — new test mirroring the open-path one
	it('Signals DirectEditingMobileInterface when creating from a template', function() {
		getTemplates(randUser, 'document')
			.then((templates) => {
				const emptyTemplate = templates.find((template) => template.name === 'Empty')
				createNewFileDirectEditingLink(randUser, 'mobile-template.odt', emptyTemplate.id)
					.then((token) => {
						cy.logout()
						cy.visit(token, {
							onBeforeLoad(win) {
								win.DirectEditingMobileInterface = {
									loaded: cy.stub().as('directEditingLoaded'),
									documentLoaded: cy.stub().as('directEditingDocumentLoaded'),
									close: cy.stub().callsFake(() => win.documentsMain.onClose()),
								}
								cy.spy(win, 'postMessage').as('postMessage')
							},
						})
						cy.waitForCollabora(false)
						cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
						cy.get('@directEditingLoaded').should('have.been.called')
						cy.get('@directEditingDocumentLoaded').should('have.been.called')
						cy.closeDirectDocument()
					})
			})
	})

Also add mobile-template.odt to the spec's afterEach cleanup (currently deletes document.odt/mynewfile.odt/document.rtf) so repeated local runs don't accumulate mobile-template (2).odt via getNonExistingName.

Originally posted by @moodyjmz in https://github.com/nextcloud/richdocuments/issues/5805#issuecomment-4922105975

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with createFromTemplate() in lib/Controller/OCSController.php and compare its flow with createDirect(). Review cypress/e2e/direct.spec.js and the existing open-path test before running the direct-editing Cypress suite. Done means template creation uses the native direct-view path, obsolete controller dependencies are removed, the mobile callback test passes, and mobile-template.odt is cleaned up.

Written by the indexing model from the issue text.

Assessment

Tech stack
cypress, javascript, php
Domain
api, backend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.