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

[16.0][FIX] document_page_tag: Use api.model_create_multi #491

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

anusriNPS
Copy link
Contributor

Using decorator api.model_create_multi for create method in document_page_tag model in order to avoid below warning:

WARNING devel odoo.api.create: The model odoo.addons.document_page_tag.models.document_page_tag is not overriding the create method in batch

@anusriNPS anusriNPS force-pushed the 16.0-document-tag-warn branch from 64a8591 to 4f09857 Compare July 16, 2024 08:21
@anusriNPS anusriNPS marked this pull request as draft July 16, 2024 08:25
@anusriNPS anusriNPS force-pushed the 16.0-document-tag-warn branch 2 times, most recently from d83332f to 6abb713 Compare July 16, 2024 09:21
@anusriNPS anusriNPS marked this pull request as ready for review July 16, 2024 09:23
Comment on lines 18 to 27
@api.model_create_multi
def create(self, vals_list):
"""Be nice when trying to create duplicates"""
existing = self.search([("name", "=ilike", vals["name"])], limit=1)
if existing:
return existing
return super().create(vals)
for vals in vals_list:
existing = self.search([("name", "=ilike", vals["name"])], limit=1)
if existing:
return existing
return super().create(vals_list)

Choose a reason for hiding this comment

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

Issue: When trying to create two tags at the same time, one that exists and one that doesn't, we return the one tag that does exist and don't create the one that doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes included to handle tag creation with one exist and not exist at the same time

@anusriNPS anusriNPS force-pushed the 16.0-document-tag-warn branch from 6abb713 to e6c793d Compare July 31, 2024 15:27
Copy link

@PicchiSeba PicchiSeba left a comment

Choose a reason for hiding this comment

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

Code Review: generally seems good. The only issue I have is with the variable names in the test (not your fault, they were already done like this).
We might consider converting them to snake_case format

@anusriNPS anusriNPS force-pushed the 16.0-document-tag-warn branch from e6c793d to 8a82c39 Compare August 12, 2024 06:13
@PicchiSeba
Copy link

watch out for the pre-commit error at line 68 in document_page_tag/tests/test_document_page_tag.py

   Using decorator api.model_create_multi for create
method in document_page_tag model
@anusriNPS anusriNPS force-pushed the 16.0-document-tag-warn branch from 8a82c39 to 9d749c8 Compare August 12, 2024 07:36
Copy link

@PicchiSeba PicchiSeba left a comment

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
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). 🤖

@max3903 max3903 added this to the 16.0 milestone Sep 21, 2024
@dreispt
Copy link
Member

dreispt commented Dec 27, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-491-by-dreispt-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3903d91 into OCA:16.0 Dec 27, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@PicchiSeba PicchiSeba deleted the 16.0-document-tag-warn branch January 8, 2025 11:27
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.

6 participants