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

Allow to name a version #35160

Merged
merged 19 commits into from
Jan 30, 2023
Merged

Allow to name a version #35160

merged 19 commits into from
Jan 30, 2023

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Nov 14, 2022

Done

  • Add web UI to name and delete a version
  • Create oc_files_versions table with id, file_id?, version_id, metadata metadata = JSON
  • Update version lookup logic to:
    1. Look into DB
    2. If nothing, look into FS, then load all info in DB
  • Add entry DB when creating new version
  • Delete DB entries on file deletion
  • Plug into DAV plugin
    • Support getting the version label
    • Support patching the version label
    • Expose endpoint to delete a version
  • Remove experimental flag on UI
  • Check concurrence issue when initializing entries for a file in the DB - added unique index to prevent duplicate <fileId, timestamp> tuples
  • Add version labeling capability
    • Add config switch to disable capability
    • Propagate capability to web UI
    • Limit UI functionality
  • Disable cleanup of named version in files_versions/lib/Storage.php
  • Create new interface for nameable version backend
  • Check that it does not explode with files_versions_S3 and groupfolders implementation as Carl raise some concerns

TODO

  • Write documentation about labeling and deleting version config switch

Follow up

  • Use COPY instead of MOVE when restoring version, so that it stays in the history (if not already the case)
  • Load versions sidebar tab when clicking on the tab
  • Save author of file creation and modification
  • Implement deletion and labeling in files_versions_S3 and groupfolders

Screenshots

Context Screenshot
List of version with the actions menu opened for the current version Screenshot from 2022-11-30 11-02-02
List of version with the actions menu opened for a previous version Screenshot from 2022-11-30 11-02-08
Name editor when the version do have a name Screenshot from 2022-11-16 14-52-17
Name editor when the version does not have a name Screenshot from 2022-11-16 14-52-08
Restore notification for un-named version Screenshot from 2022-11-30 11-03-46
Restore notification for named version Screenshot from 2022-11-30 11-03-30
Restore notification for initial version Screenshot from 2022-11-30 11-03-26

Screencast

Screencast.from.2022-11-21.17-36-10.webm

Need #34769

@artonge artonge added the 2. developing Work in progress label Nov 14, 2022
@artonge artonge added this to the Nextcloud 26 milestone Nov 14, 2022
@artonge artonge self-assigned this Nov 14, 2022
@tcitworld
Copy link
Member

tcitworld commented Nov 14, 2022

Checkout S3 [...] implementation as Carl raise some concerns

Yeah, since automatic expiration of versions is handled by the S3 platform itself, oc_files_versions will reference non-existing versions. Maybe there's a way to set a flag on objects on S3's side to avoid the expiration for the named versions?

@juliusknorr
Copy link
Member

Checkout S3 and group folders implementation as Carl raise some concerns

cc @mejo- as I remember there also was a groupfolder-like version backend in collectives.

@PVince81
Copy link
Member

regarding S3, this would only be handled in S3 if the app https://github.com/nextcloud/files_versions_s3
idea: we could disable the permanent version feature when this app is enabled to avoid unwanted side effects

@PVince81
Copy link
Member

we will also need to figure out how to backfill old versions that existed before this feature
probably we just do it on-demand because scanning all files would take a while

on-demand would mean that if there are no entries in the table for a given file id, we check on disk and do a one-shot scan to populate the table for said file id

@artonge
Copy link
Contributor Author

artonge commented Nov 15, 2022

we will also need to figure out how to backfill old versions that existed before this feature

How about we keep the current logic and only use the table to store versions with a name ?
Then we just populate the versions with their name.

Something like:

versions = existingLogicToGetVersionsFor(fileId)
namedVersions = newLogicToGetNamedVersionsFor(fileId) // so we just add this line
versions = merge(version, namedVersions) // and this line
return versions

In practice:

https://github.com/nextcloud/server/pull/35160/files#diff-e50e572ada0663901e21009299b02a8627937be7a783ff080c4e29ef550a997aR75-R86

@artonge artonge force-pushed the artonge/feat/version_naming_ui branch 2 times, most recently from ef378f4 to 22b5f09 Compare November 16, 2022 14:29
@artonge
Copy link
Contributor Author

artonge commented Nov 16, 2022

After discussion, let's load all data about a version on demand, ie. when the user opens the sidebar or during expiration.
Need to check concurrence issue when initing entries for a file.

@artonge artonge force-pushed the artonge/feat/version_naming_backend branch from 7c49481 to ab9c02f Compare November 17, 2022 14:14
@artonge artonge force-pushed the artonge/feat/version_naming_ui branch from 22b5f09 to 29415e4 Compare November 17, 2022 15:39
@artonge artonge force-pushed the artonge/feat/version_naming_ui branch from 29415e4 to 551c83c Compare November 17, 2022 16:06
@artonge artonge force-pushed the artonge/feat/version_naming_backend branch from 0446371 to e9b97ed Compare November 17, 2022 16:12
@artonge artonge force-pushed the artonge/feat/version_naming_backend branch from e04f3e6 to 317ba46 Compare November 21, 2022 16:32
@artonge artonge force-pushed the artonge/feat/version_naming_backend branch from 655e2c4 to 37e3886 Compare November 22, 2022 17:53
@artonge artonge changed the base branch from artonge/feat/version_naming_ui to port/vue/files_version November 23, 2022 13:11
@artonge artonge mentioned this pull request Nov 23, 2022
@artonge
Copy link
Contributor Author

artonge commented Nov 23, 2022

Changed base to include #35080 as discussed

@artonge artonge changed the title Add backend for version naming Allow to name a version Nov 23, 2022
apps/files_versions/lib/Hooks.php Fixed Show fixed Hide fixed
apps/files_versions/lib/Hooks.php Fixed Show fixed Hide fixed
apps/files_versions/lib/Hooks.php Fixed Show fixed Hide fixed
@nextcloud-command nextcloud-command force-pushed the artonge/feat/version_naming_backend branch from a41af23 to 552f5b0 Compare January 26, 2023 10:12
@artonge artonge merged commit 2f30072 into master Jan 30, 2023
@artonge artonge deleted the artonge/feat/version_naming_backend branch January 30, 2023 09:40
@blizzz blizzz mentioned this pull request Feb 1, 2023
artonge added a commit to nextcloud/documentation that referenced this pull request Feb 7, 2023
artonge added a commit to nextcloud/documentation that referenced this pull request Feb 7, 2023
@DaphneMuller
Copy link

hello @artonge
Thank you so much for your work on this pull request! This ticket seems to have the tag 'pending documentation'.
Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@artonge
Copy link
Contributor Author

artonge commented Feb 21, 2023

Did the admin one, but forgot the user doc

Admin: nextcloud/documentation#9618
User: nextcloud/documentation#9649

@artonge artonge added enhancement feature: versions javascript php Pull requests that update Php code 3. to review Waiting for reviews 4. to release Ready to be released and/or waiting for tests to finish and removed pending documentation This pull request needs an associated documentation update 2. developing Work in progress 3. to review Waiting for reviews labels Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: versions javascript php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants