-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[ADD] estate: module with property model and corresponding views. (framework 101) #1082
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
base: 19.0
Are you sure you want to change the base?
Conversation
|
Hello, could you please make one commit per chapter and also ensures that the runbot is green 😄 |
|
For you pr message, you don't have to specify all the technical part. The title is what it is done and the message is "why". You should try to explain the functional part of the feature or fix, ... So for instance, Adding a new module estate to better manage properties, ... You can add offers to a property as well as addings tags, ... Something like this. |
lost-odoo
left a comment
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 already made a small review.
Be careful with the final new lines, spacing and double quotes vs single quotes 😄
| 'version': '1.0', | ||
| 'category': 'Sales', | ||
| 'sequence': 1, | ||
| 'summary': 'Sell and bid on the hottest real estate properties.', |
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.
You could explain that in your pr message for instance
estate/models/estate_property.py
Outdated
| selection =[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), | ||
| ('sold', 'Sold'), ('canceled', 'Canceled')], |
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.
You can expand this into multiple lines.
fa19da5 to
e5a923c
Compare
| 'summary': 'Sell and bid on the hottest real estate properties.', | ||
| 'website': 'https://www.odoo.com/app/estate', | ||
| 'depends': [ | ||
| 'base_setup', |
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.
Shouldn't you use base here ?
| state = fields.Selection( | ||
| string="Status", | ||
| selection=[('new', "New"), ('offer_received', "Offer Received"), ('offer_accepted', "Offer Accepted"), ('sold', "Sold"), ('canceled', "Canceled")], | ||
| required=True, | ||
| copy=False, | ||
| 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.
Don't split your fields definitions. You should move this up with the rest of them 😄
| active = fields.Boolean("Active", default=True) | ||
| buyer_id = fields.Many2one('res.partner', string="Buyer", copy=False) | ||
| seller_id = fields.Many2one('res.users', string="Salesperson", default=lambda self: self.env.user) | ||
| tag_ids = fields.Many2many( | ||
| 'estate.property.tag', | ||
| string="Tags", | ||
| ) | ||
| offer_ids = fields.One2many( | ||
| 'estate.property.offer', | ||
| 'property_id', | ||
| string="Offers", | ||
| ) | ||
| best_offer = fields.Monetary( | ||
| string="Best Offer", | ||
| compute='_compute_best_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.
Here too, the format should be fields/constraints > create/write/unlink/compute/onchange > the rest.
| ) | ||
|
|
||
| def sold_action(self): | ||
| for record in 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.
Try to use a meaningful name instead of record -> property
| if record.state != 'canceled': | ||
| record.state = 'sold' | ||
| else: | ||
| raise UserError(record.env._("You can not sell a canceled property.")) |
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.
FYI here you could use self.env._ but that's good also 😄
| def create(self, vals_list): | ||
| for vals in vals_list: | ||
| if 'property_id' in vals and 'price' in vals: | ||
| linked_property = self.env['estate.property'].browse(vals['property_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.
don't browse in a loop, try to do it outside the loop so it is more efficient otherwise you would have to query record one by one instead of all of them at once
| @api.model_create_multi | ||
| def create(self, vals_list): | ||
| for vals in vals_list: | ||
| if 'property_id' in vals and 'price' in vals: |
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.
Shouldn't you make property_id and price required ?
| for vals in vals_list: | ||
| if 'property_id' in vals and 'price' in vals: | ||
| linked_property = self.env['estate.property'].browse(vals['property_id']) | ||
| if linked_property.best_offer and vals['price'] < linked_property.best_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.
Here too use the currency compare or float_compare 😄
| ) | ||
| _name_uniq = models.Constraint( | ||
| 'unique(name)', | ||
| 'This property tag already exists.', |
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.
Double quotes here 😄
Also don't forget about the splitting, try to put fields together
| # Exclude a variety of commonly ignored directories. | ||
| exclude = [ | ||
| ".bzr", | ||
| ".direnv", | ||
| ".eggs", | ||
| ".git", | ||
| ".git-rewrite", | ||
| ".hg", | ||
| ".ipynb_checkpoints", | ||
| ".mypy_cache", | ||
| ".nox", | ||
| ".pants.d", | ||
| ".pyenv", | ||
| ".pytest_cache", | ||
| ".pytype", | ||
| ".ruff_cache", | ||
| ".svn", | ||
| ".tox", | ||
| ".venv", | ||
| ".vscode", | ||
| "__pypackages__", | ||
| "_build", | ||
| "buck-out", | ||
| "build", | ||
| "dist", | ||
| "node_modules", | ||
| "site-packages", | ||
| "venv", | ||
| ] | ||
|
|
||
| # Assume Python 3.12 | ||
| target-version = "py312" | ||
|
|
||
| line-length = 255 | ||
| [lint] | ||
| # Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`) codes by default. | ||
| select = ["ALL"] | ||
| ignore = ["A","ARG","ANN","B","C901","D","DTZ","DOC","E501","ERA001","FBT","N","PD","PERF","PIE790","PLR","PT","Q","RSE102","RUF001","RUF012","S","SIM102","SIM108","SLF001","TID252","UP031","TRY003","TRY300","E713","SIM117","PGH003","RUF005","FIX","TD","TRY400","C408","PLW2901","PTH","EM102","INP001","CPY001","E266","PIE808","PLC2701","RUF100","FA100","FURB","C420","COM812","TRY002","B904","EM101","I001","UP006","UP007","RET","RUF021","E741","FAST","ASYNC","AIR","DJ","NPY","FA102","F401"] | ||
|
|
||
| # Allow fix for all enabled rules (when `--fix`) is provided. | ||
| fixable = ["ALL"] | ||
| unfixable = ["A","ARG","ANN","B","C901","D","DTZ","DOC","E501","ERA001","FBT","N","PD","PERF","PIE790","PLR","PT","Q","RSE102","RUF001","RUF012","S","SIM102","SIM108","SLF001","TID252","UP031","TRY003","TRY300","E713","SIM117","PGH003","RUF005","FIX","TD","TRY400","C408","PLW2901","PTH","EM102","INP001","CPY001","E266","PIE808","PLC2701","RUF100","FA100","FURB","C420","COM812","TRY002","B904","EM101","I001","UP006","UP007","RET","RUF021","E741","FAST","ASYNC","AIR","DJ","NPY","FA102","F401"] | ||
| [format] | ||
| quote-style = "preserve" |
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.
Don't put your ruff file inside this commit it should only stays on your computer 😄

/estate/init.py file imports all the models inside models related to estate
/estate/manifest.py file defines the name. version, category, sequence, summary, website link, dependencies, related data (views and access right files) of the "Estate" application.
/estate/models/estate_property.py creates the table "estate.property" with all its attributes:
fields.Charis the title of the posted propertyfields.Selectionis the type of the posted property (office, apartment, house, studio)fields.Charis the postcode where the property is locatedfields.Dateis the date from when the apartment is availablefields.Textis the description of the posted propertyfields.Integeris the number of the bedrooms in the apartmentfields.Integeris the area (in sqm) of the apartmentfields,Many2oneimports the EUR currencyfields.Monetaryis the expected price of the posted apartment in eurosfields.Monetaryis the selling price of the posted apartment in eurosfields.Integeris the number of facades in the posted apartmentfields.Booleanindicates if there's a garage for the apartmentfields.Booleanindicates if there's a garden for the apartmentfields.Integeris the area (in sqm) of the apartment's gardenfields.Selectionis the orientation of the posted property's garden (north, south, east, west)fields.Integeris the total area (in sqm) of the apartmentfields.Selectionis the status of the posted property in regards to offers (new. offer_received, offer_accepted, sold, canceled)fields.Booleanindicates if the posting of the property is still active/estate/models/init.py file imports the estate_property.py table
/estate/models/security/ir.model.access.csv file defines the access rights of the estate_property model
/estate/models/views/estate_menus.xml: defines the top menu in the Estate application which views properties returned from the action
estate_property_action/estate/models/views/estate_property_views.xml:
estate_property_actionwhich retrieves all the properties inside the estate.propety model in list or form views.= search for a property using the fields name, postcode, expected_price, bedrooms. living_area, or facades
= filter the apartments based on the availability state (results will show the apartments where the 'state' field is 'new' or 'offer_received')
= group properties by postcode