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.

Sign up for GitHub

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

Jump to bottom

[IMP] stock_picking_partner_note check note already in use #1541

Merged
merged 1 commit into from
Mar 26, 2024
Merged

[IMP] stock_picking_partner_note check note already in use #1541

merged 1 commit into from
Mar 26, 2024

Conversation

santostelmo
Copy link
Contributor

@santostelmo santostelmo commented Mar 14, 2024 •
edited
Loading

Add setting option to validate if picking note is already used in other partners, preventing it to be modified or deleted,

@simahawk simahawk mentioned this pull request Mar 15, 2024
Merged
@rousseldenis rousseldenis added this to the 16.0 milestone Mar 20, 2024
rousseldenis
rousseldenis requested changes Mar 20, 2024
View reviewed changes
Copy link
Contributor

@rousseldenis rousseldenis 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.

@santostelmo could you also add some tests ?

stock_picking_partner_note/models/res_company.py Show resolved Hide resolved
stock_picking_partner_note/models/res_config_settings.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking_note.py Outdated Show resolved Hide resolved
@santostelmo santostelmo force-pushed the 16.0-imp-stock_picking_partner_note-check_note_already_in_use branch 2 times, most recently from 5348f1a to d61cc94 Compare March 20, 2024 11:03
@santostelmo
Copy link
Contributor Author

@rousseldenis Thanks for the review.
I updated the code with your change requests

@santostelmo
Copy link
Contributor Author

Code review.

@santostelmo could you also add some tests ?

Unit test added

@santostelmo santostelmo force-pushed the 16.0-imp-stock_picking_partner_note-check_note_already_in_use branch from d61cc94 to e801395 Compare March 20, 2024 11:16
@cyrilmanuel
Copy link

@rousseldenis Little ping, is it good for you ?

cyrilmanuel
cyrilmanuel approved these changes Mar 26, 2024
View reviewed changes
Copy link

@cyrilmanuel cyrilmanuel left a comment

Choose a reason for hiding this comment

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

LGTM

rousseldenis
rousseldenis approved these changes Mar 26, 2024
View reviewed changes
Copy link
Contributor

@rousseldenis rousseldenis 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

@rousseldenis
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1541-by-rousseldenis-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit c3d8ef0 into OCA:16.0 Mar 26, 2024
7 of 9 checks passed
@OCA-git-bot
Copy link
Contributor

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

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

@rousseldenis rousseldenis rousseldenis approved these changes

@cyrilmanuel cyrilmanuel cyrilmanuel approved these changes

Assignees
No one assigned
Labels
approved enhancement merged 🎉 needs review
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
4 participants
@santostelmo @cyrilmanuel @rousseldenis @OCA-git-bot