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][IMP] shopinvader_schema_address: add field res_partner.company_type on Address Schema #1561

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

Conversation

chaule97
Copy link

No description provided.

@@ -16,6 +16,7 @@ class Address(StrictExtendableBaseModel):
email: str | None = None
state_id: int | None = None
country_id: int | None = None
company_type: str | None = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would have been a good idea to declare the company_type as an enum in order to provide api users with the possible values for this field....

Copy link
Member

Choose a reason for hiding this comment

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

I also think we should do a little something to document the possible values for company_type.
@lmignon do you know if Literal["person", "company"] | None would work with pydantic? Else a str | None = Field(description="...").

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lmignon For sure it works. 'Literal' will be rendered as 'enum' into the openapi doc. {...'company_type': {'enum': ['person', 'company'], ....}.

@lmignon
Copy link
Collaborator

lmignon commented Jul 23, 2024

/ocabot merge patch

@shopinvader-git-bot
Copy link

@lmignon The merge process could not start, because command git fetch --quiet --force --prune https://github.com/shopinvader/odoo-shopinvader 'refs/heads/*:refs/heads/*' failed with output:

fatal: detected dubious ownership in repository at '/app/run/.cache/oca-mqt/github.com/shopinvader/odoo-shopinvader'
To add an exception for this directory, call:

	git config --global --add safe.directory /app/run/.cache/oca-mqt/github.com/shopinvader/odoo-shopinvader

@sbidoul
Copy link
Member

sbidoul commented Jul 23, 2024

Let's run the tests first ;)

And I'll check what's up with the bot.

@shopinvader-git-bot
Copy link

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

@sebastienbeau sebastienbeau added this to the 16.0 milestone Sep 19, 2024
Copy link
Contributor

@paradoxxxzero paradoxxxzero left a comment

Choose a reason for hiding this comment

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

LGTM and Literal["person", "company"] | None would indeed be better.

@chaule97 chaule97 force-pushed the 16.0-imp-shopinvader_schema_address branch from 212477f to 56b764c Compare December 4, 2024 02:59
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.

8 participants