feat(ui): Add rename/delete category actions in the sidebar#1732
feat(ui): Add rename/delete category actions in the sidebar#1732aronovgj wants to merge 1 commit intonextcloud:mainfrom
Conversation
4407016 to
c147030
Compare
|
@marcoambrosini In my opinion it does make sense and makes usibility easier around using notes app. What would you say? |
98e04a0 to
63014ec
Compare
|
Ping @marcoambrosini |
There was a problem hiding this comment.
Hi @aronovgj and thank you for your contribution!
A few recommendations from my side regarding this implementation.
Categories in notes are simply directories in Nextclouds file storage under the notes root directory. This means renaming a category is basically just renaming a directory, and deleting a category is just deleting a directory, one operation via the OCP\Files API. The oc_notes_meta rows are cleaned on the next sync.
What this current implementation does is loop through every note on the client-side and send one HTTP request per note to move or delete each file individually, which for large categories means a high amount of sequential requests. Beyond performance, this is also not atomic as if the network drops or the browser is closed mid-loop, you end up with partially renamed or partially deleted data.
To fix this properly, we should add two new backend endpoints to the NotesApiController, a PATCH for renaming a category and a DELETE for deleting one and implement the logic for it in NotesService calling OCP\Files move() and delete().
Once those endpoints exist, the frontend simplifies to a single request per operation, and UI state should only be updated after a confirmed successful response.
One smaller thing: getDraggedNoteId is duplicated, maybe they could be extracted into a shared utility function.
4e10d57 to
e14a959
Compare
There was a problem hiding this comment.
@aronovgj thank you for making the changes, it looks good to me now! 🚀
Would it be possible for you to add playwright e2e tests for the new renameCategory and the deleteCategory actions?
There is also a conflict that needs to be resolved.
Also please amend and sign off your commit to fix the DCO failure.
4a42499 to
ef73d1e
Compare
(cherry picked from commit e14a959) Signed-off-by: Grigorij Aronov <aronovgj@gmx.net>
ef73d1e to
55265af
Compare
|
fixed linting issue |
|
holding off on merging since playwright test is failing. Weird that the failure isn't properly reported in CI |
This test seems flaky and returns a page not found on the first try. Maybe CI is green because it passed within the allowed retries? Maybe it is running the first test too early when the notes app is not yet fully setup? This seems unrelated to this PR though and also is silently failing for a while now. |
This introduces a per‑category actions menu (three‑dot button) in the left navigation.
Features:
Closes #1397
Closes #1120
Probably closes #1669
https://github.com/aronovgj/notes/blob/98e04a03435a384c557e6004a6c9c1cae6a672e4/src/components/NoteItem.vue#L299
This is how it would look: