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

[11.0] [MIG] sale_procurement_group_by_line #673

Conversation

murtuzasaleh
Copy link
Member

@murtuzasaleh murtuzasaleh commented Jul 18, 2018

Sale Procurement Group by line

  • This module was written to extend the functionality of procurement groups
    created from a Sale order.

  • On itself, this module does nothing it is a requirement for modules which
    needs to create procurement group per sale order line basis.

Migration of module Sale Procurement Group by line v10 to v11.

@pedrobaeza
Copy link
Member

@jbeficent is this intended for the same as OCA/purchase-workflow#563?

@murtuzasaleh murtuzasaleh force-pushed the 11.0-mig-sale_procurement_group_by_line branch 3 times, most recently from a41dba2 to 9e145a5 Compare July 18, 2018 11:58
@murtuzasaleh murtuzasaleh force-pushed the 11.0-mig-sale_procurement_group_by_line branch from 0eb6ede to 0180cbb Compare July 24, 2018 07:35
@murtuzasaleh murtuzasaleh mentioned this pull request Jul 24, 2018
65 tasks
@murtuzasaleh
Copy link
Member Author

@pedrobaeza @mreficent Could you please check it.

Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

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

Code Review LGTM

Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

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

Code Review LGTM

@nikul-serpentcs
Copy link
Member

@pedrobaeza @JayVora-SerpentCS Could you please review and move forward.

@pedrobaeza pedrobaeza added this to the 11.0 milestone Oct 11, 2018
@pedrobaeza
Copy link
Member

I'm afraid I don't know this module, so others should review it.

needs to create procurement group per sale order line basis.


Usage
Copy link
Member

Choose a reason for hiding this comment

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

Improve usage section (add an usage example, where you canbenefit from this module avoid using technical language), add some pictures, would be nice to move readme to new format

sale_procurement_group_by_line/__init__.py Outdated Show resolved Hide resolved
sale_procurement_group_by_line/model/__init__.py Outdated Show resolved Hide resolved
@@ -52,5 +52,8 @@ def test_full_automatic(self):
invoice = sale.invoice_ids
Copy link
Member

Choose a reason for hiding this comment

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

That file is not related with this module, please remove

sale_procurement_group_by_line/tests/__init__.py Outdated Show resolved Hide resolved
line.order_id.partner_shipping_id.
property_stock_customer, line.name, line.order_id.name,
values)
except UserError as error:
Copy link
Member

Choose a reason for hiding this comment

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

As an UserError is caught, maybe it's interesting to append which products have raised it.

Copy link
Contributor

Choose a reason for hiding this comment

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

If super cannot be called then you need to add a comment telling clearly that you are overwriting the original method. Also include it in the known issues section.

@JayVora-SerpentCS
Copy link
Contributor

@murtuzasaleh check.

Copy link
Contributor

@MiquelRForgeFlow MiquelRForgeFlow left a comment

Choose a reason for hiding this comment

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

@murtuzasaleh will you finish this PR?

subtype_id=self.env.ref('mail.mt_note').id)
new_procs += new_proc
new_procs.run()
super(SaleOrderLine, self)._action_procurement_create()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you delete this super?

Copy link
Member Author

@murtuzasaleh murtuzasaleh Nov 15, 2018

Choose a reason for hiding this comment

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

@mreficent Because In odoo v11 _action_procurement_create method is removed and added _action_launch_procurement_rule method odoo/odoo@c914df5#diff-bf6a9af0e4d7537746d73deb353f3f8aL210 so I have overwrite this method and added code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but why you remove the super? You can do super with the _action_launch_procurement_rule method.

Copy link
Member

Choose a reason for hiding this comment

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

@mreficent Here super return True , So How can modify the code with super?.

That's way @murtuzasaleh overwrite code and modify that.

Here Super is not need. @aheficent already #673 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

return super(self, SaleOrderLine)._action_launch_procurement_rule()

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @murtuzasaleh @nikul-serpentcs Did you try to call super at the end? what does the system do in that case? Are other procurement groups created or the ones you create updated? Maybe they are not. I am not sure looking at the code.

Copy link
Member

Choose a reason for hiding this comment

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

Did you try to call super at the end?

@aheficent Yes, But it's method Call again.

Copy link
Contributor

Choose a reason for hiding this comment

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

But does that affect the result?

yvaucher and others added 12 commits November 15, 2018 10:52
This module extract logic to create multiple procurement group
for a single sale order for grouped sale order lines.
* Fix OCA#264
* Fix : Can't create delivery from shipping exception
* [ADD] [9.0] sale_delivery_block

* [IMP] Add data with an example Delivery Block Reason.

* [FIX]
 * Able to edit the Delivery Block Reason in state 'sent'
 * README

* [FIX] make sale_delivery_block compatible with sale_procurement_group_by_line

* [FIX] travis and data nouptade.

* [ADD] delivery block tests.

* [IMP] Add the 'Default Delivery Block Reason' in partners.

* [IMP] track visibility of delivery block reason

* [9.0][IMP] sale_delivery_block:
  * default_delivery_block is now a comercial field.
  * When duplicating a sale order the delivery block is recomputed.

* [9.0][UPD] sale_delivery_block_proc_group_by_line: Update README.

* [FIX] api.model used with api.constrains
@murtuzasaleh murtuzasaleh force-pushed the 11.0-mig-sale_procurement_group_by_line branch from 0180cbb to d372e42 Compare November 15, 2018 06:53
Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

Code seems ok to me, it is not calling super but conserving the original behavior, plus adding the grouping by key feature.

line.order_id.partner_shipping_id.
property_stock_customer, line.name, line.order_id.name,
values)
except UserError as error:
Copy link
Contributor

Choose a reason for hiding this comment

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

If super cannot be called then you need to add a comment telling clearly that you are overwriting the original method. Also include it in the known issues section.

@LoisRForgeFlow
Copy link
Contributor

@murtuzasaleh Can you rebase? Error in CI was due to another module, but it's fixed now.

@AaronHForgeFlow
Copy link
Contributor

BTW sale_source_by_line will be deprecated. This, this module is only a dependency for this one #668 Do you agree @murtuzasaleh?

@MiquelRForgeFlow
Copy link
Contributor

Superseeded by #758.

@pedrobaeza pedrobaeza closed this Dec 29, 2018
@murtuzasaleh murtuzasaleh deleted the 11.0-mig-sale_procurement_group_by_line branch May 6, 2019 05:03
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.