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][IMP] partner_company_type: allow to set up countries and states, display shortcut in name #1604

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

[16.0][IMP] partner_company_type: allow to set up countries and states, display shortcut in name #1604

wants to merge 1 commit into from

Conversation

OriolMForgeFlow
Copy link
Contributor

No description provided.

@OriolMForgeFlow OriolMForgeFlow force-pushed the 16.0-imp-partner_company_type branch 5 times, most recently from 944bccc to 914e55c Compare November 17, 2023 09:18
This was referenced Nov 17, 2023
Closed
Closed
@OriolMForgeFlow OriolMForgeFlow force-pushed the 16.0-imp-partner_company_type branch 2 times, most recently from 44a8411 to 5350466 Compare November 17, 2023 10:50
JordiMForgeFlow
JordiMForgeFlow approved these changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

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

@JordiMForgeFlow JordiMForgeFlow force-pushed the 16.0-imp-partner_company_type branch from 5350466 to 9c4d7ef Compare January 9, 2024 13:36
@github-actions GitHub Actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 12, 2024
AaronHForgeFlow
AaronHForgeFlow approved these changes May 13, 2024
View reviewed changes
Copy link

@AaronHForgeFlow AaronHForgeFlow 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 LG

LoisRForgeFlow
LoisRForgeFlow approved these changes May 13, 2024
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow 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 👍

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 19, 2024
@github-actions GitHub Actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 22, 2024
@JordiMForgeFlow
Copy link
Contributor

@rousseldenis could we get a review here? :)

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 29, 2024
@rousseldenis
Copy link
Contributor

@OriolMForgeFlow @len-foss Isn't it the same problematic ?

#1917

I would say I'd prefer an EXCLUDE

rousseldenis
rousseldenis reviewed Dec 6, 2024
View reviewed changes
partner_company_type/models/res_partner.py Outdated
rec.partner_company_type_id.country_ids
and rec.country_id not in rec.partner_company_type_id.country_ids
):
raise UserError(
Copy link
Contributor

Choose a reason for hiding this comment

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

You should raise ValidationErrorin a contraint.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rousseldenis corrected :)

@len-foss
Copy link

len-foss commented Dec 6, 2024 •
edited
Loading

@rousseldenis
#1917 is to relax the name check to allow the same name to be used for 2 different country (which I assume would speak the same language, which isn't so rare in the world), which isn't actually done here.

Here there's a check to choose a company type to match the partner's country which is nice though.

Since it seems that different use-cases want for different constraint, it would make sense to have a configuration option to choose what is allowed. Therefore entirely drop the sql constaint for the python check.

@JordiMForgeFlow JordiMForgeFlow force-pushed the 16.0-imp-partner_company_type branch from 9c4d7ef to ac966c5 Compare December 10, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JordiMForgeFlow JordiMForgeFlow JordiMForgeFlow approved these changes

@rousseldenis rousseldenis rousseldenis left review comments

@LoisRForgeFlow LoisRForgeFlow LoisRForgeFlow approved these changes

@AaronHForgeFlow AaronHForgeFlow AaronHForgeFlow approved these changes

Assignees
No one assigned
Labels
approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@OriolMForgeFlow @JordiMForgeFlow @rousseldenis @len-foss @AaronHForgeFlow @LoisRForgeFlow @OCA-git-bot