[11.0] mig operating unit #99

gdgellatly · 2017-11-03T03:00:06Z

Change groups_id inheritance in view to specific group as groups_id gets replaced by a same priority view.
Remove encodings
Bump Version
Test
Add known issues for docs - if you know the answer I can update docs.
Update name_search signature to match BaseModel

gdgellatly · 2017-11-03T13:00:23Z

Review URL 5592d2c to make it easier for reviewers

AaronHForgeFlow

Code seems ok to me. Functional ok also so 👍

AaronHForgeFlow · 2017-11-03T12:22:55Z

operating_unit/README.rst

+
+* Eficent <contact@eficent.com>
+* Serpent Consulting Services Pvt. Ltd. <support@serpentcs.com>
+


You can add yourself as contributor

It doesn't matter. I don't usually unless its a substantial amount of work.

AaronHForgeFlow · 2017-11-08T16:25:45Z

@gdgellatly regarding the issue mentioned, the partner means to contain the operating unit information. That is, address, logo, phone all contact information that can be used to customized printouts by operating unit or provide the address to other contacts.

gdgellatly · 2017-11-08T21:13:54Z

@aheficent alright simple enough, I added a help message to partner id and removed the known issues

LoisRForgeFlow

LGTM 👍

gustavovalverde · 2018-02-05T17:55:40Z

As far as I tested, this seems OK for the first migration. Do you need / want further testing before making it available for v11?

AaronHForgeFlow · 2018-02-06T12:56:28Z

@gustavovalverde if you think this is ok to be merged you can approved this PR and then it will be merged. Currently it has 3 approvals but all of them are from @eficent team

AaronHForgeFlow · 2018-02-07T14:28:33Z

@gdgellatly there are too many commits from older versions, can you squash those a little bit?

gdgellatly · 2018-02-07T20:25:57Z

@aheficent Nope, it keeps failing on rebase.

simahawk · 2018-02-08T07:36:18Z

operating_unit/README.rst

+To configure this module, you need to:
+
+* Assign *Multi Operating Unit* group to user.
+* Go to *Settings / Companies / Operating Units* and create Operating Units.


Settings / Users & Companies / Operating Units

simahawk · 2018-02-08T07:36:29Z

operating_unit/README.rst

+
+.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
+   :alt: Try me on Runbot
+   :target: https://runbot.odoo-community.org/runbot/213/10.0


simahawk · 2018-02-08T07:37:28Z

operating_unit/__init__.py

@@ -0,0 +1,5 @@
+# © 2015-2017 Eficent


replace ©️ w/ "Copyright"

simahawk · 2018-02-08T07:37:38Z

operating_unit/__manifest__.py

+    "author": "Eficent, "
+              "Serpent Consulting Services Pvt. Ltd.,"
+              "Odoo Community Association (OCA)",
+    "website": "http://www.eficent.com",


https://github.com/OCA/operating-unit

simahawk · 2018-02-08T07:38:47Z

operating_unit/models/__init__.py

@@ -0,0 +1,7 @@
+# -*- coding: utf-8 -*-


remove coding from *.py

simahawk · 2018-02-08T07:42:28Z

operating_unit/models/res_users.py

+    @api.model
+    def operating_unit_default_get(self, uid2):
+        if not uid2:
+            uid2 = self._uid


user = self.env.user if uid2: user = self.env['res.users'].browse(uid2)

simahawk · 2018-02-08T07:42:56Z

operating_unit/models/res_users.py

+
+    @api.model
+    def _get_operating_unit(self):
+        return self.operating_unit_default_get(self._uid)


isn't this the same as passing nothing?

@simahawk Honestly I don't know, not the original author and I just couldn't understand why it was like it was. Plus I've not looked at this in months. But it is different, passing nothing would give an error.

I think I changed most of this file, then reverted it because other modules used this code.

But in practice I've found these methods are unsuitable anyway and have rewritten because the only time you really care about a users default operating unit is in creation of a business document, and then you get all sorts of weird issues (like in a sales order, the warehouse, Sales Team, salesperson and the user entering might all belong to different OU's and in multicompany it is a nightmare).

EDIT: I guess what I'm saying is I don't even know why these methods exist, especially the plural one.

@gdgellatly we are already doing it https://github.com/OCA/operating-unit/blob/10.0/sale_operating_unit/models/sale.py#L14
This is implementation is really old, there's no reason to keep it as it is.
The ones that rely on this old way will have to migrate modules to v11 as well.
So, this is the right time to make this change ;)

Same goes for the indirection of default methods: ppl used to add 2 methods to make them overridable but if you use lambda to proxy the call you don't need the double step anymore.

simahawk · 2018-02-08T07:44:19Z

operating_unit/models/res_users.py

+
+    @api.model
+    def _get_operating_units(self):
+        return self._get_operating_unit()


use a lambda to proxy the method call instead of referencing the method's object:

default=lambda self: self._default_operating_units() [...] default=lambda self: self._default_operating_unit()

Note the change in the name of the methods ;)

simahawk · 2018-02-08T07:46:26Z

operating_unit/security/operating_unit_security.xml

+    <record id="ir_rule_operating_unit_allowed_operating_units"
+            model="ir.rule">
+        <field name="model_id" ref="model_operating_unit"/>
+        <field name="domain_force">[('id','in',[g.id for g in user.operating_unit_ids])]</field>


no need for list comprehension, use .ids: [('id','in', user.operating_unit_ids.ids])]

simahawk · 2018-02-08T07:47:29Z

operating_unit/view/operating_unit_view.xml

+        <field name="model">operating.unit</field>
+        <field name="arch" type="xml">
+            <form string="Operating Unit" version="7.0">
+                <group>


name groups, for the sake of future extension (when you'll add a new group you will copy paste it w/ a name attr ;))

ghost · 2018-02-08T14:25:44Z

operating_unit/models/operating_unit.py

+    @api.model
+    def name_search(self, name='', args=None, operator='ilike', limit=100):
+        # Make a search with default criteria
+        names1 = super(models.Model, self).name_search(


calls to super() no longer require arguments as of Python3 🙂

edsersolis · 2018-02-14T01:31:41Z

When will this be merged?

elicoidal · 2018-02-14T03:55:23Z

@simahawk @gdgellatly anything that can be added to improve the original PR or shall we add some FIXME in the code?

simahawk · 2018-02-14T07:31:25Z

@elicoidal to me those methods should be fixed now before merge. Is not a big change BTW.
I'm not blocking tho, if we put some #FIXME comment 😉

gdgellatly · 2018-02-14T08:57:10Z

I'll fix them. Just got new laptop so it takes a while to set up.

…

On Wed, 14 Feb 2018, 8:31 PM Simone Orsi, ***@***.***> wrote: @elicoidal <https://github.com/elicoidal> to me those methods should be fixed now before merge. Is not a big change BTW. I'm not blocking tho, if we put some #FIXME comment 😉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#99 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE23kPQShdNACO57qPLDX-unpw-ddR3sks5tUovOgaJpZM4QQmqw> .

oca-clabot · 2018-02-15T02:45:57Z

Hey @gdgellatly, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Graeme Gellatly (no github login found)

Appreciation of efforts,
OCA CLAbot

gdgellatly · 2018-02-15T02:49:19Z

is CLA a me or you issue?

AaronHForgeFlow · 2018-02-15T08:09:56Z

@gdgellatly the problem is the last commit I think. The email address of the author it's different as the one it's registered. It will be solved by amending the last commit and changing the author.

gdgellatly · 2018-02-18T22:13:32Z

@simahawk happy now :). btw, ignore the CLA message, I force committed after. New laptop had wrong git config.

JordiBForgeFlow · 2018-02-18T22:15:19Z

Can you rebase? I am not able to merge.

gdgellatly · 2018-02-18T22:17:00Z

@jbeficent I can try, but should be nothing to rebase.

JordiBForgeFlow · 2018-02-18T22:19:30Z

I am getting a message:
This branch cannot be rebased safely
Rebasing the commits of this branch on top of the base branch cannot be performed automatically as this would create a different result than a regular merge.

gdgellatly · 2018-02-18T22:31:47Z

@jbeficent don't worry, new laptop issues, had remotes around wrong way, will fix up now

…per the OCA guidelines.

…uggestions.

* [MIG] operating_unit to v10.0

gdgellatly · 2018-02-18T23:14:25Z

@simahawk @jbeficent Wow I made some messes for myself. Had been force pushing to this repo on 2 seperate branches mig-operating-unit and mig-operating_unit and then through some magic these were merging into each other. And then my upstream repo wasn't oca but a personal one. I seem to recall why this arrangement made sense when I started this, but months later forgot I did it.

So what I did was copy the files from the last branch I was working on which was what was reviewed.
Delete original branches.
Restarted migration process from scratch.
Copy files back
Commit
Force push.

So please just give it one final check and make sure I've not messed/missed anything in this process. I've given it a run through and it seems fine.

AaronHForgeFlow

👍 From my side this is ready to merge.

simahawk

@gdgellatly seems good. Just 2 small comments. Most important: translation of sql constraints.

simahawk · 2018-02-19T08:33:16Z

operating_unit/models/operating_unit.py

+
+    _sql_constraints = [
+        ('code_company_uniq', 'unique (code,company_id)',
+         'The code of the operating unit must '


make this translatable?

simahawk · 2018-02-19T08:33:24Z

operating_unit/models/operating_unit.py

+    code = fields.Char('Code', required=True)
+    active = fields.Boolean('Active', default=True)
+    company_id = fields.Many2one(
+        'res.company', 'Company', required=True, default=lambda self:


edsersolis

All seems good.

gdgellatly mentioned this pull request Nov 3, 2017

11.0 mig sale team operating unit #100

Merged

1 task

pedrobaeza mentioned this pull request Nov 3, 2017

Migration to version 11.0 #94

Closed

11 tasks

pedrobaeza added this to the 11.0 milestone Nov 3, 2017

pedrobaeza added the needs review label Nov 3, 2017

gdgellatly mentioned this pull request Nov 3, 2017

[11.0] mig analytic operating unit #101

Merged

2 tasks

AaronHForgeFlow approved these changes Nov 3, 2017

View reviewed changes

MiquelRForgeFlow approved these changes Nov 7, 2017

View reviewed changes

LoisRForgeFlow approved these changes Nov 9, 2017

View reviewed changes

tschanzt approved these changes Feb 7, 2018

View reviewed changes

simahawk requested changes Feb 8, 2018

View reviewed changes

ghost approved these changes Feb 8, 2018

View reviewed changes

edsersolis approved these changes Feb 14, 2018

View reviewed changes

sudhir-serpentcs and others added 20 commits February 19, 2018 11:53

[ADD] Added migrated Operating Unit module for v9.

c2bb110

[IMP] Improved the license terms and improved the module standard as …

a707172

…per the OCA guidelines.

[ADD] Renamed README.md to README.rst

7416738

[IMP] Assign multi_ou grup and add OCA logo.

e4a4d2f

[FIX] remove description from __openerp__.py

35b71a0

[FIX] Corrections as per review comments

9f685e9

[ADD] Added travis yml file and changed the code as per travis test s…

7d3f39e

…uggestions.

[IMP] Added name_search to search the OU based on name and code.

0dc6f28

[FIX] Review as per @dreispt comments

1b74868

[FIX] Review as per travis

6ae6b80

[FIX] flake8

d126e23

[IMP] improve name_search

e0d0049

[IMP] changed readme.rst

f63c8c6

[IMP] changed Eficent copyright

6698350

[IMP] removed size from char fields. General improvement to XML style.

57ba5d7

[MIG] Make modules uninstallable

a4e11fe

[MIG] Rename manifest files

07bfa0e

[MIG] operating_unit to v10.0 (#45)

4c7112a

* [MIG] operating_unit to v10.0

A user can only select the operating units that he is allowed (#59)

bc17500

Port to v11 - retry

b480a94

AaronHForgeFlow approved these changes Feb 19, 2018

View reviewed changes

simahawk requested changes Feb 19, 2018

View reviewed changes

edsersolis approved these changes Feb 19, 2018

View reviewed changes

JordiBForgeFlow merged commit f31a1cf into OCA:11.0 Feb 20, 2018


		* Eficent <contact@eficent.com>
		* Serpent Consulting Services Pvt. Ltd. <support@serpentcs.com>

[11.0] mig operating unit #99

[11.0] mig operating unit #99

Conversation

gdgellatly commented Nov 3, 2017

gdgellatly commented Nov 3, 2017

AaronHForgeFlow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronHForgeFlow commented Nov 8, 2017

gdgellatly commented Nov 8, 2017

LoisRForgeFlow left a comment

Choose a reason for hiding this comment

gustavovalverde commented Feb 5, 2018

AaronHForgeFlow commented Feb 6, 2018

AaronHForgeFlow commented Feb 7, 2018

gdgellatly commented Feb 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdgellatly Feb 8, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edsersolis commented Feb 14, 2018

elicoidal commented Feb 14, 2018

simahawk commented Feb 14, 2018

gdgellatly commented Feb 14, 2018 via email

oca-clabot commented Feb 15, 2018

gdgellatly commented Feb 15, 2018

AaronHForgeFlow commented Feb 15, 2018

gdgellatly commented Feb 18, 2018

JordiBForgeFlow commented Feb 18, 2018

gdgellatly commented Feb 18, 2018

JordiBForgeFlow commented Feb 18, 2018

gdgellatly commented Feb 18, 2018

gdgellatly commented Feb 18, 2018

AaronHForgeFlow left a comment

Choose a reason for hiding this comment

simahawk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edsersolis left a comment

Choose a reason for hiding this comment

gdgellatly Feb 8, 2018 •

edited