Conversation
cea047a to
da2ea25
Compare
|
Rebased to reword some commit messages, |
27ab2cd to
979ca12
Compare
…state properties.
A simple counter component that increases when its button is clicked task-xxxxx
A basic card component with a title and a description task-xxxxx
… items task-xxxxx
979ca12 to
a10a151
Compare
dbeguin
left a comment
There was a problem hiding this comment.
I started a review but I see that there is still conflicts markers inside your commit, So I suggest to first get rid of them (edit commit by commit to see them) before I review the rest. :) ping me once done
again, if you have questions, feel free to ask :)
| @@ -1,3 +1,8 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
you can remove all the utf-8 header lines, not needed since we're in python3
| @@ -1,3 +1,8 @@ | |||
| # -*- coding: utf-8 -*- | |||
|
|
|||
| from . import res_users | |||
| _order = 'sequence' | ||
| _order = 'id desc' | ||
| _sql_constraints = [ | ||
| ('check_expected_price_positive', 'CHECK (0 < expected_price)', 'Check that the expected price is strictly positive'), |
There was a problem hiding this comment.
please fix your indentations (4 whitespaces per indent)
| garden = fields.Boolean('Garden') | ||
| garden_area = fields.Integer('Garden Area') | ||
| garden_orientation = fields.Selection(string='Garden Orientation', selection=[ | ||
| ('N', 'North'), |
There was a problem hiding this comment.
use lowercase for technical names, and be explicit, 'north' instead of 'N'. We always like explicit code more than super compacted code that is unreadable.
| status = fields.Selection(string='Status', selection=[ | ||
| ('accepted', 'Accepted'), | ||
| ('refused', 'Refused') | ||
| ], copy=False) |
There was a problem hiding this comment.
usually, we define selection field by giving directly the selection param without naming it.
status = fields.Selection([
('accepted', 'Accepted'),
('refused', 'Refused')
], string='Status', copy=False)
| @@ -0,0 +1,56 @@ | |||
| from odoo import api, exceptions, fields, models | |||
|
|
|||
There was a problem hiding this comment.
2 empty lines before class. apply this everywhere
| _name = 'estate.property.offer' | ||
| _description = 'estate property offer' | ||
| _order = 'price desc' | ||
| _sql_constraints = [ |
There was a problem hiding this comment.
_sql_constraints are put after field definitions
| ], copy=False) | ||
| partner_id = fields.Many2one('res.partner', string='Partner') | ||
| property_id = fields.Many2one('estate_property', string='Property') | ||
| validity = fields.Integer('Validity', default=7) |
There was a problem hiding this comment.
could be nice to add an helper (help='') to tell that validity is expressed in days.
or change validity field into validity_days
| @@ -0,0 +1,54 @@ | |||
| <?xml version="1.0"?> | |||
| <odoo> | |||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
you have conflicts markers here.
check other files
maybe you don't see in final diff by working commit by commit will display you those markers
Each commit should be clean and contain changes as if they were the last commit of your branch. Like what if we revert the commits after this one ?
Please kill me so I never have to do this again.
d486f83 to
2dcfb8f
Compare
|
|
||
| def _inverse_deadline(self): | ||
| for record in self: | ||
| delta = record.date_deadline - record.creation_date |
| from . import estate_property_type | ||
| from . import estate_property_tags | ||
| from . import estate_property_offer | ||
| from . import estate_property_tags |
There was a problem hiding this comment.
those changes should not be in that commit
| _name = 'estate_property' | ||
| _description = 'real estate property' | ||
| _order = 'id desc' | ||
| _sql_constraints = [ |
There was a problem hiding this comment.
_sql_constraints are placed after field definition (that's the only exception for private attributes)
| @@ -0,0 +1,109 @@ | |||
| from odoo import api, exceptions, fields, models | |||
|
|
|||
| selling_price = fields.Float('Selling Price', readonly=True, copy=False) | ||
| bedrooms = fields.Integer('Bedrooms', default=2) | ||
| living_area = fields.Integer('Living Area') | ||
| facades = fields.Integer('Facades') |
There was a problem hiding this comment.
usually when a field is basically a counter, you postfix with '_count'
because 'facade' only is not specific enough
|
|
||
| @api.onchange('garden') | ||
| def _set_garden_properties(self): | ||
| for record in self: |
There was a problem hiding this comment.
we try to be more explicit. record is too generic.
Here we are manipulating properties, so name it : for property in self.
apply this logic everywhere
| record.property_id.buyer = record.partner_id | ||
| @api.model | ||
| def create(self, vals): | ||
| estate_property = self.env['estate_property'].browse(vals["property_id"]) |
There was a problem hiding this comment.
will if property_id not in vals (well here it's a mandatory field so it should always be set, but that should not blow up by you code, but by the ORM check on required fields instead)
| @api.model | ||
| def create(self, vals): |
There was a problem hiding this comment.
create methods should be multi.
see 'create_multi' for examples
| if estate_property.state == 'new': | ||
| estate_property.state = 'offer_received' |
There was a problem hiding this comment.
I would do that after to write on every offer properties at the same time as this is the same value for all.
| offer_ids = fields.One2many('estate.property.offer', 'property_type_id', string='Offers') | ||
| offer_count = fields.Integer(string='Offer Count', compute='_count_offers') | ||
|
|
||
|
|
| def mark_cancelled(self): | ||
| for record in self: | ||
| if record.state == 'cancelled': | ||
| raise exceptions.UserError('This property is already cancelled.') |
There was a problem hiding this comment.
user errors should be translated. use _("yourtext") to allow translations

finished exercises