Skip to content

[FIX] The duplicated name check compares root Directories with non-root Directories - #526

Merged
OCA-git-bot merged 1 commit into
OCA:18.0from
PyTech-SRL:18.0-fix-dms-storage_root_directories
Oct 19, 2026
Merged

OCA-git-bot merged 1 commit into
OCA:18.0from
PyTech-SRL:18.0-fix-dms-storage_root_directories

Conversation

@SirPyTech

Copy link
Copy Markdown
Contributor

Steps:

  1. Inside a directory, create a directory "Test"
  2. Create a root directory "Test"

Current:
WARNING devel odoo.http: A directory with the same name already exists.

Expected:
The directory can be created.

Additional Notes:
The root cause is that the storage's root directories are all the storage directories and the commit focuses on that.

Comment thread dms/models/storage.py Outdated
@SirPyTech
SirPyTech requested a review from victoralmau October 15, 2026 08:45
Comment thread dms/models/storage.py Outdated
@SirPyTech
SirPyTech force-pushed the 18.0-fix-dms-storage_root_directories branch from 7243818 to a43982e Compare October 15, 2026 08:55
@SirPyTech
SirPyTech requested a review from victoralmau October 15, 2026 08:57

@HekkiMelody HekkiMelody left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review, LGTM

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza pedrobaeza added this to the 18.0 milestone Oct 19, 2026

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests is not adding value, as it doesn't test a flow, but an ORM feature of adding a domain to a o2m field, so better to just not add it.

Comment thread dms/models/storage.py Outdated
@SirPyTech
SirPyTech force-pushed the 18.0-fix-dms-storage_root_directories branch from a43982e to e62f63c Compare October 19, 2026 13:58

@SirPyTech SirPyTech left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests is not adding value, as it doesn't test a flow, but an ORM feature of adding a domain to a o2m field, so better to just not add it.

Thanks for the review!
Right we shouldn't test ORM features, the test made sense at the beginning of the PR when I was computing the value in a compute method.
I substituted the test with another one that checks the failing use case, please check if that's better now 🙏

Comment thread dms/models/storage.py Outdated
@SirPyTech
SirPyTech requested a review from pedrobaeza October 19, 2026 14:03

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-526-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit d8e4b5b into OCA:18.0 Oct 19, 2026
7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at c433e7f. Thanks a lot for contributing to OCA. ❤️

@SirPyTech

Copy link
Copy Markdown
Contributor Author

/ocabot merge patch

Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants