Skip to content

Conversation

@BhaveshHeliconia
Copy link
Contributor

No description provided.

@BhaveshHeliconia BhaveshHeliconia changed the title 18.0][MIG] sale_margin_delivered_dropshipping: Migration to 18.0 [18.0][MIG] sale_margin_delivered_dropshipping: Migration to 18.0 Feb 17, 2025
@BhaveshHeliconia BhaveshHeliconia force-pushed the 18.0-mig-sale_margin_delivered_dropshipping branch from 6e30aac to 7e67989 Compare February 17, 2025 10:30
@BhaveshHeliconia BhaveshHeliconia force-pushed the 18.0-mig-sale_margin_delivered_dropshipping branch 2 times, most recently from 6abedd7 to 0759841 Compare May 9, 2025 09:50
@BhaveshHeliconia BhaveshHeliconia force-pushed the 18.0-mig-sale_margin_delivered_dropshipping branch 4 times, most recently from 04d5fe1 to 41bfc44 Compare July 28, 2025 10:17
@EmilioPascual
Copy link

@BhaveshHeliconia could you rebase for run tests and runboat again to check the PR? Thank you.

@BhaveshHeliconia BhaveshHeliconia force-pushed the 18.0-mig-sale_margin_delivered_dropshipping branch from 41bfc44 to 51fe6d8 Compare November 12, 2025 03:42
@BhaveshHeliconia BhaveshHeliconia force-pushed the 18.0-mig-sale_margin_delivered_dropshipping branch from 51fe6d8 to 419026e Compare November 12, 2025 05:45
@BhaveshHeliconia
Copy link
Contributor Author

@EmilioPascual : CI is green and Runboat is up now — ready for review. Thanks!

Copy link

@EmilioPascual EmilioPascual left a comment

Choose a reason for hiding this comment

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

Copy link

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

I have executed the basic functional tests for this module.

Tests performed:

  • Test 1: Correct delivered margin calculation in a simple dropshipping scenario with full delivery.

Result: OK.

  • Test 2: Dropshipping with partial return.

Result: Not OK.
The value shown in Margin dlvd. does not match the expected calculation based on the net delivered quantity.

Reproduction steps for the incorrect test (Test 2):

  1. Create a Sales Order with a dropshipping-configured product (cost 10 €, price 25 €) and a quantity of 6 units.
  2. Confirm the order and validate the dropshipping delivery of the 6 units.
  3. Create a dropshipping return for 2 units using the To Refund option.
  4. Open the sales order line and review the Margin dlvd. field.
Image Image Image

Observed behavior:
The delivered margin shown is 100 €, while the expected value should be 60 € (4 net delivered units × 15 € margin per unit).

@BhaveshHeliconia Please review the delivered margin calculation for dropshipping scenarios with returns, as the current result is not correct.

@EmilioPascual
Copy link

@Gelojr I recreate the test case that you mentioned is wrong, but I have obteined different result.
image

Can you review the test case or perhaps provide more information about the data used? Thank you.

Copy link

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

I have continued the functional testing of the module.
Test 1 and Test 2 are now both OK.

However, Test 3 reveals an issue that must be reviewed, specifically in a mixed scenario where part of the delivery comes from on-hand stock and part from dropshipping.

Below are the full reproduction steps and the behavior observed.

Test 3 (Mixed: stock delivery + dropshipping) — NOT OK

Reproduction steps:

  1. Create product. In this case I have assigned it to a product category configured with AVCO as the cost method. Configure the product with the Dropship route enabled. Price: 25$ Purchase price: 10$/unit

  2. Simulate a situation where 2 units are already available in the warehouse:

  • Purchase 2 units of this product. 10$/unit
  • Receive them into stock.
  1. Create a Sales Order for 5 units of this product.
  2. The system generates a Purchase Order for dropshipping to buy 5 units ; since 2 units are already in stock, change quantity and purchase only 3 units.
  3. Validate the dropshipping picking for those 3 units.
  4. The Sales Order correctly shows Delivered = 3 units, and both Odoo’s margin and the module’s delivered margin are correct at this point.
  5. From the Sales Order, open the Pickings smart button.
    8 Manually create a new picking of type Delivery Order and deliver the 2 units available in stock.
Image 9. After validating this picking, a new line is added to the Sales Order. Image In standard Odoo I have performed the same flow, and that line is not added to the sales order, and the delivery note is not linked to the sales order.

Observed issue:

  • The margins for this new line are not calculated correctly.
  • The standard Odoo margin, and margings calculated by this module are incorrect for the stock-based delivery line created after the dropshipping delivery.
Image

This indicates that the mixed scenario (dropshipping first, stock delivery afterwards) is not being handled properly, and the delivered margin calculation needs to be reviewed for these cases.

Request
Please review the behavior of the margin and delivered margin computation when a Sales Order contains multiple delivery flows (dropshipping + standard delivery), especially when the stock delivery is created manually after the dropshipping picking has already been processed.

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

@Gelojr

  1. From the Sales Order, open the Pickings smart button.
  2. Manually create a new picking of type Delivery Order and deliver the 2 units available in stock.
  3. After validating this picking, a new line is added to the Sales Order.

That's quite an strange workflow, don't you think?

In standard Odoo I have performed the same flow, and that line is not added to the sales order, and the delivery note is not linked to the sales order.

I don't think that behavior has anything to do with this module...

Copy link

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

Congratulations on the work done on this module.

The following functional tests have been executed and reviewed.

  • Test 1: Simple dropshipping scenario with full delivery. The delivered margin and delivered margin percentage are calculated correctly. Result OK.
  • Test 2: Dropshipping with partial return. The delivered margin is correctly recalculated based on the net delivered quantity after the return. Result OK.
  • Test 3: Mixed delivery scenario (dropshipping first, stock delivery afterwards). The test was executed and the behavior was reported. This scenario has been confirmed as out of scope and therefore is not covered by the module design.
  • Test 4: Partial dropshipping delivery without completing the full quantity. The delivered margin reflects only the delivered units. Result OK.
  • Test 5: Not performed. This test involves mixed delivery scenarios, which are explicitly not contemplated by this module.

Note: Mixed delivery scenarios combining dropshipping and standard stock deliveries for the same product are not supported by this module, as clarified during the review.

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

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.

8 participants