Skip to content

[18.0][MIG] sequence_check_digit: Migration to 18.0 #988

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

Merged
merged 23 commits into from
Jan 27, 2025

Conversation

yankinmax
Copy link
Contributor

No description provided.

etobella and others added 18 commits January 9, 2025 13:19
* [ADD] Sequence check Digit

* Change of License to LGPL

* Code review

* Reviews
Currently translated at 100,0% (13 of 13 strings)

Translation: server-ux-11.0/server-ux-11.0-sequence_check_digit
Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-sequence_check_digit/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-ux-12.0/server-ux-12.0-sequence_check_digit
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-sequence_check_digit/
Fix external dependency from `stdnum` to `python-stdnum`
@StephaneMangin
Copy link

StephaneMangin commented Jan 16, 2025

I wonder if adding a log for missing and mandatory dependencies makes sense. Despite the fact that it fails the codecov plugin.

+ import logging
+ 
+ from odoo.exceptions import ValidationError
+ from odoo.tests import common
+ 
+ _logger = logging.getLogger(__name__)
+ try:
+     from stdnum import damm, luhn, verhoeff
+     from stdnum.iso7064 import mod_11_2, mod_11_10, mod_37_2, mod_37_36, mod_97_10
+ except (OSError, ImportError) as err:
+     _logger.debug(err)
+ 
+ 

@StephaneMangin
Copy link

A quick unitest to handle the codecov requirements could be to add such a method in the tests battery:

    def test_get_check_digit_exceptions(self):
        sequence = self.get_sequence("none")
        with self.assertRaises(ValidationError):
            sequence.get_check_digit("ABC")

Copy link

@vvrossem vvrossem left a comment

Choose a reason for hiding this comment

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

@yankinmax
Copy link
Contributor Author

I wonder if adding a log for missing and mandatory dependencies makes sense. Despite the fact that it fails the codecov plugin.

+ import logging
+ 
+ from odoo.exceptions import ValidationError
+ from odoo.tests import common
+ 
+ _logger = logging.getLogger(__name__)
+ try:
+     from stdnum import damm, luhn, verhoeff
+     from stdnum.iso7064 import mod_11_2, mod_11_10, mod_37_2, mod_37_36, mod_97_10
+ except (OSError, ImportError) as err:
+     _logger.debug(err)
+ 
+ 

I've checked the other typical modules. In general, it's the same way to inform the admin of missing dependency

@yankinmax yankinmax force-pushed the 18.0-mig-sequence_check_digit branch from dc17f8b to b21105c Compare January 21, 2025 12:51
@yankinmax yankinmax force-pushed the 18.0-mig-sequence_check_digit branch from b21105c to 899f0f3 Compare January 21, 2025 13:18
@yankinmax
Copy link
Contributor Author

A quick unitest to handle the codecov requirements could be to add such a method in the tests battery:

    def test_get_check_digit_exceptions(self):
        sequence = self.get_sequence("none")
        with self.assertRaises(ValidationError):
            sequence.get_check_digit("ABC")

Added

Copy link

@StephaneMangin StephaneMangin left a comment

Choose a reason for hiding this comment

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

LGTM

@simahawk
Copy link

/ocabot migration sequence_check_digit

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Jan 27, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Jan 27, 2025
26 tasks
@simahawk
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-988-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 01a7669 into OCA:18.0 Jan 27, 2025
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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