-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lezgo #360
base: 18.0
Are you sure you want to change the base?
Lezgo #360
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job for the work! You've already done a lot for the first day. Keep going 😄
estate/__init__.py
Outdated
# -*- coding: utf-8 -*- | ||
# Part of Odoo. See LICENSE file for full copyright and licensing details. | ||
|
||
#from . import controllers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for all this 😃
the coding: utf-8 isn't necessary since a few versions
estate/__init__.py
Outdated
#from . import controllers | ||
from . import models | ||
|
||
from odoo import api, SUPERUSER_ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the right place for this line of code
estate/models/__init__.py
Outdated
from . import estate_recurring_plan | ||
from . import estate_property | ||
from . import estate_property_type | ||
from . import estate_property_tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some of your file are missing an empty line at the bottom. You can check on the PR the files with a warning at the bottom to see which ones.
estate/models/estate_property.py
Outdated
state = fields.Selection( | ||
string='State', | ||
selection = [('new', "new"), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', 'Cancelled')], | ||
default='new' | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
state = fields.Selection( | |
string='State', | |
selection = [('new', "new"), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', 'Cancelled')], | |
default='new' | |
) | |
state = fields.Selection( | |
string='State', | |
selection = [ | |
('new', "new"), | |
('offer_received', 'Offer Received'), | |
('offer_accepted', 'Offer Accepted'), | |
('sold', 'Sold'), | |
('cancelled', 'Cancelled') | |
], | |
default='new', | |
) |
just styling preference. More globally, don't hesitate to give space to your code.
Also don't forget the coma on the last line, because if someone one day must add another attibute to the field, he'll have to add the coma, which will force him to modify your line of code, therefore changing the git history of this line.
<field name="model">estate.property.type</field> | ||
<field name="arch" type="xml"> | ||
<form string="Property Types"> | ||
<h1 class="mb32"><field name="name"/></h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does the class mb32 do in runbot ? Not sure it exists
|
||
<separator/> | ||
|
||
<filter string="Available" name="available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<filter string="Available" name="available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/> | |
<filter string="Available" name="available" domain="[('state', 'in', ('new', 'offer_received'))]"/> |
a bit more concise
estate/models/estate_property.py
Outdated
from odoo import fields, models | ||
from dateutil.relativedelta import relativedelta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imports must be ordered according to https://www.odoo.com/documentation/18.0/contributing/development/coding_guidelines.html#imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noob.
(hahaha 😄)
@@ -0,0 +1,17 @@ | |||
# -*- coding: utf-8 -*- | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No licence ?
estate/models/__init__.py
Outdated
from . import estate_property | ||
from . import estate_property_type | ||
from . import estate_property_tag | ||
from . import estate_property_offer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing empty line at end of file.
estate/models/estate_property.py
Outdated
|
||
# Metadata | ||
name = fields.Char('Title', required=True, translate=True, default="Best House In Town") | ||
active = fields.Boolean('Active', default=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, the label will be the name of the field capitalize. In this case, it will be Active
so there is no need to define it.
estate/models/estate_property.py
Outdated
garden_area = fields.Integer('Garden Area (sqm)') | ||
garden_orientation = fields.Selection( | ||
string='Garden Orientation', | ||
selection = [('north', "North"), ('south', 'South'), ('east', 'East'), ('west', 'West')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selection = [('north', "North"), ('south', 'South'), ('east', 'East'), ('west', 'West')] | |
selection = [ | |
('north', "North"), | |
('south', 'South'), | |
('east', 'East'), | |
('west', 'West'), | |
] |
This is better. We can add values without breaking the git history.
@api.depends("property_offer_ids") | ||
def _compute_best_price(self): | ||
for record in self: | ||
record.best_price = max(record.property_offer_ids.mapped('price')) if record.property_offer_ids else 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
record.best_price = max(record.property_offer_ids.mapped('price')) if record.property_offer_ids else 0.0 | |
record.best_price = max(record.property_offer_ids.mapped('price') + [0]) |
estate/models/estate_property.py
Outdated
if self.state == 'sold': | ||
raise UserError("You cannot cancel a sold property") | ||
elif self.state == 'cancelled': | ||
raise UserError("Property already cancelled") | ||
else: | ||
self.state = 'cancelled' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if self.state == 'sold': | |
raise UserError("You cannot cancel a sold property") | |
elif self.state == 'cancelled': | |
raise UserError("Property already cancelled") | |
else: | |
self.state = 'cancelled' | |
if self.state == 'sold': | |
raise UserError("You cannot cancel a sold property") | |
if self.state == 'cancelled': | |
raise UserError("Property already cancelled") | |
self.state = 'cancelled' |
if self.status == 'accepted': | ||
raise UserError('Offer already accepted') | ||
else: self.property_id.action_accept_offer(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if self.status == 'accepted': | |
raise UserError('Offer already accepted') | |
else: self.property_id.action_accept_offer(self) | |
if self.status == 'accepted': | |
raise UserError('Offer already accepted') | |
self.property_id.action_accept_offer(self) |
<field name="model">estate.property.tag</field> | ||
<field name="arch" type="xml"> | ||
<form string="Property tags"> | ||
<h1 class="mb32"><field name="name"/></h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's mb32
?
|
||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra spaceeeee.
<div class="row"> | ||
<div class="col"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure we don't need this.
ccbce2e
to
38d0a93
Compare
No description provided.