[18.0][MIG] sale_rental: Migration to 18.0

[chaule97]

• I built my migration on top of: [17.0][MIG] sale_rental #52
• Took into account work from @hannes386 in [18.0][MIG] sale_rental #70
• the PRs I included in that were not yet merged in 16.0:
  [16.0][FIX] sale_rental: fix create a duplicate sale.rental when reconfirming #64
  [16.0][IMP] sale_rental: Notification of rental expiration and scheduled return date #65
  [16.0][IMP] sale_rental: Price list in different unit of measures #66
  [16.0][IMP] sale_rental: Add new calendar view #67
  [16.0][IMP] sale_rental: Add new graph view #68
  [16.0][IMP] sale_rental: Rental utilization report #69

[kaynnan]

@chaule97 Could you please do a rebase?

[chaule97]

Could you please do a rebase?

the PR is waiting for PR OCA/sale-workflow#3505, so the first failing test is expected. It's no merge conflict

[remi-filament]

Hi @chaule97 Thank you for migrating this module.
Tested on runboat : rental_qty is not displayed on sale order line and is not set raising a constraint error when checking that product_uom_id = rental_qty * number_of_days

[chaule97]

Hi @chaule97 Thank you for migrating this module. Tested on runboat : rental_qty is not displayed on sale order line and is not set raising a constraint error when checking that product_uom_id = rental_qty * number_of_days

Can you check again?

[remi-filament]

Hi @chaule97 Thank you for migrating this module. Tested on runboat : rental_qty is not displayed on sale order line and is not set raising a constraint error when checking that product_uom_id = rental_qty * number_of_days

Can you check again?

Hi @chaule97 thank you for looking into iti.
I still have the same behavior : (Pricing hint is not updated, rental quantity not shown (so cannot change from 0), quantity shown instead which is not taking into account number of days * rental quantity

Leading to Validation Error when trying to create the sale order :

[remi-filament]

Thanks @chaule97 now that part is not raising errors anymore and you can properly set rental quantity.

I spotted another issue with version 18.0 of this module : it does not generate return picking at the end of rental. This is related to how routes/rules are applied in version 18.0, push rules are not automatically triggered anymore.

Also, although less important, UI/UX of this module should be reworked to get views coherent with the other modules (for instance product form views have been kept from old version but have not been adapted to newer versions).
I made a lot of comments on these topics on version 17.0 migration PR : #52 (review)

FYI, I have made a simplified version of this module for version 18.0 for a customer who had simpler requirements than what this module covers (no need for autocompute of duration / quantity, no need for rental prices and periods, etc.) here : https://github.com/lefilament/vertical-rental/tree/18.0-mecaoctet (this branch is in production for that customer)

In particular, you may want to have a look here on how to force to apply push rule for generating return picking : https://github.com/lefilament/vertical-rental/blob/ac1537c29cae3cbe0e62af38da1a14a26de3ecf5/sale_rental/models/sale_order_line.py#L183-L184

Also you would need to fix the failing tests

[chaule97]

Hi @chaule97 Thank you for migrating this module. Tested on runboat : rental_qty is not displayed on sale order line and is not set raising a constraint error when checking that product_uom_id = rental_qty * number_of_days

Can you check again?

Hi @chaule97 thank you for looking into iti. I still have the same behavior : (Pricing hint is not updated, rental quantity not shown (so cannot change from 0), quantity shown instead which is not taking into account number of days * rental quantity

Leading to Validation Error when trying to create the sale order :

Sorry, I forgot about my fix in version 16.0, can you check again?

[Kev-Roche]

Thank you for this migration.

I do not understand the benefit of adding "rental period" and "product rental pricing" logic. Using pricelists and configuring some units of measure seems to be enough. Maybe I'm missing something... Maybe, if "product rental pricing" and "rental period" have a clear use case, putting them in a separate module since they are not required for this migration.

IMHO, this module is already complex, it would be better to split new features that are not relevant to all projects into separate modules, as I did in v16 here: #36 and #37 (I will migrate them soon).

Also, if we introduce new features, we need tests for them.

[Kev-Roche]

Why introducing a new model "product.rental.pricing" and not using the existing pricelist.item with unit of mesure logic ?

[Kev-Roche]

It is disturbing to see this menu between quotations and sales. Maybe put it in sale analysis menu or at least add a sequence to put it under locations within the same menu.