QUCOL Onboarding

[qucol-odoo]

No description provided.

[robodoo]

[alan-odoo]

Hi @qucol-odoo 👋,

Here is a review of your code. You went through the tutorials without much difficulty so I've been a little bit more touchy on your PR 😃.

Your branch is very clean! And we're not going to dirty it. Then can you do the corrections in the right commits ? In the end, your branch shouldn't contain new commit. If you meet some issue doing it, ping me 😃.

Thanks for your job and for helping your teammates!

[alan-odoo]

They should be listed alphabetically.

[alan-odoo]

That's the right way to write constants but I am not sure to understand why it is required. They are used only in _update_garden_area_and_orientation, can they be defined directly in that method?

[alan-odoo]

No need of that blank line 😃.

[alan-odoo]

As this method is used to compute the best_price, it should be named _compute_best_offer 😃, that way we know easily 1. it computes field and 2 which field.

FYI, _get or get methods should return an object and should not modify the database. For example, _get_res_user should only return a res_user and do nothing else.

[alan-odoo]

Same here.

[alan-odoo]

Suggested change

	<button name="%(estate.estate_property_offer_action)d" string="Offers" type="action" title="Offers list" invisible="not offer_count > 0"/>
	<button name="%(estate.estate_property_offer_action)d" string="Offers" type="action" title="Offers list" invisible="offer_count == 0"/>

[alan-odoo]

Maybe you can test to add it inside your oe_stat_button ? (not a priority). You can find a solution here (line 847)

[alan-odoo]

You can call it as its parent. Other modules will refer to this one as estate.res_user_view_form. So here we know that the main form of the user has been modified and this is not a new one.

Suggested change

	<record id="estate_res_user_view_form" model="ir.ui.view">
	<record id="res_user_view_form" model="ir.ui.view">

[alan-odoo]

No need of this blank line. Do not forget that a lot of developers are working on the project and we must avoid unnecessary changes.

[alan-odoo]

Do you want to select the first number missing in the sequence or the one above the maximum ?
If it is the last case, I would do something like that to avoid too many loops.

Suggested change

	taken_ids = set()
	for invoice in self.env["account.move"].search([]):
	if (match := re.match(r"Invoice\s+(\d+)", invoice.name)):
	taken_ids.add(int(match.group(1)))
	i = 1
	while i in taken_ids:
	i += 1
	return f"Invoice {i}"
	invoice = self.env["account.move"].search([("name", "like", "Invoice %")], order='id desc', limit=1)
	if invoice and (match := re.match(r"Invoice (\d+)", invoice.name)):
	return f"Invoice {match.group(1)}"
	return "Invoice 1"