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

[ADD] stock_barcodes_gs1: Add support to stock_barcodes module to read GS1 barcodes #203

Merged
merged 1 commit into from Oct 3, 2019
Merged

[ADD] stock_barcodes_gs1: Add support to stock_barcodes module to read GS1 barcodes #203

merged 1 commit into from Oct 3, 2019

Conversation

sergio-teruel
Copy link

@pedrobaeza pedrobaeza added this to the 11.0 milestone Aug 27, 2019
@pedrobaeza
Copy link
Member

Please check Travis

@pedrobaeza
Copy link
Member

@ernestotejeda @victormmtorres please review this

@sergio-teruel
Copy link
Author

Mhuuu... it seems that DB connection is failing
https://travis-ci.org/OCA/stock-logistics-barcode/jobs/577395248#L364

@sergio-teruel
Copy link
Author

Travis template updated

@sergio-teruel
Copy link
Author

Fails due to product_multi_ean module that only alow to have EAN13 primary barcodes...

@pedrobaeza
Copy link
Member

That's not valid anymore when field was generalized. That module must be changed.

@sergio-teruel
Copy link
Author

This PR #205 is needed for pass the tests

rafaelbn
rafaelbn approved these changes Sep 30, 2019
View reviewed changes
Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

👍 tested functional in test enviroment

@rafaelbn
Copy link
Member

Please @carlosdauden review thanks

rousseldenis
rousseldenis reviewed Oct 1, 2019
View reviewed changes
stock_barcodes_gs1/wizard/stock_production_lot.py
return
if package_barcode:
packaging = self.env['product.packaging'].search([
('barcode', '=', package_barcode),
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Shouldn't be useful to reuse _barcode_domain() function as in https://github.com/OCA/stock-logistics-barcode/pull/203/files#diff-fb62e484a3862c535a01cefb6a9d013cR45 ?

rousseldenis
rousseldenis reviewed Oct 1, 2019
View reviewed changes
stock_barcodes_gs1/wizard/stock_production_lot.py
self.product_id = packaging.product_id
elif product_barcode:
product = self.env['product.product'].search([
('barcode', '=', product_barcode),
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Same as above.

rousseldenis
rousseldenis approved these changes Oct 1, 2019
View reviewed changes
Copy link
Sponsor Contributor

@rousseldenis rousseldenis 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. Just a question

@OCA-git-bot
Copy link
Contributor

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

@sergio-teruel
Copy link
Author

Hi @rousseldenis !!! Changes done... I can't be there this year !!!

@sergio-teruel
Copy link
Author

@rousseldenis the method _prepare_barcode_domain is not defined in wiz.stock.barcodes.new.lot so I can not use it in this model, but You are rigth, so in other PR, after this has been merged, I will do this improvement ... OK?

@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 11.0-ocabot-merge-pr-203-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 3, 2019
@OCA-git-bot
Merge PR #203 into 11.0
4a21dd1 
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit a7b57a8 into OCA:11.0 Oct 3, 2019
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 9a1807f. Thanks a lot for contributing to OCA. ❤️

@pedrobaeza pedrobaeza mentioned this pull request Oct 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlosdauden carlosdauden carlosdauden approved these changes

@rafaelbn rafaelbn rafaelbn approved these changes

@rousseldenis rousseldenis rousseldenis approved these changes

@victormmtorres victormmtorres Awaiting requested review from victormmtorres

Assignees
No one assigned
Labels
approved merged 🎉 needs review ready to merge
Projects
None yet
Milestone
11.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@sergio-teruel @pedrobaeza @rafaelbn @OCA-git-bot @carlosdauden @rousseldenis @oca-travis