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][ADD] stock_picking_partner_note #1496

Merged

Conversation

santostelmo
Copy link
Contributor

@santostelmo santostelmo commented Feb 5, 2024

Allow to record a message for the person in charge of order preparation at the level of the customer,
then have it as a note on the picking transfer.

Example: Max pallet height 1m20

Note: This will allow to remove from shopfloor the shopfloor.packing.info and replace it by this more generic module.

@santostelmo santostelmo force-pushed the 16.0-add-stock_picking_partner_note branch from 6c9a45d to 9931852 Compare February 5, 2024 13:01
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

LG. Just a few small remarks

stock_picking_partner_note/models/stock_picking.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking.py Outdated Show resolved Hide resolved
stock_picking_partner_note/views/res_partner.xml Outdated Show resolved Hide resolved
<field name="name" />
<field name="note_type_id" />
<field name="active" />
</group>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be useful to see the list of partners having this note

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but use a smart btn rather than a direct listing of m2m

Copy link
Contributor

Choose a reason for hiding this comment

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

nooooo 😨
Keop it simple, just list them, there is nothing on that view. Easier to visualize and change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A smart button wouldn't work because stock_picking_note_ids because is a M2M and not O2M.
With a button you would be able to create a new note BUT detached from the partner.
I keep the simple version showing the notes directly in the form

Copy link
Contributor

Choose a reason for hiding this comment

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

A smart button wouldn't work because stock_picking_note_ids because is a M2M and not O2M.
With a button you would be able to create a new note BUT detached from the partner.

This is wrong. You will have an action to handle the btn and you can search for the records you need. No matter if o2m or m2m.
Regarding creation, that's wrong too: you simply have to set the default partner in the context.

I keep the simple version showing the notes directly in the form

Ok, but that's about time and not about possibilities 😉

stock_picking_partner_note/models/stock_picking_note.py Outdated Show resolved Hide resolved
stock_picking_partner_note/__manifest__.py Outdated Show resolved Hide resolved
stock_picking_partner_note/__manifest__.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking.py Outdated Show resolved Hide resolved
stock_picking_partner_note/models/stock_picking_type.py Outdated Show resolved Hide resolved
<field name="name" />
<field name="note_type_id" />
<field name="active" />
</group>
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but use a smart btn rather than a direct listing of m2m

@santostelmo santostelmo force-pushed the 16.0-add-stock_picking_partner_note branch 2 times, most recently from a380262 to 8a2e5fe Compare February 6, 2024 10:31
@santostelmo
Copy link
Contributor Author

@jbaudoux @simahawk I updated the code . Can you please review again?

<field name="name" />
<field name="note_type_id" />
<field name="active" />
</group>
Copy link
Contributor

Choose a reason for hiding this comment

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

A smart button wouldn't work because stock_picking_note_ids because is a M2M and not O2M.
With a button you would be able to create a new note BUT detached from the partner.

This is wrong. You will have an action to handle the btn and you can search for the records you need. No matter if o2m or m2m.
Regarding creation, that's wrong too: you simply have to set the default partner in the context.

I keep the simple version showing the notes directly in the form

Ok, but that's about time and not about possibilities 😉

@santostelmo santostelmo force-pushed the 16.0-add-stock_picking_partner_note branch from 8a2e5fe to 1495d90 Compare February 6, 2024 12:51
@santostelmo santostelmo force-pushed the 16.0-add-stock_picking_partner_note branch from 1495d90 to 355a504 Compare February 6, 2024 12:53
@simahawk simahawk changed the title [16.0][ADD] stock picking partner note [16.0][ADD] stock_picking_partner_note Feb 6, 2024
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. IMHO, quite tough to manage for picking users if several per picking types. Fortunately, it's not one per customer 😅

@jbaudoux
Copy link
Contributor

jbaudoux commented Feb 8, 2024

Note: Purpose is to remove from shopfloor the shopfloor.packing.info and replace it by this more generic module.

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

@simahawk
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1496-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7913484 into OCA:16.0 Feb 13, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 57860b2. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants