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.

Sign up for GitHub

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

Jump to bottom

feat(directediting): Allow opening by file id #36727

Merged
merged 1 commit into from
Feb 20, 2023
Merged

feat(directediting): Allow opening by file id #36727

merged 1 commit into from
Feb 20, 2023

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 15, 2023 •
edited
Loading

Summary

Make sure that it is easy for the notes iOS/Android apps to open files by just knowing the file id. For cases where multiple files with the id are found we prefer the one with update permissions over others.

Checklist

@juliusknorr juliusknorr added enhancement 3. to review Waiting for reviews labels Feb 15, 2023
@juliusknorr juliusknorr added this to the Nextcloud 26 milestone Feb 15, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 15, 2023
View reviewed changes
apps/files/lib/Controller/DirectEditingController.php
if (!$this->directEditingManager->isEnabled()) {
return new DataResponse(['message' => 'Direct editing is not enabled'], Http::STATUS_INTERNAL_SERVER_ERROR);
}
$this->eventDispatcher->dispatchTyped(new RegisterDirectEditorEvent($this->directEditingManager));

try {
$token = $this->directEditingManager->open($path, $editorId);
$token = $this->directEditingManager->open($path, $editorId, $fileId);

Check notice

Code scanning / Psalm

UndefinedInterfaceMethod

Method OCP\DirectEditing\IManager::open does not exist
@juliusknorr juliusknorr force-pushed the enh/file-id-direct-editing branch from 6ff560a to 6abf5d3 Compare February 16, 2023 07:29
@AlvaroBrey
Copy link
Member

@juliushaertl any possibility to add a capability indicating whether this change is available? Otherwise I'll just check if server version >= 26

@AlvaroBrey AlvaroBrey mentioned this pull request Feb 16, 2023
Merged
16 tasks
@juliusknorr
Copy link
Member Author

Capability added:

Screenshot 2023-02-16 at 15 02 33

@juliusknorr juliusknorr force-pushed the enh/file-id-direct-editing branch from 6abf5d3 to 7ff5b5a Compare February 16, 2023 14:03
max-nextcloud
max-nextcloud approved these changes Feb 20, 2023
View reviewed changes
Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Code looks good.

@max-nextcloud
Copy link
Contributor

Maybe add a test?
I did not find any tests for this controller - but I may have missed them. However there's https://github.com/nextcloud/server/blob/master/tests/lib/DirectEditing/ManagerTest.php which might be fairly easy to extend?

@juliusknorr
feat(directediting): Allow opening by file id
614981a 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr force-pushed the enh/file-id-direct-editing branch from 7ff5b5a to 614981a Compare February 20, 2023 11:16
@juliusknorr
Copy link
Member Author

Pushed two further cases to cover the open method with and without a file id.

mejo-
mejo- approved these changes Feb 20, 2023
View reviewed changes
Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

@juliusknorr
Copy link
Member Author

Failure unrelated.

@juliusknorr juliusknorr merged commit 7858b83 into master Feb 20, 2023
@juliusknorr juliusknorr deleted the enh/file-id-direct-editing branch February 20, 2023 13:39
@marinofaggiana
Copy link
Member

@juliushaertl thanks

@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
Merged
@marinofaggiana
Copy link
Member

marinofaggiana commented Mar 5, 2023 •
edited
Loading

@juliushaertl I have tested it, example:

https://cloud.nextcloud.com/ocs/v2.php/apps/files/api/v1/directEditing/open?fileId=7155303&editorId=text
Response Error 500

@AlvaroBrey
Copy link
Member

@juliushaertl I have tested it, example:

https://cloud.nextcloud.com/ocs/v2.php/apps/files/api/v1/directEditing/open?fileId=7155303&editorId=text Response Error 500

You need to also provide path, with the value being the notes storage path (from https://github.com/nextcloud/notes/blob/master/docs/api/v1.md#settings).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@max-nextcloud max-nextcloud max-nextcloud approved these changes

@mejo- mejo- mejo- approved these changes

@marcelklehr marcelklehr marcelklehr approved these changes

@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

@julien-nc julien-nc Awaiting requested review from julien-nc

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
6 participants
@juliusknorr @AlvaroBrey @max-nextcloud @marinofaggiana @marcelklehr @mejo-