Skip to content
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

mohu - Technical Training #52

Closed
wants to merge 39 commits into from
Closed

Conversation

mizosoft
Copy link

No description provided.

Copy link

@vchu-odoo vchu-odoo left a comment

Choose a reason for hiding this comment

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

Great work!
You made your first PR! 👍

Only one small comment.

estate/__manifest__.py Outdated Show resolved Hide resolved
@mizosoft mizosoft force-pushed the training-mohu branch 2 times, most recently from 857b18a to 7bef0d5 Compare March 19, 2024 15:31
Copy link

@vchu-odoo vchu-odoo left a comment

Choose a reason for hiding this comment

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

Good work!

Only one small nitpicking and suggestion.

estate/security/ir.model.access.csv Outdated Show resolved Hide resolved
estate/models/estate_property.py Show resolved Hide resolved
@mizosoft mizosoft force-pushed the training-mohu branch 3 times, most recently from 2bcfb5a to 7720d18 Compare March 21, 2024 05:36
Copy link

@vchu-odoo vchu-odoo left a comment

Choose a reason for hiding this comment

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

Good job!
Continue like that 👍

There are some fields in estate property that do not correspond to the task description in the tutorial, is it intended?

estate/models/estate_property.py Outdated Show resolved Hide resolved
estate/models/estate_property.py Outdated Show resolved Hide resolved
estate/models/estate_property.py Outdated Show resolved Hide resolved
estate/models/estate_property.py Outdated Show resolved Hide resolved
estate/models/estate_property.py Outdated Show resolved Hide resolved
estate/models/estate_property.py Outdated Show resolved Hide resolved
estate/views/estate_property_views.xml Outdated Show resolved Hide resolved
estate/views/estate_property_views.xml Outdated Show resolved Hide resolved
estate/views/estate_property_views.xml Outdated Show resolved Hide resolved
estate/views/estate_property_views.xml Outdated Show resolved Hide resolved
@mizosoft
Copy link
Author

There are some fields in estate property that do not correspond to the task description in the tutorial, is it intended?

Yes, I added accepted_offer_id to allow a NO-OP to handle accepting or refusing an already accepted offer. I've modified the code to rely on estate_property.selling_price & estate_property_offer.status instead for these cases.

Copy link

@vchu-odoo vchu-odoo left a comment

Choose a reason for hiding this comment

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

Congrats on completing python tutorial, great job!

estate/views/res_users_views.xml Outdated Show resolved Hide resolved
estate/views/estate_property_views.xml Outdated Show resolved Hide resolved
@AntoineVDV AntoineVDV closed this May 6, 2024
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.

None yet

3 participants