[ADD] estate: a new module to manage estate properties#164
Closed
[ADD] estate: a new module to manage estate properties#164
Conversation
|
Ping Lucky you I'm your reviewer :D |
|
Ah ouais, ça delete mon commit comme ça ... :peppa_sad: |
ouou-odoo
force-pushed
the
18.0-tutorial-ouou
branch
from
df1755e to
a72fa1d
Compare
aboo-odoo
reviewed
Oct 23, 2024
aboo-odoo
left a comment
aboo-odoo
left a comment
There was a problem hiding this comment.
Very good work keep going 🏅
I've been extra picky and nitpicked a lot but honestly most of this is just styling stuff.
Two last comments I can make so far are:
- Could you rename your PR to something like [ADD] estate: a new module to manage estate properties and embellish a bit your description
- Mind that commit messages (and PR title) must be as followed [TAG] module(s): short description. The body of your commit message is as important as the title. I advice you to build that body use the following three sections:
- Problem
- Objective
- Solution
and end your commit message with a reference to the task your assigned to (here there is none so you can put task-xxxxxx)
Check the commit message I pushed before on your branch for inspiration.
Once again, extremely good work. Keep it up 🥇
Comment on lines
6
to
13
| 'data': ['data/ir.model.access.csv', | ||
| 'views/estate_property_views.xml', | ||
| 'views/estate_menus.xml', | ||
| 'views/estate_property_type_views.xml', | ||
| ], |
There was a problem hiding this comment.
Sightly nicer indentation IMO
Suggested change
| 'data': ['data/ir.model.access.csv', | |
| 'views/estate_property_views.xml', | |
| 'views/estate_menus.xml', | |
| 'views/estate_property_type_views.xml', | |
| ], | |
| 'data': [ | |
| 'data/ir.model.access.csv', | |
| 'views/estate_property_views.xml', | |
| 'views/estate_menus.xml', | |
| 'views/estate_property_type_views.xml', | |
| ], |
estate/data/ir.model.access.csv
Outdated
| access_estate_prop,access_estate_prop,model_estate_property,base.group_user,1,1,1,1 | ||
| access_estate_prop_type,access_estate_prop_type,model_estate_property_type,base.group_user,1,1,1,1 | ||
| access_estate_prop_offer,access_estate_prop_offer,model_estate_property_offer,base.group_user,1,1,1,1 | ||
| access_estate_prop_tag,access_estate_prop_tag,model_estate_property_tag,base.group_user,1,1,1,1 No newline at end of file |
There was a problem hiding this comment.
Even though runbot does not tag it red, it's better to always add a line break at the end of a file. This makes further modification and git diff smaller later on when a line is added
estate/models/__init__.py
Outdated
| @@ -0,0 +1 @@ | |||
| from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer | |||
There was a problem hiding this comment.
Same reason as previously, it helps reduce the diff later and ease reviewing process if someone modifies the lines later on
Suggested change
| from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer | |
| from . import estate_property | |
| from . import estate_property_type | |
| from . import estate_property_tag | |
| from . import estate_property_offer |
| from odoo.tools import float_compare | ||
| from .helper import format_selection | ||
|
|
||
| from odoo.fields import Many2many, One2many |
There was a problem hiding this comment.
Not needed, just use fields.Many2many directly since it is already imported
estate/models/estate_property.py
Outdated
Comment on lines
1
to
6
| from odoo import api, fields, models | ||
| from dateutil.relativedelta import relativedelta | ||
|
|
||
| from odoo.exceptions import UserError, ValidationError | ||
| from odoo.tools import float_compare | ||
| from .helper import format_selection |
There was a problem hiding this comment.
Regarding imports, we try to order them as followed
- Python imports
- Other imports
- Odoo imports
Suggested change
| from odoo import api, fields, models | |
| from dateutil.relativedelta import relativedelta | |
| from odoo.exceptions import UserError, ValidationError | |
| from odoo.tools import float_compare | |
| from .helper import format_selection | |
| from dateutil.relativedelta import relativedelta | |
| from odoo import api, fields, models | |
| from odoo.exceptions import UserError, ValidationError | |
| from odoo.tools import float_compare | |
| from .helper import format_selection |
| @@ -0,0 +1,111 @@ | |||
| <odoo> | |||
| <data> | |||
| <record id="property_view_list" model="ir.ui.view"> | |||
| </field> | ||
| </record> | ||
|
|
||
| <record id="property_view_form" model="ir.ui.view"> |
| </field> | ||
| </record> | ||
|
|
||
| <record id="property_view_search" model="ir.ui.view"> |
| <field name="facades"/> | ||
| <filter name="Available properties" | ||
| domain="['|', ('state', '=', 'new'), ('state', '=', 'offer received')]"/> | ||
| <filter name="Postal code" domain="[]" context="{'group_by': 'postcode'}"/> |
There was a problem hiding this comment.
Suggested change
| <filter name="Postal code" domain="[]" context="{'group_by': 'postcode'}"/> | |
| <filter name="Postal code" context="{'group_by': 'postcode'}"/> |
| </field> | ||
| </record> | ||
|
|
||
| <record id="action_property" model="ir.actions.act_window"> |
ouou-odoo
changed the title
ouou-odoo
changed the title
ouou-odoo
force-pushed
the
18.0-tutorial-ouou
branch
from
92c8ede to
5e5c3fe
Compare
Features : - Creating properties, types of properties - Handling customers offers - Sorting your properties by tags or by type - Generating invoices when properties are sold and much more...
ouou-odoo
force-pushed
the
18.0-tutorial-ouou
branch
from
5e5c3fe to
5d1ae0f
Compare
aboo-odoo
reviewed
Oct 25, 2024
| Command.create({'name': 'Administrative fees', 'price_unit': fees, 'quantity': 1}), | ||
| ] | ||
|
|
||
| journal = self.env['account.journal'].search([('type', '=', 'sale')]) |
There was a problem hiding this comment.
This could return more than one journal, add limit=1 to make sure
|
|
||
| def action_sold(self): | ||
| if not (self.buyer and self.selling_price): | ||
| raise UserError('Customer or selling price was not set.') |
There was a problem hiding this comment.
I noticed your error messages are not getting translation
Add _() -> from odoo import _ to make them translatable
Problem: The errors were not automatically translated. Solution: Imported _ from odoo and added _ to each error.
|
@ouou-odoo Hey! Could you close the PR please, thank you <3 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We should add an estate module for real estates companies, here is a small proof of work module.
Features :