[17.0][MIG] sale_rental by edescalona · Pull Request #52 · OCA/vertical-rental

edescalona · 2024-11-27T18:32:46Z

Some minor changes were made to it.

rousseldenis · 2024-12-04T15:32:05Z

/ocabot migration sale_rental

remi-filament

Thanks @edescalona for porting this module.
I discovered it with your PR, it seems to be working fine.
However, display on products and sale order could be improved !

sale_rental/views/product.xml

sale_rental/views/sale_rental.xml

edescalona · 2025-02-05T16:02:55Z

Thanks @edescalona for porting this module. I discovered it with your PR, it seems to be working fine. However, display on products and sale order could be improved !

Hi @remi-filament thank you for your comments, I will review them and comment on them, greetings

sale_rental/views/sale_rental.xml

edescalona · 2025-05-15T14:26:11Z

Hello @remi-filament , ready for review, thanks.

remi-filament

Thanks for your work on that module !

I made a first review, and I think there are some improvements to be made to that module, I made some suggestions inline (for aligning UI with the rest of Odoo interface, for handling multi-company properly), documentation also would need to be updated.

Regarding PR process, you need to clean up bot commits as specified here : https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0 :
"Squash administrative commits (if any) with the previous commit for reducing commit noise. Check https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate for details."

sale_rental/__manifest__.py

sale_rental/README.rst

sale_rental/data/rental_data.xml

remi-filament · 2025-06-17T07:52:33Z

sale_rental/data/rental_data.xml

+<record id="stock.warehouse0" model="stock.warehouse">
+    <field name="rental_allowed" eval="True" />
+</record>


I do not think we should do that in data, maybe in demo data rather.
When installing the module on existing DB with multiple companies or warehouses, we do not know on which one they want to allow rental, so it is probably better if this is done manually and added to configuration part of readme.

remi-filament · 2025-06-17T08:03:58Z

sale_rental/models/stock_warehouse.py

+        if "rental_allowed" in vals:
+            rental_route = self.env.ref("sale_rental.route_warehouse0_rental")
+            sell_rented_route = self.env.ref(
+                "sale_rental.route_warehouse0_sell_rented_product"
+            )
+            if vals.get("rental_allowed"):
+                self._create_rental_locations()
+                self.write(
+                    {
+                        "route_ids": [(4, rental_route.id)],
+                        "rental_route_id": rental_route.id,
+                        "sell_rented_product_route_id": sell_rented_route.id,
+                    }
+                )
+                rental_rules = self.env["stock.rule"].search(
+                    [
+                        ("route_id", "in", [rental_route.id, sell_rented_route.id]),
+                        ("active", "=", False),
+                    ]
+                )
+                if rental_rules:
+                    rental_rules.write({"active": True})
+                else:
+                    for rule_vals in self._get_rental_push_pull_rules():
+                        self.env["stock.rule"].create(rule_vals)
+            else:
+                for wh in self:
+                    rules_to_archive = self.env["stock.rule"].search(
+                        [
+                            (
+                                "route_id",
+                                "in",
+                                (
+                                    wh.rental_route_id.id,
+                                    wh.sell_rented_product_route_id.id,
+                                ),
+                            ),
+                            ("company_id", "=", wh.company_id.id),
+                        ]
+                    )
+                    rules_to_archive.write({"active": False})
+                    wh.write(
+                        {
+                            "route_ids": [(3, rental_route.id)],
+                            "rental_route_id": False,
+                            "sell_rented_product_route_id": False,
+                        }
+                    )


This should be done in function create as well.
Otherwise if you create a new warehouse, setting rental_allowed to true the locations / rules will not be created and routes will not be assigned.

remi-filament · 2025-06-17T08:20:46Z

sale_rental/views/product.xml

+            <group name="sale" position="after">
+                <group
+                    name="rental"
+                    string="Rental"
+                    invisible="detailed_type != 'service'"
+                >
+                    <field name="rented_product_id" />
+                </group>
+                <group
+                    string="Rental Services"
+                    name="rental_services"
+                    invisible="detailed_type == 'service'"
+                >
+                    <button
+                        colspan="2"
+                        type="action"
+                        name="%(sale_rental.create_rental_product_action)d"
+                        string="Create Rental Service"
+                    />
+                    <field
+                        colspan="2"
+                        name="rental_service_ids"
+                        nolabel="1"
+                        readonly="1"
+                        invisible="rental_service_ids == []"
+                    />
+                </group>
+            </group>


I suggest to add a page specific for Rental and to add cols and colspan for avoiding getting a full width button.

Suggested change

<group name="sale" position="after">

<group

name="rental"

string="Rental"

invisible="detailed_type != 'service'"

>

<field name="rented_product_id" />

</group>

<group

string="Rental Services"

name="rental_services"

invisible="detailed_type == 'service'"

>

<button

colspan="2"

type="action"

name="%(sale_rental.create_rental_product_action)d"

string="Create Rental Service"

/>

<field

colspan="2"

name="rental_service_ids"

nolabel="1"

readonly="1"

invisible="rental_service_ids == []"

/>

</group>

</group>

<page name="sales" position="after">

<page

name="rental"

string="Rental"

invisible="not sale_ok and detailed_type=='service'"

>

<group>

<group

name="rental_product"

invisible="detailed_type != 'service'"

>

<field name="rented_product_id" />

</group>

<group

name="rental_services"

string="Rental Services"

invisible="detailed_type == 'service'"

col="1"

colspan="3"

>

<field

colspan="2"

name="rental_service_ids"

nolabel="1"

readonly="1"

invisible="rental_service_ids == []"

/>

<button

colspan="2"

type="action"

name="%(sale_rental.create_rental_product_action)d"

string="Create Rental Service"

/>

</group>

</group>

</page>

</page>

remi-filament · 2025-06-17T08:23:01Z

sale_rental/views/product.xml

+            <group name="sale" position="inside">
+                <label
+                    for="rented_product_tmpl_id"
+                    string="Rental"
+                    invisible="detailed_type != 'service' or product_variant_count &gt; 1"
+                />
+                <field
+                    name="rented_product_tmpl_id"
+                    invisible="detailed_type != 'service' or product_variant_count &gt; 1"
+                />
+                <label
+                    for="rental_service_tmpl_ids"
+                    string="Rental Services"
+                    invisible="detailed_type == 'service' or product_variant_count &gt; 1"
+                />
+                <field
+                    name="rental_service_tmpl_ids"
+                    nolabel="1"
+                    invisible="rental_service_tmpl_ids == []"
+                />
+                <button
+                    type="action"
+                    name="%(sale_rental.create_rental_product_action)d"
+                    string="Create Rental Service"
+                    invisible="detailed_type == 'service' or product_variant_count &gt; 1"
+                />
+            </group>


Same here :

Suggested change

<group name="sale" position="inside">

<label

for="rented_product_tmpl_id"

string="Rental"

invisible="detailed_type != 'service' or product_variant_count > 1"

/>

<field

name="rented_product_tmpl_id"

invisible="detailed_type != 'service' or product_variant_count > 1"

/>

<label

for="rental_service_tmpl_ids"

string="Rental Services"

invisible="detailed_type == 'service' or product_variant_count > 1"

/>

<field

name="rental_service_tmpl_ids"

nolabel="1"

invisible="rental_service_tmpl_ids == []"

/>

<button

type="action"

name="%(sale_rental.create_rental_product_action)d"

string="Create Rental Service"

invisible="detailed_type == 'service' or product_variant_count > 1"

/>

</group>

<page name="sales" position="after">

<page

name="rental"

string="Rental"

invisible="not sale_ok and detailed_type=='service'"

>

<group>

<group

name="rental-product"

invisible="detailed_type != 'service' or product_variant_count > 1"

>

<field name="rented_product_tmpl_id" />

</group>

<group

name="rental-services"

string="Rental Services"

col="1"

colspan="3"

invisible="detailed_type == 'service' or product_variant_count > 1"

>

<field

nolabel="1"

name="rental_service_tmpl_ids"

invisible="not rental_service_tmpl_ids"

>

<tree create="0" delete="0">

<field name="name" />

<field

name="list_price"

string="Rental Price per Day"

/>

</tree>

</field>

<button

type="action"

name="%(sale_rental.create_rental_product_action)d"

string="Create Rental Service"

colspan="3"

/>

</group>

</group>

</page>

</page>

edescalona · 2025-06-18T06:28:59Z

Hi @remi-filament , I'll take your comments into account, but this is a PR for the migration itself. It would be better to add the improvements you mentioned to a separate PR so as not to change the original format. Thanks for your feedback.

chaule97

LGTM

Replace openerp by odoo in import declarations

…ot writeable

Convert readme to new subdir format

the creation of the stock location for rental respects the translations

No more need to add users to the packaging group View of rental group is now possible on product.template form view

Standard migration + add some tests

edescalona · 2026-02-27T19:24:22Z

Hi @remi-filament , I've corrected some of your suggestions. I think the rest should be addressed in a pull request, unless @rousseldenis thinks they can be done in this same pull request? I also cleaned up the commit history a bit. Let me know what you think. Thanks.

edescalona mentioned this pull request Nov 27, 2024

[17.0][MIG] sale_rental OCA/sale-workflow#3435

Closed

rousseldenis added needs review migration labels Dec 4, 2024

OCA-git-bot added this to the 17.0 milestone Dec 4, 2024

OCA-git-bot mentioned this pull request Dec 4, 2024

Migration to version 17.0 #35

Open

1 task

remi-filament suggested changes Feb 4, 2025

View reviewed changes

sale_rental/views/product.xml Outdated Show resolved Hide resolved

sale_rental/views/product.xml Show resolved Hide resolved

sale_rental/views/sale_rental.xml Outdated Show resolved Hide resolved

remi-filament reviewed Apr 8, 2025

View reviewed changes

sale_rental/views/sale_rental.xml Show resolved Hide resolved

edescalona force-pushed the 17.0-mig-sale_rental branch 2 times, most recently from 8342b4a to 01a286b Compare May 15, 2025 14:23

edescalona requested a review from remi-filament June 16, 2025 17:14

remi-filament suggested changes Jun 17, 2025

View reviewed changes

chaule97 approved these changes Oct 13, 2025

View reviewed changes

hannes386 mentioned this pull request Nov 3, 2025

[18.0][MIG] sale_rental #70

Open

This was referenced Nov 6, 2025

[18.0][MIG] sale_rental: Migration to 18.0 chaule97/vertical-rental#7

Open

[18.0][MIG] sale_rental: Migration to 18.0 #71

Open

edescalona requested a review from remi-filament February 27, 2026 13:25

Alexis de Lattre and others added 3 commits February 27, 2026 11:54

Port sale_start_end_dates and sale_rental (#353)

a3ac249

sale_start_end_dates and sale_rental

fb3cab1

Product qty on rental pickings

97e1ec7

Replace openerp by odoo in import declarations

alexis-via and others added 7 commits February 27, 2026 11:54

sale_rental: fix warning @constrains parameter 'must_have_dates' is n…

2ebbdb8

…ot writeable

pylint

f9b53ef

sale_rental: Migration to 11.0

277baea

[MIG] sale_start_end_dates and sale_rental to v12

5c219c4

Convert readme to new subdir format

sale_rental: Improve the code of object sale.rental

a697a5c

[IMP] enhancements suggested in code reviews

4449079

the creation of the stock location for rental respects the translations

[FIX] Archive rental routes rather than delete

7028e0a

edescalona force-pushed the 17.0-mig-sale_rental branch from 01a286b to 9657bf3 Compare February 27, 2026 17:37

OCA-git-bot and others added 20 commits February 27, 2026 12:52

[UPD] README.rst

71e048f

[UPD] Update sale_rental.pot

02f0d33

[IMP] sale_rental: extract function _run_rental_procurement

3b18f97

sale_rental 12.0.1.1.0

1fb647f

[MIG] sale_rental from v12 to v14

fe63ffd

No more need to add users to the packaging group View of rental group is now possible on product.template form view

sale_rental: black, isort, etc.

5a62274

[UPD] README.rst

b6e1ffd

[MIG] sale_rental: Migration to 15.0

e864a99

Standard migration + add some tests

[IMP] : black, isort, prettier, pylint

3eb4096

[UPD] Update sale_rental.pot

3ac4a51

[UPD] README.rst

b1b69d7

sale_rental 15.0.1.0.1

1a9775a

[UPD] Update sale_rental.pot

f5e8024

[16.0][MIG] sale_rental

e3c18b5

[IMP] sale_rental: black, isort, prettier

afbf1bd

[FIX] rework unit test

e415f04

[IMP] sale_rental: black, isort, prettier

171cb83

[BOT] post-merge updates

4241cc3

[IMP] sale_rental: pre-commit auto fixes

54b0d64

[MIG] sale_rental: Migration to 17.0

3152d32

edescalona force-pushed the 17.0-mig-sale_rental branch from 9657bf3 to 3152d32 Compare February 27, 2026 19:20

Uh oh!

Conversation

edescalona commented Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rousseldenis commented Dec 4, 2024

Uh oh!

remi-filament left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edescalona commented Feb 5, 2025

Uh oh!

Uh oh!

edescalona commented May 15, 2025

Uh oh!

remi-filament left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

remi-filament Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

remi-filament Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

remi-filament Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

remi-filament Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

edescalona commented Jun 18, 2025

Uh oh!

chaule97 left a comment

Choose a reason for hiding this comment

Uh oh!

edescalona commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

edescalona commented Nov 27, 2024 •

edited

Loading