-
-
Notifications
You must be signed in to change notification settings - Fork 773
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
[16.0][MIG] migrate purchase_cancel_reason #1776
Conversation
…_cancel_reason (OCA/sale-workflow) developed by CampToCamp. (OCA#438) * [ADD] Add the module purchase_cancel_reason. Copy/past from the module sale_cancel_reason (OCA/sale-workflow) developed by CampToCamp.
Currently translated at 40.9% (9 of 22 strings) Translation: purchase-workflow-14.0/purchase-workflow-14.0-purchase_cancel_reason Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-14-0/purchase-workflow-14-0-purchase_cancel_reason/pt_BR/
Currently translated at 36.3% (8 of 22 strings) Translation: purchase-workflow-14.0/purchase-workflow-14.0-purchase_cancel_reason Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-14-0/purchase-workflow-14-0-purchase_cancel_reason/it/
Currently translated at 36.3% (8 of 22 strings) Translation: purchase-workflow-14.0/purchase-workflow-14.0-purchase_cancel_reason Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-14-0/purchase-workflow-14-0-purchase_cancel_reason/it/
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.
@pierre-halleux Some little details...
|
||
cancel_reason_id = fields.Many2one( | ||
comodel_name="purchase.order.cancel.reason", | ||
string="Reason for cancellation", |
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.
string="Reason for cancellation", | |
string="Cancellation reason", |
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.
Cancelation with one L then as we use US english
<label for="cancel_reason_id" string="Cancellation reason:" /> | ||
<field | ||
name="cancel_reason_id" | ||
class="oe_inline" | ||
options='{"no_open": 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.
I propose to improve the view as follow:
<label for="cancel_reason_id" string="Cancellation reason:" /> | |
<field | |
name="cancel_reason_id" | |
class="oe_inline" | |
options='{"no_open": True}' | |
/> | |
<label for="cancel_reason_id"/> | |
<pan> </pan> | |
<field name="cancel_reason_id" class="oe_inline" options="{"no_open": True}"/> |
After
930ff43
to
4d9c8cf
Compare
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.
LGTM (Code review + functional test)
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.
Small fixes needed,
) | ||
|
||
|
||
class PurchaseOrderCancelReason(models.Model): |
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 make dedicate file per model
</field> | ||
</record> | ||
|
||
<record id="view_purchase_order_cancel_reason_form" model="ir.ui.view"> |
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.
same here
from odoo.exceptions import UserError | ||
|
||
|
||
class PurchaseOrderCancel(models.TransientModel): |
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.
the file name don't match the model name
if purchase_ids is None: | ||
return act_close | ||
if len(purchase_ids) > 1: | ||
raise UserError(_("Only 1 purchase ID expected")) |
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.
raise UserError(_("Only 1 purchase ID expected")) | |
raise UserError(_("Only 1 purchase expected")) |
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 would even drop this restriction
self.ensure_one() | ||
act_close = {"type": "ir.actions.act_window_close"} | ||
purchase_ids = self._context.get("active_ids") | ||
if purchase_ids is None: |
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.
if purchase_ids is None: | |
if purchase_ids is None or self._context.get("active_model") != "purchase.order": |
<odoo> | ||
<record id="view_purchase_order_cancel" model="ir.ui.view"> | ||
<field name="name">Reason for the cancellation</field> | ||
<field name="model">purchase.order.cancel</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.
same here, the file name don't match model name
* Guewen Baconnier, Camptocamp SA | ||
* Sylvain Van Hoof <sylvain@okia.be> | ||
|
||
* `Ecosoft <http://ecosoft.co.th>`_: | ||
|
||
* Kitti U. <kittiu@ecosoft.co.th> | ||
* Tharathip C. <tharathipc@ecosoft.co.th> |
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.
reformat list: remove blank lines and remove indent
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.
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.
ah ok :)
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 reverted this commit so it looks like it used to
@pierre-halleux Could you attend comments ? |
b42161c
to
8068627
Compare
8068627
to
4a4854e
Compare
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.
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.
Same minor comment as @rousseldenis and @lmignon:
The purchase view can be improved
8df601a
to
480704a
Compare
480704a
to
dc1385d
Compare
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
@HviorForgeFlow CAn you merge this one since you merged the one for v17 based on this. |
/ocabot merge nobump |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at ddafa41. Thanks a lot for contributing to OCA. ❤️ |
Thank you! |
No description provided.