[MIG] [17.0] hotel

[Rajan-SCS]

Migrated By RaishKhan Pathan

[Trivedi-Vacha-SerpentCS]

Define readonly in view

[Trivedi-Vacha-SerpentCS]

Define readonly in view instead of model

[Trivedi-Vacha-SerpentCS]

Suggested change

	"author": "Odoo Community Association (OCA), Serpent Consulting \
	"author": "Odoo Community Association (OCA),
	Serpent Consulting Services Pvt. Ltd.,

[Trivedi-Vacha-SerpentCS]

Suggested change

	Services Pvt. Ltd., OpenERP SA",
	OpenERP SA",

[Trivedi-Vacha-SerpentCS]

Add Docstring
"Override the create method to link the account move with a hotel folio"

[Trivedi-Vacha-SerpentCS]

You have added comment instead of docstring:
Docstring is the method description defined inside the method between triple quote in case of multiline

@api.model
def create(self, vals):
    "Override the create method to link the account move with a hotel folio"
     

[Trivedi-Vacha-SerpentCS]

Suggested change

	if not "service_line_ids" and "folio_id" in vals:
	if "service_line_ids" not in vals and "folio_id" in vals:

[Rad0van]

This change completely alters the behavior. The original precedence would be like this:
(not "service_line_ids") and ("folio_id" in vals)
which would always evaluate to False (because of not "service_line_ids")

If this fixes incorrect behavior shouldn't that rather be a PR with fixing that in an existing branch that would be propagated to other branches and possibly cherry-picked in this PR?

However the code does not make much sense. This is inside create method of hotel.folio model and there is no such attribute as folio_id in that model. So what is actually the intention of that code? Was it to check presence of folio_id in service_line_ids ? But then see remarks regarding create method signature.

Also the code after the else is pretty funny. It only handles the situation when vals is empty. If it would not be it will end up with nasty exception of using folio_id before initialization.

Also the whole method signature:

    @api.model
    def create(self, vals):
        """
        Overrides orm create method.
        @param self: The object pointer
        @param vals: dictionary of fields value.
        @return: new record set for hotel folio.
        """

Where is this coming from? The create method had @api.model in 11.0 Since 12.0 it is something like:

    @api.model_create_multi
    def create(self, vals_list):

When @api.model or @api.model_create_multi is used, the parameter self has no meaning at all. Certainly not a pointer to object. The @api.model on create method will end up being @api.model_create_single which in turn will result in logger warning of not overriding the create method in batch - see https://github.com/odoo/odoo/blob/f01b55636fecf2193ea4bd7551b5daef5bfbb12a/odoo/api.py#L369-L379 and https://github.com/odoo/odoo/blob/f01b55636fecf2193ea4bd7551b5daef5bfbb12a/odoo/api.py#L397-L404

[Trivedi-Vacha-SerpentCS]

use variable name "room" instead of "rm" for better readability

[Trivedi-Vacha-SerpentCS]

Suggested change

	room_obj = hotel_room_obj.search([("product_id", "=", rm.id)])
	room_obj = hotel_room_obj.search([("product_id", "=", rm.id)], limit=1)

[Trivedi-Vacha-SerpentCS]

define self.env at first line on method outside for loop

[Trivedi-Vacha-SerpentCS]

define all object at starting of the method

[Trivedi-Vacha-SerpentCS]

You have added comment instead of docstring:
Docstring is the method description defined inside the method between triple quote in case of multiline

@api.model
def create(self, vals):
    "Override the create method to link the account move with a hotel folio"
     

[Trivedi-Vacha-SerpentCS]

Why removed the required=True?

[Trivedi-Vacha-SerpentCS]

Are you sure its working?