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

Error in calculating quantity ordered or reserved for manufacturing order and distribution order #3472

Merged
merged 3 commits into from Jun 29, 2021

Conversation

e-Evolution
Copy link
Contributor

Related with #3266

  • Remove unnecessary code
  • Improvement of the code separating the behavior of calculation of weight and volume of the reservation process.

A good practice is that a function method in the code should be responsible for only one behavior

replace #3452

…Sales Order, manufacturing order and distribution order adempiere#3266

- Create new method to force same Warehouse for Sales Order Line but SO/PO
- Create new calculate order sizes
- Added new logic for that the Qty Reserved is only setting when the  product is Stocked
…Sales Order, manufacturing order and distribution order adempiere#3266

- Fixed ideation code
…manufacturing order and distribution order adempiere#3266

- Remove unnecessary code
- Improvement of the code separating the behavior of calculation of weight and volume of the reservation process.

A good practice is that a function method in the code should be responsible for only one behavior
@marcalwestf
Copy link
Collaborator

@e-Evolution hi Victor, this seems to be an important issue for Libero Manufacturing.

Could you please write a step-by-step way to test it?
As I understand from your explanations, the behavior as of now should not change, even if calling calculateOrderSizes() has been moved and its implementation redefined.

Thanks!

@marcalwestf
Copy link
Collaborator

... and the build conflict solved...

@marcalwestf marcalwestf added the 17 Waiting for Information Information needed to continue testing label Jun 28, 2021
@e-Evolution
Copy link
Contributor Author

@e-Evolution hi Victor, this seems to be an important issue for Libero Manufacturing.

Could you please write a step-by-step way to test it?
As I understand from your explanations, the behavior as of now should not change, even if calling calculateOrderSizes() has been moved and its implementation redefined.

Thanks!

@marcalwestf Hi Mario This change applies a best practice in code development, based on the SOLID responsibility principle, a method should only have one purpose or responsibility, so the reservation method not should also calculate the total weight and volume.

So this pull request separates the logic of calculating weight and volume from the reservation method to an independent method and calls the method after the quantities were reserved, it also removes commented code that should not be more.

To validate the tests you must follow the following steps:

1.- Find a product and add weight and volume
2.- Create a sales order for the product with volume and weight
3.- Complete the sales order
4.- Validate that the reservation was made correctly in the inventory.
5.- Validate that the weight and volume for the order is correctly calculated

@yamelsenih
Copy link
Member

Hi @e-Evolution @marcalwestf I approve this, the old PR was very wrong but @e-Evolution change this following my comments.

Best regards

@marcalwestf
Copy link
Collaborator

@e-Evolution Victor, could you please solve the conflicts in order to be able to accept the PR?
Thanks

@marcalwestf marcalwestf added 10 Reviewed by Peer 14 Waiting for User Changes Waiting for Pull request User make changes and removed 09 Pending Peer Review 17 Waiting for Information Information needed to continue testing labels Jun 28, 2021
Copy link
Collaborator

@marcalwestf marcalwestf left a comment

Choose a reason for hiding this comment

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

  • Create a product with Weight and Volume
  • Do an Inventory or a Material Receipt on this product
  • Complete a Sales Order with this product
  • Check the Volume and the Weight in the Sales Order Header
  • Do the same with orders having several products with Weight and Volume defined

The Volume and the Weight in the Sales Order Header are according to the Weight/Volume defined and the quantities of the Order Lines. The same applies to reserved quantities.

-> Test passed.

@marcalwestf marcalwestf removed the 14 Waiting for User Changes Waiting for Pull request User make changes label Jun 28, 2021
@marcalwestf marcalwestf merged commit b515a77 into adempiere:develop Jun 29, 2021
e-Evolution added a commit to e-Evolution/adempiere that referenced this pull request Jul 1, 2021
* develop: (29 commits)
  Add overwrite values from Smart Browser field
  Allows define system events for validation of Standard Request Type
  Validate client
  Add better approach for validation engine
  Add optional value for source price list
  Add support to data type as mapping from entities (adempiere#3476)
  Adding the Weight and Volume field into Sales Order and Purchase Order  Feature/3473 (adempiere#3474)
  Add the database function to get the daily salary (adempiere#3458)
  Error in calculating quantity ordered or reserved for manufacturing order and distribution order  (adempiere#3472)
  Fixed error with missing parameter for Requisition PO Create (adempiere#3475)
  Fixed error with Dependent Entities for Record Access (adempiere#3480)
  Remove deprecated methods for Order (adempiere#3484)
  Fixed error when a Purchase order is voided (adempiere#3449)
  Update MPPForecastDefinitionLine.java
  adempiere#3469 [Bug Report] In the process Generate an Invoice from the Receipt is not validate if an Invoice was generated adempiere#3469 (adempiere#3470)
  adempiere#3467 [Bug Report] Generate Receipt from Invoice process is not validate if was generate a Material Receipt from a previous execution adempiere#3467 (adempiere#3468)
  adempiere#3462 [Bug Report] Total matched invoice qty adempiere#3462 (adempiere#3464)
  Fixed error with translation value for reference
  fix: BP Group getSQlWhere in Forecast Run Create.
  Fixed error with Crosstab for Smart Browser
  ...
@e-Evolution e-Evolution deleted the feature/3452 branch July 12, 2021 22:44
@e-Evolution e-Evolution self-assigned this Jan 25, 2023
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.

None yet

3 participants