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

[16.0][FIX] pos_product_multi_barcode: incoherent data #1281

Merged
merged 1 commit into from
Jan 13, 2025
Merged

[16.0][FIX] pos_product_multi_barcode: incoherent data #1281

merged 1 commit into from
Jan 13, 2025

Conversation

danielduqma
Copy link
Contributor

@danielduqma danielduqma commented Dec 26, 2024 •
edited
Loading

If a barcode is already in product_by_barcode, do not override it, as Odoo does with product_by_id. This can lead to incoherent data if something changes after product is first loaded and then reloads (if limited loading is enabled), or it is modified in product_by_id and not in product_by_barcode.

FL-556-4600

FL-556-5129

@danielduqma
[FIX] pos_product_multi_barcode: incoherent data
aa51e5f 
If a `barcode` is already in `product_by_barcode`, do not override
it, as Odoo does with `product_by_id`. This can lead to incoherent
data if something changes after product is first loaded and then
reloads (if limited loading is enabled), or it is modified in
`product_by_id` and not in `product_by_barcode`.
LuisAlejandroS
LuisAlejandroS approved these changes Dec 26, 2024
View reviewed changes
Copy link

@LuisAlejandroS LuisAlejandroS left a comment

Choose a reason for hiding this comment

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

LGTM!

@danielduqma
Copy link
Contributor Author

Hi @OCA/pos-maintainers, can this be merged? Thanks!

ivantodorovich
ivantodorovich approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

Seems fine

Could you provide more information about how to reproduce the error trying to solve?

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

@danielduqma
Copy link
Contributor Author

Thanks for the review @ivantodorovich! The way we found this is a little tricky, and involves another module loading some product info asynchronously after PoS first load. The process is:

  1. First load of PoS, with limited loading of products
  2. Another module loads additional info asynchronously, updating db.products_by_id (and consequently db.products_by_barcode as it points to same objects)
  3. Some process loads again a product already loaded in first step
  4. Even though point_of_sale discards this info, pos_product_multi_barcode overrides db.products_by_barcode entry
  5. Product in db.products_by_id is different to db.products_by_barcode

Regards

@ivantodorovich
Copy link
Contributor

Thanks!

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1281-by-ivantodorovich-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 850a5fe into OCA:16.0 Jan 13, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 5fb12ad. 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
Reviewers

@LuisAlejandroS LuisAlejandroS LuisAlejandroS approved these changes

@ivantodorovich ivantodorovich ivantodorovich approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@danielduqma @OCA-git-bot @ivantodorovich @LuisAlejandroS