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][NEW] partner_delivery_zone: New module to group partners, orders and pickings by delivery zones #167

Merged
merged 1 commit into from
Oct 23, 2018

Conversation

sergio-teruel
Copy link
Contributor

@rousseldenis rousseldenis added this to the 11.0 milestone Oct 18, 2018
)

@api.model
def _commercial_fields(self):
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

@sergio-teruel Really? This could be too restrictive as it forbids to have a company with several delivery zones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uuuupss!!! You are right

Copy link
Member

Choose a reason for hiding this comment

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

You can still provide a default when creating the contacts.

@sergio-teruel
Copy link
Contributor Author

@rousseldenis Changes done, also update the test

Copy link
Sponsor Contributor

@rousseldenis rousseldenis 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.

@sergio-teruel Could the 'zone' be generalized ? It could be nice to have a general defintion of it. And then reuse it on some interesting functional points (like sale zones, or ... and add after that a geoengine layer).

@rousseldenis
Copy link
Sponsor Contributor

@sergio-teruel pylint and flake8 fail

@pedrobaeza
Copy link
Member

@rousseldenis unfortunately we don't have budget for more changes, as this being very small (the model has 2 fields), it can be extracted or replicated easily if someone has that need to put this on other place without depending on this module.

@sergio-teruel, little flake8 fail:

partner_delivery_zone/models/res_partner.py:3:1: F401 'odoo.api' imported but unused

I'm reviewing code now.


@api.onchange('partner_shipping_id')
def onchange_partner_shipping_id_delivery_zone(self):
self.delivery_zone_id = self.partner_shipping_id.delivery_zone_id.id
Copy link
Member

Choose a reason for hiding this comment

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

Only change this if the partner has a delivery zone set. If not, preserve the previous value.


@api.onchange('partner_id')
def onchange_partner_id_zone(self):
self.delivery_zone_id = self.partner_id.delivery_zone_id
Copy link
Member

Choose a reason for hiding this comment

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

The same about preserving previous value

@@ -0,0 +1,4 @@
This module allows set delivery zones on partner, this information is written
Copy link
Member

Choose a reason for hiding this comment

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

s/allow set/allows to set.

Put a . instead of comma before this information...

@@ -0,0 +1,4 @@
This module allows set delivery zones on partner, this information is written
in sale orders and stock pickings.
Also adds searchs and groups in partners, sales orders asn stock pickings
Copy link
Member

Choose a reason for hiding this comment

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

s/Also/It also
s/searchs/searches
s/asn/and

@@ -0,0 +1,2 @@
* For a future, the system will can set delivery zones based on partner zip.
Copy link
Member

Choose a reason for hiding this comment

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

s/a/the
s/the system/system
s/will can/can

@@ -0,0 +1,2 @@
* For a future, the system will can set delivery zones based on partner zip.
I will create a rules for that.
Copy link
Member

Choose a reason for hiding this comment

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

Remove this sentence

@@ -0,0 +1,9 @@
To use this module you need to:

#. Go to *Contacts > Create* or *Sales > Customers > Create*.
Copy link
Member

Choose a reason for hiding this comment

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

Put second part of this sentence as a new point before "Create a sales order..."

<field name="inherit_id" ref="base.view_partner_form"/>
<field name="arch" type="xml">
<field name="industry_id" position="after">
<field name="delivery_zone_id"/>
Copy link
Member

Choose a reason for hiding this comment

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

Delivery zone is not shown nor put as default for children contacts managed from the same view.

@sergio-teruel
Copy link
Contributor Author

@rousseldenis @pedrobaeza Changes done!.
Added default delivery zone for child_ids field in context and fix typo and form view for contacts

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Next time please put a new fixup commit instead of amending previous one, as this way it's very difficult to review latest changes.

@sergio-teruel
Copy link
Contributor Author

Oks. Thanks

@rousseldenis rousseldenis merged commit 0a53983 into OCA:11.0 Oct 23, 2018
@sergio-teruel sergio-teruel deleted the 11.0-PR-partner_delivery_zone branch January 18, 2019 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants