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

[16.0][ADD] sale_partner_company #580

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from
Open

[16.0][ADD] sale_partner_company #580

wants to merge 1 commit into from

Conversation

santostelmo
Copy link

@santostelmo santostelmo commented Jan 31, 2024 •
edited
Loading

Description

Allow to set the default selling company on a partner.

Configure

On the partner, choose the default company.
Create a sale.order with this partner, the sales company will be adapted from the partner.

@santostelmo santostelmo changed the title [ADD] sale_partner_company [16.0][ADD] sale_partner_company Jan 31, 2024
@santostelmo santostelmo force-pushed the 16.0-add-sale_partner_company branch 2 times, most recently from 7355262 to 70b8190 Compare January 31, 2024 13:58
@jbaudoux
Copy link

jbaudoux commented Feb 1, 2024

Doesn't really depends on #577

jbaudoux
jbaudoux suggested changes Feb 1, 2024
View reviewed changes
sale_partner_company/__manifest__.py Outdated Show resolved Hide resolved
sebalix
sebalix reviewed Feb 2, 2024
View reviewed changes
sale_partner_company/models/res_partner.py
class ResPartner(models.Model):
_inherit = "res.partner"

sale_company_id = fields.Many2one("res.company", string="Selling Company")
Copy link

Choose a reason for hiding this comment

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

Improvement: that would be interesting to group partners by selling company (search view addition), and so adding an index here.

Copy link
Author

Choose a reason for hiding this comment

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

Group by selling company it can be interesting indeed.

Regarding the index, from my understanding as we have a low cardinality on sale_company_id we'll not gain in performance.

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

@santostelmo santostelmo force-pushed the 16.0-add-sale_partner_company branch from 0d5ece9 to 79fc9a1 Compare March 4, 2024 08:14
@santostelmo
Copy link
Author

Pushed improvement to assign default sale company value based on default sale partner => https://github.com/OCA/multi-company/compare/0d5ece9251995ec39d435d6976871a630d4d8d8c..79fc9a1006fc2968cceb7d8208db32cd86a68df0

@cyrilmanuel
Copy link

hiii @pedrobaeza can you merge this PR please ?

@pedrobaeza pedrobaeza added this to the 16.0 milestone Mar 27, 2024
@pedrobaeza
Copy link
Member

Similar to #584, I think the name should be partner_selling_company

@jbaudoux
Copy link

Similar to #584, I think the name should be partner_selling_company

It depends on sale and modifies sales order behavior. It's not just a partner attribute. in my opinion, current name is correct

@cyrilmanuel
Copy link

Hello @pedrobaeza and @jbaudoux , Could you agree on this or on the changes to be made?

@pedrobaeza
Copy link
Member

I haven't blocked the PR, so you can go on if you want to.

@santostelmo santostelmo force-pushed the 16.0-add-sale_partner_company branch from 79fc9a1 to 1817ecc Compare June 28, 2024 06:21
@cyrilmanuel
Copy link

Hi @gurneyalex, could you merge this pr please :)

@gurneyalex
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-580-by-gurneyalex-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jul 8, 2024
@OCA-git-bot
Merge PR #580 into 16.0
351a735 
Signed-off-by gurneyalex
@OCA-git-bot
Copy link
Contributor

@gurneyalex your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-580-by-gurneyalex-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@santostelmo
Copy link
Author

@gurneyalex can you please help me to understand why this build is falling.
Also i see the following log in CI tests:
Could not fetch URL https://wheelhouse.odoo-community.org/oca-simple-and-pypi/odoo-test-helper/: There was a problem confirming the ssl certificate

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

@Ricardoalso Ricardoalso Ricardoalso left review comments

@sebalix sebalix sebalix left review comments

@gurneyalex gurneyalex gurneyalex left review comments

@cyrilmanuel cyrilmanuel cyrilmanuel approved these changes

@jbaudoux jbaudoux jbaudoux approved these changes

Assignees
No one assigned
Labels
approved ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
8 participants
@santostelmo @jbaudoux @OCA-git-bot @cyrilmanuel @pedrobaeza @gurneyalex @sebalix @Ricardoalso