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

[17.0][MIG] pos_lot_selection: Migration to 17.0 #1106

Merged
merged 29 commits into from
Jan 17, 2024

Conversation

nguyenminhchien
Copy link
Contributor

ref: BSRD-704

chienandalu and others added 28 commits November 30, 2023 14:20
With PR odoo/odoo#23698 merged, clone control is
no longer needed for lots.

[FIX] pos_lot_selection: group lots by quants

fixup! [FIX] pos_lot_selection: group lots by quants

fixup! fixup! [FIX] pos_lot_selection: group lots by quants
Object.assign is introduced in ES6 (2015) which is not supported by
PhantomJS. Replace it with the equivalent _.extend function to prevent
frontend tests from failing.

1. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Specifications
2. ariya/phantomjs#14506
Currently translated at 100.0% (1 of 1 strings)

Translation: pos-11.0/pos-11.0-pos_lot_selection
Translate-URL: https://translation.odoo-community.org/projects/pos-11-0/pos-11-0-pos_lot_selection/ca/
Regular Point of Sale Users with the minimal access rights are not
allowed to read the `stock.production.lot` model.
Currently translated at 100.0% (2 of 2 strings)

Translation: pos-16.0/pos-16.0-pos_lot_selection
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_lot_selection/it/
Currently translated at 100.0% (2 of 2 strings)

Translation: pos-16.0/pos-16.0-pos_lot_selection
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_lot_selection/it/
Currently translated at 100.0% (2 of 2 strings)

Translation: pos-16.0/pos-16.0-pos_lot_selection
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_lot_selection/es/
Currently translated at 100.0% (2 of 2 strings)

Translation: pos-16.0/pos-16.0-pos_lot_selection
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_lot_selection/it/
@nguyenminhchien nguyenminhchien marked this pull request as ready for review December 7, 2023 07:19
@nguyenminhchien nguyenminhchien mentioned this pull request Dec 7, 2023
30 tasks
@gurneyalex
Copy link
Member

/ocabot migration pos_lot_selection

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Dec 7, 2023
@Fabrizio1305
Copy link

Fabrizio1305 commented Dec 11, 2023

It works
(but I have a bug that I think is not related to the module, but to odoo, as it happens also without the module)
Tested flow:

  1. activate lots on products
  2. take a product available in POS, and update qty to create an entry with a LOT (make sure the lot number doesn't exist yet for any product), make sure the product doesn't have qty in stock without a LOT (to avoid inconsistencies).
    Create a second and third LOT on the same product, one of them with quantities and one without
  3. open POS
  4. add that product to a cart the standard way
  5. on the lot selection popup, a drop down menu should appear with the choice of lots that are in stock, choose one of them
  6. finalize the transaction by paying in cash and closing the POS
  7. and double check that the proper product and lot has been taken out of stock (inventory moves)

The flow worked, but on point 8, in the POS session data I have the correct lot sold, but in the inventory app I have another lot being delivered. I think this is an odoo bug rather than this module's bug as it also happens without the module.
tested with and without #1105 pos_lot_barcode

@etobella
Copy link
Member

etobella commented Jan 5, 2024

@nguyenminhchien I made the migration but made it compatible with an offline PoS. Do you prefer that I make a PR directly to your PR or should I made a new one (I would keep part of your code regarding to tests for example)

@nguyenminhchien
Copy link
Contributor Author

@nguyenminhchien I made the migration but made it compatible with an offline PoS. Do you prefer that I make a PR directly to your PR or should I made a new one (I would keep part of your code regarding to tests for example)

Please create a pull request to my branch nguyenminhchien:17.0-mig-pos_lot_selection, thanks.

@etobella
Copy link
Member

etobella commented Jan 8, 2024

I made the PR:

nguyenminhchien#3

Can you review it?

@nguyenminhchien
Copy link
Contributor Author

I made the PR:

nguyenminhchien#3

Can you review it?

@etobella hi, i merged your commit but the test was failed, please check it. thanks.

@etobella
Copy link
Member

I made the fix, it was just a small typo error on the last change 😞

@cvinh
Copy link
Contributor

cvinh commented Jan 15, 2024

I made the PR:
nguyenminhchien#3
Can you review it?

@etobella hi, i merged your commit but the test was failed, please check it. thanks.

Hello, new PR here nguyenminhchien#4

Copy link
Contributor

@cvinh cvinh left a comment

Choose a reason for hiding this comment

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

Functional tests ok

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

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

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-1106-by-etobella-bump-nobump, awaiting test results.

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

@OCA-git-bot OCA-git-bot merged commit f6322d2 into OCA:17.0 Jan 17, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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