Skip to content

[12.0][MIG] pos_tare w/ barcode#491

Merged
OCA-git-bot merged 34 commits intoOCA:12.0from
Fkawala:12.0-mig-tare
Jul 23, 2020
Merged

[12.0][MIG] pos_tare w/ barcode#491
OCA-git-bot merged 34 commits intoOCA:12.0from
Fkawala:12.0-mig-tare

Conversation

@Fkawala
Copy link

@Fkawala Fkawala commented May 17, 2020

Following up our discussion with @legalsylvain here #436 and there #447 Sylvain and I decided to split the work related to tare in two parts.

This PR covers the first part. The second part will be delivered later next week.

@OCA-git-bot
Copy link
Contributor

Hi @Fkawala,
some modules you are maintaining are being modified, check this out!

@Fkawala Fkawala changed the title [12.0][MIG] pos_tare & pos_barcode_tare (partially) [12.0][MIG] pos_tare w/ barcode May 18, 2020
@legalsylvain
Copy link
Contributor

Hi @Fkawala. I'm currently reviewing this PR.
I'll do a PR against your, to change something.
kind regards.

@Fkawala
Copy link
Author

Fkawala commented May 19, 2020

Hi @Fkawala. I'm currently reviewing this PR.
I'll do a PR against your, to change something.
kind regards.

Works for me!

@Fkawala
Copy link
Author

Fkawala commented Jun 15, 2020

@legalsylvain this PR is pending a formal test with an ESCPOS printer. I'll try to get that done tomorrow. In the meantime would you have some spare cycles to review the code?

@vvrossem this PR is the base layer for a proper tare / gross weight handling in the POS. It adds a "Tare" button and includes @legalsylvain's nice rework of the scale screen (first introduced in #436). This PR features the gross weight / Tare / net weight display on receipt (works for web print and proxy print). This PR is meant to become a dependency for #470 that you've already reviewed a while ago. Do you think you'll have time to review this? If not would you be kind enough to bounce it back to someone else at CoopITEasy? Thank you in advance.

@Fkawala
Copy link
Author

Fkawala commented Jun 15, 2020

The CI tests fail b/c:

odoo.exceptions.UserError: ("You try to install module 'pos_tare' that depends on module 'base_fontawesome'.\nBut the latter module is not available in your system.", '')

I'm not sure how to fix that!?

@Fkawala
Copy link
Author

Fkawala commented Jun 15, 2020

Sneak peek at the UI changes:

image
image
image

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.

Impressive one!

Comment on lines +125 to +132
check_sanitize_value: function (input_name) {
var res = this.$(input_name)[0].value.replace(',', '.').trim();
if (isNaN(res)) {
this.$(input_name).css("background-color", "#F66");
return undefined;
}
this.$(input_name).css("background-color", "#FFF");
return parseFloat(res, 10);
},

Choose a reason for hiding this comment

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

Nice!

Copy link
Author

Choose a reason for hiding this comment

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

Kudos go to @legalsylvain on this (and most of the code actually).

Comment on lines +138 to +158
validate_order: function(options) {
var order = this.pos.get_order();
var orderlines = Array.from(order.get_orderlines());

if (orderlines.some(leq_zero_qty)) {
var _super_validate_order = this._super.bind(this);
var wrong_orderline = orderlines.find(leq_zero_qty);
var wrong_product = wrong_orderline.get_product().display_name;
this.gui.show_popup('confirm', {
title: _t('Quantity lower or equal to zero'),
body: _.str.sprintf(
_t("The quantity for \"%s\" is lower or equal to" +
" zero. Call for help unless you're perfectly" +
" sure you are doing right."), wrong_product),
confirm: function() {
_super_validate_order();
},
});
return;
}
return this._super(options);
},

Choose a reason for hiding this comment

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

🎉

Copy link
Author

Choose a reason for hiding this comment

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

Glad you liked it! Thank you for the review :)

@vvrossem
Copy link

The CI tests fail b/c:

odoo.exceptions.UserError: ("You try to install module 'pos_tare' that depends on module 'base_fontawesome'.\nBut the latter module is not available in your system.", '')

I'm not sure how to fix that!?

Neither do I, but I noticed the following logs appear in the console on the pos.config view:

Non loaded modules:       
Array [ "base_fontawesome5.FormRenderer", "base_fontawesome5.ListRenderer" ]

pos_config

@Fkawala
Copy link
Author

Fkawala commented Jun 16, 2020

The CI tests fail b/c:
odoo.exceptions.UserError: ("You try to install module 'pos_tare' that depends on module 'base_fontawesome'.\nBut the latter module is not available in your system.", '')
I'm not sure how to fix that!?

Neither do I, but I noticed the following logs appear in the console on the pos.config view:

Non loaded modules:       
Array [ "base_fontawesome5.FormRenderer", "base_fontawesome5.ListRenderer" ]

pos_config

Not sure how to read what you sent. The dependency is expected (we are trying to find a nice icon to make the tare information more explicit). The CI crash is kind of expected since it does not have the dependency (https://github.com/OCA/server-tools/tree/12.0/base_fontawesome) available.

@legalsylvain
Copy link
Contributor

this commit should fix the problem.
could you rebase ?

@Fkawala
Copy link
Author

Fkawala commented Jun 16, 2020

@legalsylvain thank you!

@Fkawala
Copy link
Author

Fkawala commented Jun 24, 2020

Howdy, can we try to close this one in what is left of this week?

@legalsylvain
Copy link
Contributor

it seems you have some conflicts. could you try to fix it ? I guess a rebase should solve the issue.

@Fkawala
Copy link
Author

Fkawala commented Jul 4, 2020

Hello @legalsylvain, hope you're doing good! Am kind of lost with the codecov tool. Would you be able to provide any guidance to get that working? I looked around in codecov but https://codecov.io/gh/OCA/pos/tree/aa2ffbf91645ab059a36fe08b74a0f367b209cd0/pos_tare/models looks ok to me and I don't know what to do. Thank you!

@legalsylvain
Copy link
Contributor

Hi @Fkawala.
regarding codecov, it's not a blocking point. it just indicative to see if the coverage is good or not. for the current case, I don't understand why a 100% covered code make the ratio down...

I made some little improvment here : could you review it ? Fkawala#5

thanks !

@legalsylvain legalsylvain force-pushed the 12.0-mig-tare branch 2 times, most recently from 1a52e95 to 9fcd244 Compare July 6, 2020 12:50
[ADD] barcode nomenclature for tare
[ADD] tare field on pos.order.line model
[ADD] Tare button in PoS numpad
[IMP] handle correctly different UoM
[IMP] add warning at checkout if qty <=0
[IMP] display gross and tare weight for each line

Co-authored-by: François Kawala <[email protected]>
Co-authored-by: Sylvain LE GAL <[email protected]>
@Fkawala
Copy link
Author

Fkawala commented Jul 23, 2020

Could we merge and close this PR? Could/Should I do it my self as the maintainer? Thank you and have a nice day!

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

@Fkawala
Copy link
Author

Fkawala commented Jul 23, 2020

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Sorry @Fkawala you are not allowed to merge.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@legalsylvain
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-491-by-legalsylvain-bump-patch, awaiting test results.

@Fkawala
Copy link
Author

Fkawala commented Jul 23, 2020

@legalsylvain Merci!

@OCA-git-bot OCA-git-bot merged commit ba4ebdf into OCA:12.0 Jul 23, 2020
@OCA-git-bot
Copy link
Contributor

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

@Fkawala Fkawala deleted the 12.0-mig-tare branch August 31, 2020 06:56
@Fkawala Fkawala restored the 12.0-mig-tare branch August 31, 2020 06:57
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.

6 participants