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

[12.0][MIG] operating_unit #144

Merged
merged 21 commits into from
Nov 21, 2018
Merged

Conversation

AdriaGForgeFlow
Copy link
Contributor

Migration of operating_unit to v12.0

<field name="category_id" ref="module_operating_units"/>
</record>

<record id="group_manager_operating_unit" model="res.groups">
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this new group added in this version. I think we can propose this to previous versions. That will solve #129

Copy link
Contributor

Choose a reason for hiding this comment

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

It is easy said than done. Applying those changes to v11/v10 will man to make global rules to a simple group rules and cause many errors in database with the modules installed. I changed my mind, and better to keep this from v12 onwards.

@AdriaGForgeFlow AdriaGForgeFlow force-pushed the 12.0-mig-operating_unit branch 2 times, most recently from d5c3374 to 10cdf9c Compare November 19, 2018 15:59
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.

Improve Code

@@ -0,0 +1,29 @@
# Copyright 2015-2017 Eficent
# - Jordi Ballester Alomar
# Copyright 2015-2017 Serpent Consulting Services Pvt. Ltd. - Sudhir Arya
Copy link
Member

Choose a reason for hiding this comment

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

@ageficent Replace all Copyright 2015-TODAY Serpent Consulting Services Pvt. Ltd.

@@ -0,0 +1,5 @@
# Copyright 2015-2017 Eficent
Copy link
Member

Choose a reason for hiding this comment

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

@ageficent Remove copyright from all __init__ file.

@@ -0,0 +1 @@
from . import test_operating_unit
Copy link
Member

Choose a reason for hiding this comment

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

@ageficent add blank new line

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.

👍 Functional + code review

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
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Small comments inline, thanks for adding tests!

To configure this module, you need to:

* Assign *Multi Operating Unit* group to user.
* Go to *Settings / Users & Companies / Operating Units* and create Operating Units.
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be added to a new fragment CONFIGURE.rst. See https://github.com/OCA/maintainer-tools/tree/master/template/module/readme

# Create
self._create_operating_unit(self.user2.id, "Test", "TEST")
# Write
self.b2b.sudo(self.user2.id).write({'code': 'B2B_changed'})
Copy link
Contributor

Choose a reason for hiding this comment

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

this shold be a different assertRaises because the previous line already raised the error, so you are not testing this really.

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Great 👍

@nikul-serpentcs
Copy link
Member

@pedrobaeza @jbeficent Can you please review and move forward.

@pedrobaeza
Copy link
Member

Sorry, but I'm not a qualified reviewer for this repository.

@nikul-serpentcs
Copy link
Member

Ok @pedrobaeza ,Wait If Done after

@AaronHForgeFlow AaronHForgeFlow merged commit deef9a0 into OCA:12.0 Nov 21, 2018
@AaronHForgeFlow AaronHForgeFlow deleted the 12.0-mig-operating_unit branch November 21, 2018 11:12
@pedrobaeza
Copy link
Member

@aheficent remember to keep up to date migration issue checking the mark for merged modules (I have already done it for this one).

@AaronHForgeFlow
Copy link
Contributor

@pedrobaeza Thank you! I will keep the issue updated.

@AaronHForgeFlow
Copy link
Contributor

BTW, the default branch to 12.0 is updated automatically?

@pedrobaeza
Copy link
Member

No, that should be done manually now when you consider that branch is completed enough.

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

Successfully merging this pull request may close these issues.

10 participants