[19.0][MIG] dms: Migration to 19.0#475
Conversation
|
Thanks for the contribution. Please preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-19.0. If the jump is between several versions, you have to modify the source branch in the main command to accommodate it to this circumstance. |
43752f4 to
660b3c9
Compare
|
Thanks for the review @pedrobaeza — sorry I missed the technical method on the first round. I've now:
CI is re-running now. |
4affd39 to
724ff8b
Compare
|
Friendly ping @pedrobaeza @victoralmau @eLBati @etobella |
etobella
left a comment
There was a problem hiding this comment.
Using AI for coding is fine (I personally think it is a nice tool to improve your results), but you need to understand the code you're submitting.
In this PR specifically:
- Some functionalities were removed
- A lot of unnecessary diff was introduced
These are basic mistakes that would be caught by simply reviewing the changes your AI assistant supplied before opening a PR.
That is also a license risk: AI tools are trained on code with various licenses (GPL, MIT, Apache…). If you can't read the output, you can't detect contamination, missing attribution, or patent-encumbered patterns. Evenmore, you cannot claim ownership and copyright (completely necessary in an OpenSource community). "The AI wrote it" is not a legal defense.
Review what you ship, always.
24e9535 to
7128196
Compare
Follow-up to OCA#475 phase 1 (the minimal-viable 19.0 [MIG]). This PR layers the OWL/UX modernisations that were held back to keep the base MIG reviewer-friendly. Opens on the ledoent fork against the phase 1 branch so reviewers can preview the delta on top of OCA#475 before it lands. - **Kanban buttons**: drop the classic `dms.KanbanButtons` template (mobile-Scan + desktop-Upload + hidden file input) and rewire `buttonTemplate` to the modern `dms.FileKanbanView.Buttons` (single Upload button, `t-ref="uploadFileInput"` hook pattern, modern `onFileInputChange` handler). Mobile-Scan UX is intentionally dropped — the file drop-zone (already wired via `createFileDropZoneExtension()`) covers the mobile-upload flow. - **Breadcrumb modernisation**: replace `path_owl.xml`'s inline-style `<a oe_form_uri>` + `<span style="display: inline">` pattern with Odoo 19's `<ol class="o_breadcrumb breadcrumb"><li class="breadcrumb-item">` Bootstrap idiom. Final segment uses `class="breadcrumb-item active"` with `aria-current="page"` per Bootstrap a11y guidance. - **Restore `filter_domain` on `dms_category.xml`** search view — `filter_domain="['|', ('name', 'ilike', self), ('parent_id', 'child_of', raw_value)]"` was removed in OCA#475 and re-introduces the 18.0 UX of filtering by parent-category subtree when typing a parent's name. The bare 19.0 default `ilike` on name lost that behaviour. (Tag search left as default; 18.0's `filter_domain` was equivalent to 19.0's default for that field.) Deliberately held for follow-up rounds (not in this PR): - **JS dead-code audit** of `attachment_image.esm.js` / `attachment_viewer_viewable.esm.js`: grep across odoo/odoo:19.0 + OCA org confirmed only our own files reference the patches, and core 19.0 `LinkPreview` has no native `imageUrl` getter — verification inconclusive on whether the patches are still load-bearing. Keeping both files until a runtime regression test or a maintainer's explicit "core handles it now" signal. - **Coverage closure**: add `coverage.py` integration + tests for the new 19.0 code paths (`_search()` override on `dms_security_mixin`; `_search_starred` operator normalisation in `directory`). Separable. - **Code-style sweep** (whitespace + indentation): not bundled here to keep the diff focused on user-visible UX changes. Signed-off-by: Daniel Kendall <dkendall@ledoweb.com>
|
Thanks for the partial response, but several inline comments were left unanswered... More importantly, your own Phase 2 PR restores things that were removed in this PR (kanban buttons, breadcrumb, filter_domain). That is the clearest possible confirmation that functionality was silently dropped and went unnoticed — exactly the problem I feared in the review. Please address all inline comments before requesting another review, and make sure this PR is complete and self-consistent before opening follow-ups to fix what it broke. Vibecoding without knowing what you are doing is a problem for the code quality, for licensing, and for the reviewers who have to clean up after it. So, can you justify all the actions done? |
|
@etobella certainly, thanks for the review! Apologies for the AI-review failure on the first round — I do try to keep it reined in. Your feedback is the best teacher; sometimes between all the notes and skill updates I wonder, but I believe next year the careful conventions will pay back in speed without losing quality. this PR feedback is delays as I fix my internal runboat. for the reasoning on the button change I wanted to get the phase 2 IMP up. will try and get that later today. Force-pushed
Final scope: 31 files / +190-122 (down from 36 / +430-431, ~64% churn reduction). All 7 CI checks green. A follow-up PR is opened as a draft preview on the ledoent fork: ledoent#2 — base = this PR's branch, so the diff visible there is only the phase-2 delta. Contents:
Deferred to a later round in that PR: JS dead-code audit (verification was inconclusive; core 19.0 Will promote ledoent#2 to an OCA upstream draft PR once this one lands |
Follow-up to OCA#475 phase 1 (the minimal-viable 19.0 [MIG]). Opens on the ledoent fork against the phase 1 branch so reviewers can preview the delta on top of OCA#475 before it lands. - **Breadcrumb modernisation**: replace `path_owl.xml`'s inline-style `<a oe_form_uri>` + `<span style="display: inline">` pattern with Odoo 19's `<ol class="o_breadcrumb breadcrumb"><li class="breadcrumb-item">` Bootstrap idiom. Final segment uses `class="breadcrumb-item active"` with `aria-current="page"` per Bootstrap a11y guidance. - **Restore `filter_domain` on `dms_category.xml`** search view — `filter_domain="['|', ('name', 'ilike', self), ('parent_id', 'child_of', raw_value)]"` was removed in OCA#475 and re-introduces the 18.0 UX of filtering by parent category subtree when typing a parent's name. The bare 19.0 default `ilike` on name lost that behaviour. (Tag search left as default — 18.0's filter was equivalent to 19.0's default for that field.) Originally this PR also rewired the kanban `buttonTemplate` from `dms.KanbanButtons` to the orphan `dms.FileKanbanView.Buttons` template, but that template uses invalid OWL inheritance syntax (`<div role="toolbar" position="inside">` instead of `<xpath ...>`) AND references controller hooks (`uploadFileInputRef`, `onFileInputChange`) that don't exist on the controller. Reverted; phase 1's classic `dms.KanbanButtons` template stays in place (it's now self-contained without `t-inherit` since 19.0's `web.KanbanView.Buttons` is empty). Deferred to a later round: JS dead-code audit (inconclusive — core 19.0 `LinkPreview` has no native `imageUrl` getter, so our patches may still be load-bearing), coverage closure, and a proper kanban UX modernisation once `dms.FileKanbanView.Buttons` is fixed or replaced. Signed-off-by: Daniel Kendall <dkendall@ledoweb.com>
|
Even this response seems AI-generated. Please, stop doing this, it is only making things worse @dnplkndll We don't have yet an AI Policy, but I hope we will have one soon. meanwhile, you can see one example of what should be and shouldn't be allowed to be done with AI by checking other community policies... |
|
@etobella looking over the policy that matched up with the pycore team podcast from maintainers I just mentioned to @JordiBForgeFlow . I get it. the social part of this think is important and maintainers are not here to fix slop. what I actually do is cli 6 sessions and always try my best to review and update. clarify fix everything I do. I understand I own it and I do understand the value of being the human. but I make mistakes. miss things, like I have in the past in many contributions I made before llms. but I can assure you I am present trying to be an enguaged policy following participant in the community. I continue to have much more grandiuos goals than my time and budget and am included to learn and grow with all the feedback I can get. I will comment on your inlines and to be honest that last one I was like wth, but I did not then have time / remember to open that file to verify. what is hard for me at this point is going through. then working on something else and a change happend in the wrong window I miss. I get it this is your time and will try to do better at making sure nothing hits the upstream repo intill I am ready to own it. |
… error Changes done: - Improve _dms_operations() method to do nothing if the model is not one of those used in dms.storage. - Avoid access error with .browse() if we access a non-existing record (for example, deleted by database). TT50231 (cherry picked from commit 13d32a8)
UserWarning: dms.file: inconsistent 'store' for computed fields, accessing migration may recompute and update require_migration. Use distinct compute methods for stored and non-stored fields [BOT] post-merge updates (cherry picked from commit 2d418f8)
Translated using Weblate (Persian) Currently translated at 33.3% (118 of 354 strings) Translation: dms-17.0/dms-17.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-17-0/dms-17-0-dms/fa/ Translated using Weblate (Persian) Currently translated at 96.6% (342 of 354 strings) Translation: dms-17.0/dms-17.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-17-0/dms-17-0-dms/fa/ (cherry picked from commit 5e89fa6)
It was missing a placeholder. (cherry picked from commit e9ebf77)
Currently translated at 100.0% (354 of 354 strings) Translation: dms-17.0/dms-17.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-17-0/dms-17-0-dms/it/ (cherry picked from commit 5e3b551)
(cherry picked from commit 8361244)
Add the mk_file_kanban_view class to the custom kanban view so that it is displayed correctly according to the styles set for that class (mk_file_kanban_view). TT50055 (cherry picked from commit 32fe05a)
…llowed to create TT50055 [BOT] post-merge updates [BOT] post-merge updates (cherry picked from commit a3ebf26)
[BOT] post-merge updates (cherry picked from commit 8667e21)
(cherry picked from commit 860607c)
Co-authored-by: kobros-tech TT55505 (cherry picked from commit 1f08ada)
(cherry picked from commit 8783a1a)
(cherry picked from commit ba27aa3)
Currently translated at 100.0% (354 of 354 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/it/ (cherry picked from commit b82c5ad)
Currently translated at 100.0% (359 of 359 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/it/ (cherry picked from commit 547eaaf)
Currently translated at 100.0% (359 of 359 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/it/ (cherry picked from commit 0a7e660)
(cherry picked from commit 34b922d)
… is applied correctly. Before these changes, the domain was never applied to search by the parent, so all subfolders were being displayed instead of only the direct children of the selected one. After making the changes, only a single level of folders is shown, without displaying all descendant folders. (cherry picked from commit 12c54a9)
Currently translated at 89.6% (322 of 359 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/es/ (cherry picked from commit 687ec9c)
The _check_access_dms_record method uses self.search(domain) to verify write permissions. However, search() applies active_test=True by default, which excludes archived records (active=False). This means when a user tries to unarchive a file via toggle_active(), the write permission check cannot find the archived record in the search results, causing an AccessError even when the user has full write permissions through DMS access groups. Fix: add active_test=False context to include archived records in the permission check search. (cherry picked from commit 86cc746)
(cherry picked from commit e5f4b46)
Currently translated at 100.0% (359 of 359 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/ar/ (cherry picked from commit 9605b5b)
Currently translated at 99.7% (358 of 359 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/es/ (cherry picked from commit 69f82dc)
Currently translated at 100.0% (358 of 358 strings) Translation: dms-18.0/dms-18.0-dms Translate-URL: https://translation.odoo-community.org/projects/dms-18-0/dms-18-0-dms/it/ (cherry picked from commit b5c4821)
Port of
dmsfrom 18.0 to 19.0