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

[ADD] product_internal_reference_generator #1453

Merged

Conversation

ilyasProgrammer
Copy link
Member

@ilyasProgrammer ilyasProgrammer commented Nov 20, 2023

This module allows to generate internal references for Product templates and variants using sequences, setting codes as read-only.

In product template, it's possible to choose among different Internal Reference Templates related to a sequence, and then generate an internal reference with the following structure:

Internal Reference Prefix + progressive number for variant, eg: “Main0001001”, where:

"Main0001" is the prefix generated by sequence and assigned to product template, and

"001" the variant identifier.

Every time a new variant is created, a new internal reference is automatically assigned with progressive variant code.

A specific access rights allows specific users to change internal reference template for a product template once an internal reference has been generated, as well as editing existing ones.

Copy link
Contributor

@aleuffre aleuffre left a comment

Choose a reason for hiding this comment

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

Moving my review over from OCA/product-variant#309

Code review, LGTM!

@ilyasProgrammer
Copy link
Member Author

Module is incompatible with product_sequence. Is there a way to specify incompatibility?

@aleuffre
Copy link
Contributor

aleuffre commented Nov 20, 2023

There are two steps you need to take:

  1. Add the key excludes to your __manifest__.py. See for example: https://github.com/OCA/account-financial-tools/blob/14.0/account_asset_management/__manifest__.py#L11

  2. run copier update --trust -A -o rej on the repo (install copier if necessary: https://copier.readthedocs.io/en/stable/ ),
    when you get to the question about rebel modules, specify the list of module(s) you want to test separately from the rest of the modules in the repo. This should generate the appropriate files in .github/workflows/test.yaml Example:
    image

    Fix conflicts and remove any .rej files if any were created, and commit the changes.

    If the command doesn't seem to generate the correct files, you can try copier recopy instead of copier update

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-product_internal_reference_generator branch from daee95c to 68bc34a Compare November 20, 2023 12:04
@aleuffre
Copy link
Contributor

Make sure to also push the new .copier-answers.yml file!

@ilyasProgrammer
Copy link
Member Author

ilyasProgrammer commented Nov 20, 2023

There was too many questions from copier after copier recopy and lots of files changed. So i just copied only tests yml. I dont think my new answers file was correct.
Is it possible to run copier recopy to regenerate only tests yml ?

@aleuffre
Copy link
Contributor

Aside from the one question that I took a screenshot of, you don't have to change any of the other answers. if you don't commit the .copier-answers.yml, your changes may be reverted. In general the commit may not be accepted if you only change test.yaml

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-product_internal_reference_generator branch from 5dc039c to 3d0634f Compare November 20, 2023 15:12
@francesco-ooops
Copy link
Contributor

@ilyasProgrammer why don't you change copier in a separate PR so it gets merged straight away?

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-product_internal_reference_generator branch from 3d0634f to f406b5d Compare November 20, 2023 15:13
@ilyasProgrammer
Copy link
Member Author

@ilyasProgrammer why don't you change copier in a separate PR so it gets merged straight away?

Problem not in copier, but in modules conflicts.

@ilyasProgrammer
Copy link
Member Author

But looks like need to update copier as well

@francesco-ooops
Copy link
Contributor

@ilyasProgrammer copier files should be removed from here I think

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-product_internal_reference_generator branch from f406b5d to 6b65aed Compare November 21, 2023 12:43
@ilyasProgrammer
Copy link
Member Author

rebased.
I guess rebel_module_groups needs to stay in the answers file

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional review ok!

@pedrobaeza feedback? thanks!

@pedrobaeza pedrobaeza added this to the 14.0 milestone Nov 21, 2023
.copier-answers.yml Show resolved Hide resolved
@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-product_internal_reference_generator branch from 6b65aed to 04ba879 Compare November 22, 2023 09:47
@francesco-ooops
Copy link
Contributor

@pedrobaeza good to go?

@pedrobaeza
Copy link
Member

No, the change should be done in the copier template and should be reflected in the CI yaml, not manually edited the file .copier-answers....

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-product_internal_reference_generator branch from 04ba879 to b028717 Compare November 24, 2023 16:55
@ilyasProgrammer
Copy link
Member Author

So what i did is:
copier update --trust
Are there in this repo modules that don't get along with their friends? If so, list them here (YAML format) and they will be tested in separate
Beware, if rebel modules should stay separated in groups, you should join them with commas, which could be misinterpreted by YAML.
Example: ["rebel_module_1,rebel_module_2", even_more_rebel_module]
[product_internal_reference_generator]

@pedrobaeza is it correct ?

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.

OK, now it is correct

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-1453-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit da7b4ab into OCA:14.0 Nov 24, 2023
8 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at ca4cefe. 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.

5 participants