-
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
[ADD] estate: adding a new real estate module #313
base: 18.0
Are you sure you want to change the base?
Conversation
Chapter 1: Implemented the multitier architecture followed by Odoo. Chapter 2: Created a module named estate, with the following files: __manifest__.py: Added basic module information and set application to True. __init__.py: Initialized the module. Chapter 3: Created a models directory within the estate module, containing: estate_property.py: Defined model and fields to create a table for estate properties. __init__.py: Initialized the models.
Chapter 4: Initialize a new file ir.model.access.csv and provide different permissions in it. Chapter 5: In this chapter, we learn about XML files and why Odoo uses them. We then create the estate_property_views.xml file to define actions. After that, we create a file called estate_menus.xml to display menus for these actions. We define the necessary fields and attributes for our real estate interface, then set some limitations on the fields, such as making them read-only. We also add default values to some fields and introduce two additional fields: active and state, providing values for them. Chapter 6: In this chapter, we understand the different types of views (list, form, search) in an interface. We first create a list view to display our real estate properties. Then, we create a form view for editing or updating existing properties, using some HTML tags for customization. Lastly, we create a search view to allow users to filter properties based on different criteria. We also add filters and groupings to display specific data, such as properties with the state set to "new" or "offer received". During this process, we learn about the use of domains in Odoo.
…of models. Chapter 7: In this chapter, we created different models and established links between them. First, we created a model called estate_property_type, and then we created a variable or XML file for property types, allowing us to define different types of properties like houses and apartments. In this model, we used the Many2one concept. Next, we created a different model named estate_property_tag by following the same steps as the property type model, with the only difference being the use of the Many2many concept. With this model, we can define various tags, such as cozy and renovated. The final model we created in this chapter is estate_property_offer. This model follows the same steps, but in this case, we don't need to create an XML file, and we used the One2many concept. Overall, we gained an understanding of the concepts Many2one, Many2many, and One2many.
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.
Hello @akus-odoo 👋,
Great work so far!
This is the first set of the review.
Please make sure to add a new line at the end of every file.
estate/__manifest__.py
Outdated
], | ||
'license': 'LGPL-3' | ||
} | ||
# SHA256:Hmym4hCb2IR8asKknQs4KfejqLJ6MOota2asZe9IQTU [email protected] |
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.
Remove this line
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.
done
estate/data/ir.model.access.csv
Outdated
@@ -0,0 +1,2 @@ | |||
id,name,model_id/id,group_id/id,perm_read,perm_write,perm_create,perm_unlink | |||
estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1 |
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.
Could you please explain why there's a 1,1,1,1 at the end?
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.
id=estate.access_estate_property
name=access_estate_property
model_id/id=estate.model_estate_property
group_id/id=base.group_user
giving permission to read ,write,create and unlink by doing there value to true for above model
name = fields.Char(string="Title",required=True) | ||
description = fields.Text(string="Description") | ||
postcode = fields.Char(string="Postcode") | ||
date_availability = fields.Date(string="Date Availability", copy=False, default=lambda self: (datetime.today() + timedelta(days=90)).date()) |
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.
Could you please explain the purpose of adding the default and what function it serves?
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.
using default we can set a value date for date_availability when a user opens the page a default date is mentioned.
so we have a requirement to set a default date of after 3 months so we need to calculate that by adding 90 days to current date so with the help of lambda function we fulfilled the requirement.
estate/models/estate_property.py
Outdated
expected_price = fields.Float(string="Expected Price",required=True) | ||
selling_price = fields.Float(string="Selling Price",readonly=True,copy=False) |
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.
expected_price = fields.Float(string="Expected Price",required=True) | |
selling_price = fields.Float(string="Selling Price",readonly=True,copy=False) | |
expected_price = fields.Float(string="Expected Price", required=True) | |
selling_price = fields.Float(string="Selling Price", readonly=True, copy=False) |
Please ensure the entire module follows coding guidelines by adding spaces wherever necessary.
Also, could you explain the purpose of "required," "readonly," and "copy" parameter in this context?
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 have added the necessary spaces and blank lines for follow coding guidelines.
required parameter means that field is important to fill out which is mandatory.
readonly parameter is for for read only we cannot set the value.
copy parameter is set to false by which if we make duplicate property the selling price will not be copied.
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')], required=True, copy=False) | ||
active = fields.Boolean(string="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.
Please remove the extra lines and ensure there is only a single empty line in each file.
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.
ok changes has been done.
<record id="estate_model_action" model="ir.actions.act_window"> | ||
<field name="name">Properties</field> | ||
<field name="res_model">estate_property</field> | ||
<field name="view_mode">list,form</field> |
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 is the purpose of adding view_mode here?
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.
view_mode when this action will performed which view we need to observe for that we selected list and form.
<field name="living_area" /> | ||
<field name="facades" /> | ||
<filter name="active" string="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.
Could you please explain, What is the purpose of adding domain
attribute here?
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.
it is basically means on what basis we want to filter out the property list in this case we want only property which has an state new either offer received.
domain="['|',('state', '=', 'new'),('state','=','offer received')]" /> | ||
<group expand="1" string="Group By"> | ||
<filter string="Postcode" name="postcode" | ||
context="{'group_by':'postcode', 'residual_visible':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.
Could you please explain, What is the purpose of adding context
attribute here?
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.
context defines in which the filter operates like in this we are grouping the properties by their postcode.
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.
Could you please clarify if it is necessary to use residual_visible
in the context?
Additionally, could you explain its purpose?
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 it is not necessary because in latest version of odoo even without specifying residual_visible=True, records that do not have a postcode will still appear in the grouped view under a 'false' category.
Its purpose was in older version of odoo to ensure that ungrouped records (e.g., records with no postcode) were visible.
Chapter 8: In this chapter, we understand the concepts of @api.depends, compute(), which are basically Python logic. Our first task was to add a new field in estate.property named total_area, which is the sum of living_area and garden_area. To achieve this, we used the @api.depends or compute() method. Our second task was to initialize a new field named best_price in estate_ property, with the goal of getting the highest offer from a buyer. In this, we used the same steps as in total_area. Our third task was to understand the inverse function. For that, we created two fields, validity and date_deadline, in the estate.property.offer model. The purpose was to allow either validity or date_deadline to be updated, and when one is updated, the other gets recalculated. For this, we implemented inverse methods. Our last task was to understand the onchange() method. By implementing this, when the garden field is set, default values will be given to the garden_area and garden_orientation fields, or vice versa. Overall, we learned how to write logic in Python by understanding different methods.
Chapter9:Our first task was to add Cancel and Sold buttons to the estate.property model.The logic behind these buttons ensures that if a property is cancelled, it cannot be sold, and vice versa. We implemented business logic methods for the buttons and explored how to raise exceptions. The second task involved adding Accept and Refuse buttons to the estate.property.offer model. These buttons were represented with cross and check icons in the offer form. When clicked, they trigger actions that set the offer status to accepted or refused by initializing the buttons and corresponding action methods. The final task required us to implement business logic such that when an offer is accepted, the property’s buyer and selling price are set based on the accepted offer. Moreover, only one offer can be accepted for a property at a time; all other offers are automatically set to refused. Overall, we gained an understanding of how to trigger actions through buttons by writing business logic.
<record id="estate_property_type_action" model="ir.actions.act_window"> | ||
<field name="name">Property Types</field> | ||
<field name="res_model">estate.property.type</field> | ||
<field name="view_mode">list,form</field> |
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.
Could you kindly confirm if these are the only available attributes?
Also, is it necessary to include the view_mode?
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 we have some more attributes but as per the requirements we used these three only.
If we talk about view mode it is not necessary but if we don't specify view mode it will use the default one so as per our requirement it is recommended to specify.If we required a different view from list and form so we can use specify view_mode because the default one is already list and form.
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.
Hello @akus-odoo 👋
It seems you're still lagging behind 🥶
Anyways, This is the second set of reviews.
Thank you!
self.garden_area = 10 | ||
self.garden_orientation = 'north' |
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.
Why set a static value here?
Why can't this be set in the default
or through a compute
method
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 used static value here because when the user checks/unchecks garden, the corresponding fields (garden_area and garden_orientation) update in the form before saving the record,user can change it and explicitly save the record.
We cant use default because as per the requirement if we checks the garden then only set the values but if we use default in unchecks of garden then also we observe the values.
Or in case of compute we can use it , or that is more preferable but as per the documentation guideline i used onchange.
def button_cancel_action(self): | ||
for record in self: | ||
if(record.state == 'sold'): | ||
raise exceptions.UserError("Sold properties can't be canceled") | ||
else: | ||
record.state= 'canceled' | ||
return True | ||
|
||
def button_sold_action(self): | ||
for record in self: | ||
if(record.state == 'canceled'): | ||
raise exceptions.UserError("Canceled properties can't be sold") | ||
else: | ||
record.state = 'sold' | ||
return 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.
Can this be managed using a single method?
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.
Yes it can be and that would be a proper approach because it is better than two separate methods which avoids duplicate logic , improves readability & maintainability,easier to extend in the future. I am performing the approach.
for offer in all_offers: | ||
if offer.id != record.id: # Excluding the current offer | ||
offer.status = 'refused' |
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.
Can we directly set the status of all_offers
to "refused" instead of looping through them?
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.
We can but the requirement was if one get accepted than set that property is accepted and else was refused that why i used looping.
Can you please mark me where i am lagging behind for improving my performance. |
Created a real estate model.