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

manual edit of a note's title #474

Merged
merged 4 commits into from
Apr 13, 2020
Merged

manual edit of a note's title #474

merged 4 commits into from
Apr 13, 2020

Conversation

korelstar
Copy link
Member

@korelstar korelstar commented Feb 25, 2020

Fix #190

  • new: edit note title manually (web frontend)
  • don't change title automatically (web frontend)
  • autotitle for new notes (web frontend)
  • add capabilities over Nextcloud OCS API for API versioning
  • maintenance: complete refactoring/rewrite of NotesService and other back-end classes
  • new API v1: don't change title automatically
  • documentation for new API

Screenshots

Actions

Screenshot

Edit mode

Screenshot

@korelstar korelstar added feature: app navigation Related to the app navigation feature: backend Related to the PHP backend labels Feb 25, 2020
@stefan-niedermann
Copy link
Member

Hey @korelstar,

would be nice if you could give me an overview about the changes you are planning / making 🙂.

Will the API be up- and/or downcompatible?

Thank you in advance

@korelstar
Copy link
Member Author

Regarding the general functionality: I've added some screenshots to the original posting. Third-party apps (I call them "mobile apps" in the following) will have to implement the functionality to edit the title, too.

API v0.2

My goal is to be as much compatible as possible. I think we have three options. For the evaluation, we have to differ between people who are using both web app and mobile app, and people who are using the mobile app only (I think they exist).

Option 1: leave behavior as is

Behavior definition

  • If the content is changed through the API, the title is updated using the old algorithm (I will call it getTitleFromContent here).
  • If the title is changed through the API, the note's title is untouched (current behavior, title is ignored).

Evaluation

  • Pro: Behavior of old mobile apps don't change.
  • Con: old mobile apps change notes' title on content change while web app has different behavior => Confusing for users (e.g. if a user changes the title manually using the web app and afterwards changes the note's content using an old mobile app, then the title is overwritten with getTitleFromContent).
  • We have to provide a new major version of the API for new mobile apps, which set the title explicitly.

Option 2: no automatic change of title

Behavior definition

  • If the content is changed through the API, the title is untouched.
  • If the title is changed through the API, the note's title is set to the new title.

Evaluation

  • Pro: New mobile apps don't require a new major version of the API, they just have to send the new title to the old API.
  • Con: Old mobile apps cannot change the title anymore => Confusing for users and not full functionality

Option 3: Change existing API with magic

Behavior definition

  • If the content is changed through the API and if oldTitle == getTitleFromContent(oldContent), then the title is changed using the old algorithm.
  • If the title is changed through the API, the note's title is untouched (current behavior, title is ignored).

Evaluation

  • Pro: If the note's title wasn't changed manually (e.g. using the web app), the behavior of old mobile apps doesn't change (i.e. the title is generated from content). If the note's title was changed manually (e.g. using the web app), changes of the note's content using an old mobile app don't overwrite the manually set title.
  • Cons: If a note's title was changed manually (e.g. using the web app), the old mobile app can't change the title anymore.
  • We have to provide a new major version of the API for new mobile apps, which set the title explicitly.

API v1

We require a new major version of the API for options 1 and 3. This API will have the following behavior definition (for general thought about a new major version of the API please see #311):

Behavior definition

  • If the content is changed through the API, the title is untouched.
  • If the title is changed through the API, the note's title is set to the new title.

Conclusion

I hope, my thoughts are understandable (if not, please ask). I tend to prefer option 1, since it's the clearest one; straight forward, no behavior change for old mobile apps; least limitations. But I'm looking forward to hear other opinions!

@stefan-niedermann
Copy link
Member

Thanks for the excellent description.

I haven't had time to think about it in detail, but i will comment as soon as i know what i would prefer.

I assume that you will keep API v1 still alive, so we can wait a few weeks or months to make sure that most people updated the server app?

To be honest i don't want to deal with multiple parallel supported API versions.

Also i have some big other changes for the Android Notes app (new Markdown editor, Password protection and so on) and currently little spare time to manage all those changes in parallel.

@korelstar korelstar added this to the 3.2.0 milestone Feb 28, 2020
@jancborchardt
Copy link
Member

Just to clarify – is note creation still simple and like right now? As per my comment at #190 (comment)

We could still handle it exactly like now on note creation: By default the first line you type is used as filename.

But once created, when you change the first line, the note title is not changed. For that you have to use the 3-dot menu in the note list to »Change filename«.

@korelstar
Copy link
Member Author

@jancborchardt Yes, I've implemented this already (and just pushed it to this branch).

@stefan-niedermann
My goal is to support old versions as long as possible. The plan is to support the API v0.2 for at least two years. Due to that, client apps should have enough time to wait until a reasonable amount of users have installed a server app with the new API, and to implement the new API on client-side.

I think it's sufficient if clients implement only one API version at the same time. But in order to show meaningful error messages, they should check the capabilities API (at least on the first connect and if an error occurs)

@stefan-niedermann
Copy link
Member

I also think option 1 is the way to go because an API (as nice it would be) should be reliable and stable and must not change its behavior like in option 2 or 3.

cc @phedlund from iOS client

@korelstar korelstar mentioned this pull request Mar 1, 2020
@korelstar korelstar linked an issue Mar 1, 2020 that may be closed by this pull request
@korelstar korelstar modified the milestones: 3.2.0, 3.3.0 Mar 13, 2020
@korelstar korelstar marked this pull request as ready for review April 6, 2020 18:54
@korelstar
Copy link
Member Author

Since nobody entered an objection, I'm merging this, now. I will add the docs in a separate PR: #491

@korelstar korelstar merged commit ffe6a02 into master Apr 13, 2020
@korelstar korelstar deleted the edit-title branch April 13, 2020 06:49
@stefan-niedermann
Copy link
Member

@korelstar just a question about discoverability - is the supported API version exposed by the capabilities-endpoint or how can i know what the server supports?

@stefan-niedermann
Copy link
Member

Ah, just saw the PR here with the documentation: https://github.com/nextcloud/notes/pull/491/files

Since which version of the server app do the capabilities-endpoint and the response header work? Were they always there or are the also new?

@korelstar
Copy link
Member Author

That will be also new in Notes v3.3. Hence, you should cope with the case, that it doesn't exist. :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: app navigation Related to the app navigation feature: backend Related to the PHP backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Versioning of the API Use file name instead of first line as title [$20]
3 participants