Server technical tutorial (josiv) - #1390
Conversation
|
|
qucol-odoo
left a comment
qucol-odoo
left a comment
There was a problem hiding this comment.
Very good job so far 👌 I wrote a few comments, try to have a look at them and apply them when you find the time :)
Also, it would be good if you could follow our git commit guidelines. Your commit name should always follow the pattern [TAG] <module_name>: <description>. You should try to rename your first commits (hint), and apply that guideline for all the future ones 😇
| string="Available" | ||
| name="available" | ||
| help="display only available estates" | ||
| domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/> |
There was a problem hiding this comment.
You can use the in operator here :)
| domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/> | |
| domain="[('state', 'in', ('new', 'offer_received'))]"/> |
| <notebook> | ||
| <page string="Description"> | ||
| <group> | ||
| <field name="description"/> | ||
| <field name="bedrooms"/> | ||
| <field name="living_area"/> | ||
| <field name="facades"/> | ||
| <field name="garage"/> | ||
| <field name="garden"/> | ||
| <field name="garden_area"/> | ||
| <field name="garden_orientation"/> | ||
| </group> | ||
| </page> | ||
| </notebook> |
There was a problem hiding this comment.
Not necessarily important, but you could have added the state field somewhere in there too. Without adding the field anywhere, I don't know how you could test the behavior of your "Available" filter.
There was a problem hiding this comment.
Good point, I think I added at first, but removed it because it did not looked like breadcrumbs we used to see. I was thinking it will be added back later in the chapter as better UI element. I will add it again.
| 'name': "A Real Estade Advestisement Demo", | ||
| 'version': '1.0', | ||
| 'depends': ['base'], | ||
| 'author': "Jozef Sivek (josiv)", |
There was a problem hiding this comment.
Totally fine for a tutorial module, but just so you know, in a real case scenario, you'd have to set the author to 'Odoo S.A.'.
| ], | ||
| # data files containing optionally loaded demonstration data | ||
| 'demo': [ | ||
| #'demo/demo_data.xml', |
There was a problem hiding this comment.
A general rule of thumb is to avoid pushing commented code. In a bigger review, it can always be forgotten and end up in the database, which is unfortunate.
| ('cancelled', 'Cancelled')], | ||
| default='new', | ||
| help="State of the estate property") | ||
|
|
There was a problem hiding this comment.
You have a few styling errors, including this one (you should only have one blank line at the end of your file, not two).
There was a problem hiding this comment.
Indeed. I did not install linter yet. I will do at as soon as possible, as there is no single one to choose blindly.
There was a problem hiding this comment.
My advice would be to use Ruff, as it is the one we use on the Runbot, which will block your PRs if you don't respect the styling guidelines. The configuration file we use is available on the github repo here. There's also a VSCode extension.
29d293d to
0fafb7a
Compare
Also fix errors on creation of the record. Chapter 5
Add computed total area for an estate. Set defaults for garden related fields when selected and clean them when garden is unselected. Chapter 8
E.g. do not allow negative prices, and accept only offers above 90% of the expected price. Chapter 10
Fine tune UI to be compliant with the other modues. Define default list orders for the models to highlight the most important records first (highest offer) or allow manual reordering for commonly accessed property types. Add visual classes to estate listings with received, accepted and sold state. Chapter 11
791faab to
090797e
Compare
3 participants
No description provided.