Skip to content

18.0 tutorial module data aras #5

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

Open
wants to merge 10 commits into
base: 18.0-tutorial-aras
Choose a base branch
from

Conversation

abdrahmanrashed
Copy link

No description provided.

Copy link

@vava-odoo vava-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty good 👍

<field name="state">new</field>
<field name="description">A nice and big villa</field>
<field name="postcode">12345</field>
<field name="date_availability">2020-02-02</field>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should preferably be relative to the date of install (see codebase for examples)

<odoo>
<record id="estate_property_1" model="estate.property">
<field name="name">Big Villa</field>
<field name="state">new</field>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is the default value, you're not forced to provide it

Suggested change
<field name="state">new</field>

<field name="postcode">54321</field>
<field name="date_availability">1970-01-01</field>
<field name="expected_price">100000</field>
<field name="selling_price">120000</field>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a selling_price if the state is cancelled?

<function model="estate.property.offer" name="action_offer_refuse" eval="[ref('estate_property_offer_1')]"/>
<function model="estate.property.offer" name="action_offer_refuse" eval="[ref('estate_property_offer_2')]"/>
<function model="estate.property.offer" name="action_offer_accept" eval="[ref('estate_property_offer_3')]"/>
</odoo>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing EOF line-break

…d/cancelled property.

-Added check to raise an error if a property is sold without any offer accepted.
-Created tests cases to verify total area calculation, verify action_property_sold, and verify offer creation for sold property.
-Added missing new lines in some files.
Copy link

@vava-odoo vava-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite ok 👍


cls.properties = cls.env['estate.property'].create([{'name': 'test_house'}])
cls.partner = cls.env['res.partner'].create([{
'id': 'test_partner',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id?! don't touch this 😄

Suggested change
'id': 'test_partner',

Comment on lines 18 to 26
'company_name': 'test_company',
'street': 'test_street',
'city': 'test_city',
'zip': '12345',
'country_id': cls.env.ref('base.us').id,
'state_id': cls.env.ref('base.state_us_39').id,
'phone': '+1 555-555-5555',
'email': '[email protected]',
'tz': 'Europe/Brussels',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need all this? I bet a name is sufficient for your case

Suggested change
'company_name': 'test_company',
'street': 'test_street',
'city': 'test_city',
'zip': '12345',
'country_id': cls.env.ref('base.us').id,
'state_id': cls.env.ref('base.state_us_39').id,
'phone': '+1 555-555-5555',
'email': '[email protected]',
'tz': 'Europe/Brussels',

# add env on cls and many other things
super().setUpClass()

cls.properties = cls.env['estate.property'].create([{'name': 'test_house'}])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plural is misleading

Suggested change
cls.properties = cls.env['estate.property'].create([{'name': 'test_house'}])
cls.property = cls.env['estate.property'].create([{'name': 'test_house'}])

Comment on lines 36 to 38
self.assertRecordValues(self.properties, [
{'total_area': 35},
])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertEqual if only one value to check, no?

Comment on lines 59 to 60
self.properties.offer_ids.action_offer_accept()
self.properties.action_property_sold()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to tests, but I have the feeling that action_offer_accept should mark the property as sold, so why do you need the call to action_property_sold?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the estate tutorial accepting the offer and selling the property are two different states.
After accepting an offer you can still cancel the property, not sell it.

def test_action_sell_with_accepted_offer(self):
"""Test that everything behaves like it should when selling a valid property."""

self.properties.offer_ids.create({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird way to create an offer. Either use self.env['property.offer'].create(...), either create it while editing the field offer_ids with Command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants