OCA
[14.0][ADD] dms_version: DMS Version Control
Superseed: #177 DMS Version Control
I have made some changes from an old PR, that are designed by @marylla
Here is my description of the design to explain what we wanted to achieve:
Configuration
- allow versioning on storage level (only for type "database" and "filestore" -> NOT "attachment" currently)
- allow versioning on directory level
- allow versioning on file level
- allow changing the versioning setting later only for people with special group + track change in chatter of dms file
DMS Files
- link the "origin" file when creating a new version
- If a file is newly created, origin is self
- If a version is created for a file, origin is copied (so that the entire history of a file knows which file was the origin)
- use active-boolean to mark newest version
- There is only one active file for all files with the same origin (unique contraint).
- Files with active = False are readonly to avoid creating new versions from archived files.
- link the "parent" file when creating a new version
- If a file is newly created, parent is False.
- If a version is created for a file, parent is the "old" file.
- count the number of versions for files with the same origin_id (unique contraint for version number and origin)
- only show files with active=True in tree and kanban views
- track changes of almost all fields of a file
- add a smartbutton in file form view to show all versions with the same orgin (attention: show archived files)
- add icon in kanban view if a file is not the origin (to mark a file as modified)
- color file in yellow in tree view that is not the origin (to mark a file as modified)
- add a filter to show all modified files (parent != False)
- add a filter to show all not-modified files (parent = False)
- add a filter to show all archived files (active=False)
- do not allow to use the usual action "Unarchive" - instead use the restore button in archived files
Versioning
- change the content field of a file to create a new version
- do not allow to change the content if the file is active=False
- The old file is the parent of the new file.
- The old file is now active=False.
- The new file is active=True.
- A chatter message is created in the new file to know from which file it was created.
- A chatter message is created in the parent file to know which file was generated from it.
- A chatter message is created in the origin file (if origin != parent) to see the entire history of all versions in the origin file.
Restoring
- click a button in form or tree view to restore an archived file
- The old archived file is now active=True.
- If an active=True file existed with the same origin it now is active=False.
- A chatter message is created in the previously archived and now restored file.
- A chatter message is created in the previously active and now archived file (if one existed).
- A chatter message is created in the origin file (if origin != restored file and origin != previous active file).
I will test as soon as @yweng8111 fixed the commit history and the other PR errors.
@pedrobaeza I've already solved that problem in commit history. But there are still some errors in pre-commit, that I don't get locally. Did I do something wrong?
You need to execute the copier update to get a newest template fixing the pre-commit problem. Please do it in a separate PR, and once merged, you can rebase here.
You need to execute the copier update to get a newest template fixing the pre-commit problem. Please do it in a separate PR, and once merged, you can rebase here.
Thanks for the Tip. I've just used the default setting for "copier update" is this correct?
$ copier update --trust
Updating to template version 1.19.2
🎤 Which Odoo version are we deploying in this branch?
14.0
🎤 What's the organization slug? If you are creating https://github.com/OCA/server-tools, then write "OCA" here.
OCA
🎤 Tell me the Organization name. It's supposed to be human-readable. It will be used in the author key of the __manifest__ files.
Odoo Community Association (OCA)
🎤 What's the repo slug? If you are creating https://github.com/OCA/server-tools, then write "server-tools" here.
dms
🎤 Tell me the project name. It's supposed to be human-readable. So, server-tools project could be named like "Tools for server environment(s)"
Document Management System modules for Odoo
🎤 Tell me the project website. It will be used in the website key of the __manifest__ files.
https://github.com/OCA/dms
🎤 Please write a short description about what this repo is about.
OCA modules for DMS
🎤 Which CI system to use ?
GitHub
🎤 Which Odoo flavor should be used for CI tests ?
Both
🎤 Use pyproject.toml instead of setup.py.
No
🎤 Generate requirements.txt from addons manifests and optional overrides in setup.py files.
Yes
🎤 Use ruff and ruff-format instead of flake8, autoflake, pyupgrade, isort, black.
No
🎤 Are there in this repo modules that don't get along with their friends? If so, list them here (YAML format) and they will be tested in separate jobs.
Beware, if rebel modules should stay separated in groups, you should join them with commas, which could be misinterpreted by YAML.
Example: ["rebel_module_1,rebel_module_2", even_more_rebel_module]
[]
🎤 Do you need to install wkhtmltopdf? Usually only needed if you're going to test PDF report generation.
No
🎤 GitHub action checks the minimum development status?
Yes
🎤 GitHub action checks the manifest license?
Yes
🎤 GitHub action runs codecov/codecov-action?
Yes
🎤 GitHub action updates .pot files?
Yes
🎤 Create GitHub 'stale' action?
Yes
🎤 Any extra environment variables to inject into the CI tests
Example: {"KEY": "VALUE"}
{}
🎤 convert_readme_fragments_to_markdown (bool)
No
The copier template already exists. You just need to update it (copier -f --trust).
@yweng8111 I don't remember if we talked about this before, but... the "has versioning" is not automatically set in directories if the storage type is "attachment". And even if I change it manually, the newly created subdirectories and their files won't get the "has versioning" setting.
Did we forget this or was there a reason why we didn't implement it?
Did we forget this or was there a reason why we didn't implement it?
No we don't forget this. I have commented it in the estimation. I will send you the Link later.
Okay, for all people interested in dms_version module... it is only implemented for storage type "database" and "filestore"... because the attachment in attachment-storages is not saved in dms.file but in ir.attachment. We would have to make complex changes and wanted to start with the simpler cases...
but we're happy if someone has an idea how we can develop this... @victoralmau maybe you want to intervene here?
Okay, for all people interested in dms_version module... it is only implemented for storage type "database" and "filestore"... because the attachment in attachment-storages is not saved in dms.file but in ir.attachment. We would have to make complex changes and wanted to start with the simpler cases...
but we're happy if someone has an idea how we can develop this... @victoralmau maybe you want to intervene here?
In the initial approach I thought in https://github.com/OCA/dms/pull/177 it was only thought for storage type database or filestore and I think it is the most reasonable. Considering the use case of attachments would be out of the initial scope (in my opinion) since you would have to archive attachments (something that probably has a lot of side effects).
PS: It is very difficult for me to know the changes that have been done since https://github.com/OCA/dms/pull/177 because they have not kept the initial commits and added an extra one with the corresponding changes.
If you really wanted to implement versioning on attachments, it would be possible to have a record "dms.file.history" and move the attachments to it, but that would require some work and also require more configuration with the attachment storages.
If think it's fine to skip versioning on attachment. It should just be written upfront in the module documentation, README, etc. There's just a slight problem with the current implementation: you can set "versioning to true" on a DMS Storage of type attachment, and similarly on a directory on such a storage, which means risks of misconfiguration and confusing issues. I think there should be a python constraint to avoid such risks (see 952830981654f25966b93c3b08beb534fe8d24eb).
I've tested the module in v16 in conjunction with https://github.com/OCA/dms/pull/275 There needs to be an additional 1 line glue module to make both work correctly by the way (the storage_path needs to be reset during the revision copy).
The main issue I had debugging this is that the stack of write is insanely long, due to the various hacks that both base_revision and dms perform. To manage that I think there is one patch in dms that really improves readability and performance (77c4efa8a4e4965386b3843975745c5a3d90afac and c899326554e3770fbf51733caa634677e4599d33).
Assuming create_revision works correctly, 8b6999bacd9620f93f875337dd832bb5e3e7d1c7 skip the check so that archiving the current revision is taken care of in base_revision, reducing further the number of calls to write. I think this commit is more debatable, but what it changes is already kind of a hack, and it's much better in terms of performance.
There was a small bug in that you could not unarchive a file that had no revision, fixed in dfdfbbcf838bf346638bc2a5b0d1f4dd33ab887a.
Last I changed the create to work in batch in 0eb2226c9a8e1ce0656b1b56fa58d3cee82b2628.
I'm wondering: can't we remove altogether the writing of origin_id in the create case? I don't see its purpose. However I'm not familiar with base_revision so there may be a good reason.
Sorry for the wall of text, check https://github.com/lambdao-dev/dms/commits/14.0-add-dms_version-len/ for the code :-)
@yweng8111 Can you please have a look at what @len-foss and @victoralmau wrote.
- [ ] We should write in the Readme that it is not working for attachment storages.
- [ ] We should add the constraint that you cannot set / see the has_versioning boolean in attachment storages, its directories and files - so that the user won't be confused.
- [ ] Can you check if you can keep the initial commits from the original pull request as @victoralmau suggested?
- [ ] Do we need the origin_id at the first origin dms file for some filters or domains? Can we skip setting it at the creation process or do we need it?
- [ ] Can you also check the archiving / restoring process in base_revision and if we can use it for the current version and dms files without versions?
I am not very familiar with the technical background of what @len-foss described regarding the "stack of write is insanely long". Can you please check this and get it somehow integrated in your code changes? He mentioned a patch that maybe does the trick.
- [ v ] We should write in the Readme that it is not working for attachment storages.
- [ v ] We should add the constraint that you cannot set / see the has_versioning boolean in attachment storages, its directories and files - so that the user won't be confused.
- [ v ] Can you check if you can keep the initial commits from the original pull request as @victoralmau suggested?
- [ v ] Do we need the origin_id at the first origin dms file for some filters or domains? Can we skip setting it at the creation process or do we need it?
I just follow your design and we need it also for the one2many field "all_revision_ids"
- [ ? ] Can you also check the archiving / restoring process in base_revision and if we can use it for the current version and dms files without versions?
I don't understand what you mean. Can you show me later? @marylla
I am not very familiar with the technical background of what @len-foss described regarding the "stack of write is insanely long". Can you please check this and get it somehow integrated in your code changes? He mentioned a patch that maybe does the trick.
I have integrated some code from @len-foss, Thanks for the tips. But I don't want to do some changes like the write() function in core module "dms" in this PR.
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}