pos icon indicating copy to clipboard operation
pos copied to clipboard

[16.0][FIX]pos_order_remove_line: Action on trash delete line

Open DorianMAG opened this issue 1 year ago • 24 comments

Change javascript method to delete a line when click on the trash

DorianMAG avatar Jan 04 '24 14:01 DorianMAG

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

OCA-git-bot avatar Jan 04 '24 14:01 OCA-git-bot

pop @DorianMAG please correct the pre commit

flotho avatar Jan 05 '24 06:01 flotho

pop @DorianMAG please correct the pre commit

Dont understand why pre-commit failed. Xml file look good

DorianMAG avatar Jan 05 '24 08:01 DorianMAG

pop @DorianMAG please correct the pre commit

Finally understand, semicolon less in javascript file. Thx for the help Regards

DorianMAG avatar Jan 05 '24 09:01 DorianMAG

/ocabot merge minor

robyf70 avatar Jan 05 '24 09:01 robyf70

On my way to merge this fine PR! Prepared branch 16.0-ocabot-merge-pr-1120-by-robyf70-bump-minor, awaiting test results.

OCA-git-bot avatar Jan 05 '24 09:01 OCA-git-bot

@robyf70 your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1120-by-robyf70-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

OCA-git-bot avatar Jan 05 '24 10:01 OCA-git-bot

@robyf70 Odoo test failed on pos_order_to_sale_order module. Any idea to fix the problem? Regards

DorianMAG avatar Jan 05 '24 10:01 DorianMAG

@robyf70 Odoo test failed on pos_order_to_sale_order module. Any idea to fix the problem? Regards

No idea how to fix it

robyf70 avatar Jan 08 '24 12:01 robyf70

@robyf70 All tests passed in PR. Can you retry the merge plz. Regards

DorianMAG avatar Jan 08 '24 13:01 DorianMAG

@robyf70 Odoo test failed on pos_order_to_sale_order module. Any idea to fix the problem? Regards

@legalsylvain Can you please look at this error?

2024-01-05 09:04:36,584 339 ERROR odoo odoo.addons.pos_order_to_sale_order.tests.test_module: FAIL: TestUi.test_pos_order_to_sale_order
Traceback (most recent call last):
  File "/__w/pos/pos/pos_order_to_sale_order/tests/test_module.py", line 36, in test_pos_order_to_sale_order
    self.assertEqual(len(before_orders) + 1, len(after_orders))
AssertionError: 1 != 0

robyf70 avatar Jan 08 '24 13:01 robyf70

/ocabot merge minor

Let's try

robyf70 avatar Jan 08 '24 13:01 robyf70

/ocabot merge minor

robyf70 avatar Jan 08 '24 13:01 robyf70

What a great day to merge this nice PR. Let's do it! Prepared branch 16.0-ocabot-merge-pr-1120-by-robyf70-bump-minor, awaiting test results.

OCA-git-bot avatar Jan 08 '24 13:01 OCA-git-bot

@legalsylvain Can you please look at this error?

No time to investigate. indeed, something look broken in V16 branch. however, I just tested pos_order_to_sale_order solo, and it works. So I guess that it is an incompatibility with other module(s) I can take a look in a few weeks.

legalsylvain avatar Jan 08 '24 13:01 legalsylvain

@robyf70 your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1120-by-robyf70-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

OCA-git-bot avatar Jan 08 '24 13:01 OCA-git-bot

@robyf70 Odoo test failed on pos_order_to_sale_order module. Any idea to fix the problem? Regards

@legalsylvain Can you please look at this error?

2024-01-05 09:04:36,584 339 ERROR odoo odoo.addons.pos_order_to_sale_order.tests.test_module: FAIL: TestUi.test_pos_order_to_sale_order
Traceback (most recent call last):
  File "/__w/pos/pos/pos_order_to_sale_order/tests/test_module.py", line 36, in test_pos_order_to_sale_order
    self.assertEqual(len(before_orders) + 1, len(after_orders))
AssertionError: 1 != 0

Hi @legalsylvain Thx, i take a look

DorianMAG avatar Jan 08 '24 14:01 DorianMAG

This shouldn't be merged as it is:

* The commits should be squashed into a single one

* The commit message should have a proper description, that contains a tag, the module name and the essence of the change e.g.: "[FIX] pos_order_remove_line: trash action"

* There are unnecessary changes in spacings

Thx, i take a look for commit recomandation. spacings changes was come from pre-commit, the original code did not have the correct indent

DorianMAG avatar Jan 08 '24 14:01 DorianMAG

This shouldn't be merged as it is:

* The commits should be squashed into a single one

* The commit message should have a proper description, that contains a tag, the module name and the essence of the change e.g.: "[FIX] pos_order_remove_line: trash action"

* There are unnecessary changes in spacings

It's done, Regards

DorianMAG avatar Jan 08 '24 14:01 DorianMAG

Hi @DorianMAG,

These changes revert some fixes made in https://github.com/OCA/pos/pull/1044. I checked it on runboat, having pos_loyalty installed:

  1. Select a customer
  2. Add a product to cart
  3. Add another different product to cart
  4. Remove last line
  5. Loyalty points balance hasn't changed

cc @papulo79

danielduqma avatar Jan 08 '24 16:01 danielduqma

Hi @DorianMAG,

I've repeat the process described by @danielduqma and I had the same results.

This MR has broken the module with pos loyalty.

@robyf70

papulo79 avatar Jan 09 '24 17:01 papulo79

@papulo79 @danielduqma Manifestly, the entire order is updated, except loyalty. I will take a look

DorianMAG avatar Jan 10 '24 08:01 DorianMAG

@DorianMAG I take from the last comments that this PR is still not ready yet?

ivantodorovich avatar Jan 15 '24 14:01 ivantodorovich

@DorianMAG I take from the last comments that this PR is still not ready yet?

Hi, indeed, i need to find a solution. I try to find time this week to fix that.

but this one is ok https://github.com/OCA/pos/pull/1122

DorianMAG avatar Jan 15 '24 14:01 DorianMAG

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[bot] avatar Jul 14 '24 12:07 github-actions[bot]