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

[13.0][ADD]product_harmonized_system_heading & product_harmonized_system_heading_stock #229

Conversation

GuillemCForgeFlow
Copy link

New module to have a better structure on H.S. Codes. If installed, H.S. Codes will require a H.S. Heading to be created first.

cc @ForgeFlow

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_harmonized_system_heading_ branch from b7f7464 to 35b699b Compare October 3, 2023 08:08
@GuillemCForgeFlow
Copy link
Author

Hi @pedrobaeza
Whenever you have time, could you tell me what you think about this new module so that we can better structure H.S. Codes?

As seen in www.tariffnumber.com, H.S. Codes will always have a Heading:
image

By having this module, we could have a better structure on those.

@pedrobaeza pedrobaeza added this to the 13.0 milestone Oct 3, 2023
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

About the improvements on the existing module, they are OK. About the new module, not sure if very useful, as at the end, you don't manage the HS codes. You just assign them to products. And what is a blocking point is to add extra constraints that requires context tricks in other unrelated modules. The technique if you want to keep such restriction if to not apply it when on test mode (config["test_enable"]) and not testing the constraint itself.

intrastat_product/tests/test_brexit.py Outdated Show resolved Hide resolved
@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_harmonized_system_heading_ branch 2 times, most recently from bd23920 to c6123f8 Compare October 3, 2023 14:58
@GuillemCForgeFlow
Copy link
Author

Hi @pedrobaeza
Thank you for the feedback, I've applied the changes now.

Regarding the use of the module, I've done it in a way that you don't need to install the module to still be using the H.S. Codes provided by the product_harmonized_system module. This module only adds a new level of complexity in order to have H.S. Codes in a better structure, just for informational purposes.

We have a customer need that requires us to structure H.S. Codes in such a way and I thought that it could be a good improvement to the current OCA structure. If you don't agree, I will just close it and set it in on a custom module. WDYT?


@api.model_create_multi
def create(self, vals_list):
if not config["test_enable"] or self.env.context.get("check_hs_code_heading"):
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this, just do hs_code_heading_id a computed writable field, but removing the error if not found.

Copy link
Author

Choose a reason for hiding this comment

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

I've just changed it now to work in the following way:

  • When an H.S. Code is created, it will try to find the corresponding Heading, if none is found, it will automatically create a new one, instead of triggering an error to then manually create the Heading.
  • If we edit the Local Code, we also call the _get_hs_code_heading_id method to get the new Heading.

Doing it like this, we don't need to adapt any tests and by optionally having this module, the user won't need to go through any additional steps.

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_harmonized_system_heading_ branch 3 times, most recently from 7f1b8ed to 74ebd62 Compare October 5, 2023 11:00
- extract `name_get` logic in another method to be inherited
- remove _sql_constraints and change it for an api constrains method to have a better control and so that there no global duplicates (those that are not company specific)
- replaced `replace` method for strip()
@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_harmonized_system_heading_ branch from 74ebd62 to f83e953 Compare October 10, 2023 08:03
New module to have a better structure on H.S. Codes. If installed, H.S. Codes will require from a H.S. Heading to be created first.
Glue module between product_harmonized_system_heading and stock in order to add the needed Menu Item.
Just as done in the `product_harmonized_system_stock` module.
@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_harmonized_system_heading_ branch from f83e953 to efbe8ef Compare October 13, 2023 13:30
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 Feb 11, 2024
@github-actions github-actions bot closed this Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants