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

[FIX] [16.0] stock_customer_deposit: Allow to add more deposits of the same product #1775

Open
wants to merge 5 commits into
base: 16.0
Choose a base branch
from

Conversation

Shide
Copy link
Contributor

@Shide Shide commented Nov 21, 2024

Allows to add more deposits of the same product even if the customer already have a deposit.

Major refactoring in:

  • sale_order._check_can_customer_deposit to perform the check all at once + simplified sale_order._action_confirm
  • stock_move._action_assign to split casuistic and have a better control over the moves.
  • float_compare when it's necessary

MT-7979 @moduon @EmilioPascual @rafaelbn @yajo @Gelojr @fcvalgar please review if you want :)

@OCA-git-bot
Copy link
Contributor

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

@Shide Shide force-pushed the 16.0-increase_deposit-stock_customer_deposit branch 2 times, most recently from d82e915 to 90daeb5 Compare November 21, 2024 15:10
@Shide Shide force-pushed the 16.0-increase_deposit-stock_customer_deposit branch from 90daeb5 to 82d7e33 Compare November 21, 2024 15:14
Copy link

@fcvalgar fcvalgar left a comment

Choose a reason for hiding this comment

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

@rafaelbn rafaelbn added this to the 16.0 milestone Nov 24, 2024
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.

Thank you @Shide ! 😄

I looks great! and works great ❤️ !

Just please I'm missing and improvement of the readme (the readme should explain everything that the code does 😃):

The module lets you add more stock deposits for a product without needing to fully consume the previous one. This simplifies managing inventory for clients with fluctuating needs.

  • Improve README

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.

stock_customer_deposit/models/sale_order.py Show resolved Hide resolved
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.

Hey! I found a dangerous BUG! ⚠️

This video shows an issue with customer deposits when selling 250 units of 'product' for a new deposit when there is already one then the moodule apply 100% discount to the new deposit! And it should be paid by the client. The module wrongly applies a 100% discount, making the product appear free.

  • [BUG] stock_customer_deposit: Incorrect Discount Applied to New Sales Deposits (video 1 minute)

Alway with 💙

@Shide
Copy link
Contributor Author

Shide commented Nov 25, 2024

@rafaelbn All requests attended.

Done in separate commits because are thing that doesn't are related to the subject of this PR.

Copy link
Contributor

@Gelojr Gelojr left a comment

Choose a reason for hiding this comment

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

@Shide Shide requested a review from rafaelbn November 26, 2024 08:25
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.

@Shide it's still buggy 😇

  • [BUG] stock_customer_deposit: Incorrect Discount Applied to New Sales Deposits

Rev2: https://www.loom.com/share/4f76f39ef4074b738dd9acb04551143b?sid=8e017630-fee5-4c88-92b0-c1e19231e2cc

Only this detail, the rest is amazing! ❤️

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.

5 participants