Base contact by function #20

bwrsandman · 2014-08-04T17:29:27Z

[ADD] added base_contact_function module: It allow to manage contacts by function"
[ADD] Added base_contact_function_partner_firstname module: It allows to add firstname before name.Name is considered like the lastname and we have firstname and lastname on partner view and contact view."
The base_contact_function_partner_firstname module depends on partner_firstname and base_contact_function module.

Previous reviews:

I have a hard time trying to find out what this module actually does. The manifest description is very terse. The name of the module reminds me of base_contact, which allowed multiple connections between the same person and a company. Is this module similar? The description should at least explain what 'institutions' are (a concept that this module introduces).

Terminology is off at times, it seems. l.130: 'acronym': is this abbreviation, or maybe stick to the standard 'code' field name and label?

Organism: I'm guessing you mean organization. But in any case, changing the 'company' terminology should be split off as a separate module.

The translations seem to contain obsolete terms, e.g. l.241 and other occurrences of 'passport'.

@bwrsandman

This is for cases where one partner (Mike Fletcher President of Agrolait) can be part of another company (Mike Fletcher VP of AsusTEK) in a different function.

The module helps track these functions.

coveralls · 2014-08-25T15:19:29Z

Coverage increased (+2.79%) when pulling ae90448 on savoirfairelinux:partner_contact_management into 664a53a on OCA:7.0.

nbessi · 2014-08-27T15:25:25Z

base_contact_by_functions/__init__.py

+from . import functions
+from . import res_institutions
+
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:


coveralls · 2014-08-27T16:13:09Z

Coverage increased (+2.79%) when pulling bd9314b on savoirfairelinux:partner_contact_management into 664a53a on OCA:7.0.

nbessi · 2014-08-29T10:39:41Z

👍

gurneyalex · 2014-09-02T08:45:36Z

base_contact_by_functions/functions.py

+
+class functions(orm.Model):
+    _description = 'Functions'
+    _name = 'functions'


I'm concerned this may be a bit too generic and confusing (function fields...). Also the convention nowadays is to use a singular form for Model _name.

I suggest "res.partner.function" for _name.

coveralls · 2014-09-02T15:10:51Z

Coverage increased (+2.79%) when pulling 8ad1ae5 on savoirfairelinux:partner_contact_management into 664a53a on OCA:7.0.

Allow to manage contacts by function

Add firstname before name. Name is considered like the lastname and we have firstname and lastname on partner view and contact view.

bwrsandman · 2014-11-03T19:12:18Z

@hbrunn addon's description provided by @giotta

coveralls · 2014-11-03T19:18:55Z

Coverage increased (+3.07%) when pulling 8fbcb1f on savoirfairelinux:partner_contact_management into 74927c5 on OCA:7.0.

bwrsandman · 2014-11-03T20:04:04Z

Inconsistent fail between Odoo and OCB

Functions for contacts of companies should be created in the Functions Tab. Functions created straight in the Contact form will not end up in the Functions Tab. Does not change behaviour on systems without web_m2x_options

hbrunn · 2014-12-08T10:19:59Z

base_contact_function/res_partner_category.py

+            name = name.split(' / ')[-1]
+            ids = self.search(cr, user, [('name', operator, name)] + args,
+                              limit=limit, context=context)
+            if ids:


this whole section doesn't respect the limit parameter, that will yield too many results when there are a lot of subcategories

coveralls · 2014-12-08T10:30:45Z

Coverage increased (+2.47%) when pulling 20d2472 on savoirfairelinux:partner_contact_management into 74927c5 on OCA:7.0.

hbrunn · 2014-12-08T10:49:22Z

base_contact_function/res_partner_category.py

+                    cr, user, [('parent_id', 'child_of', ids)],
+                    context=context)
+                if child_ids:
+                    ids.extend(child_ids)


the childof operator also returns the id itself, so add them only if not included already

hbrunn · 2014-12-08T10:51:38Z

@bwrsandman the difference is caused by ocb using browse() in name_get and odoo using read(). The latter discards double ids and gives a random(?) order, the former delivers all ids in the order of the id list.

So to fix the test, we shouldn't pass double ids to name_get as pointed out in my comment above, and we should only check for set equality (= no order)

bwrsandman · 2014-12-16T20:07:21Z

@hbrunn I have fixed the limits and the duplicates problem.

I don't understand this issue with id order, though. In this implementation, I used set to remove duplicates, this, however is unordered. Is this good?

hbrunn · 2014-12-17T08:15:41Z

I personally think that every name_get should return its list sorted alphabetically, but not all name_gets do that.

As for the issue at hand: The test fails because it checks for list equality (https://github.com/OCA/partner-contact/pull/20/files#diff-482719082e50caa32544883db16a8259R156) and there, order matters. Given that I don't think we care about the lists' orders, I think you'd better use assertItemsEqual: https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertItemsEqual

bwrsandman · 2014-12-17T14:27:08Z

Thanks @hbrunn.

As for the order, I thought odoo relied on the defined _order.

coveralls · 2014-12-17T14:35:32Z

Coverage increased (+2.51%) when pulling bff4fbe on savoirfairelinux:partner_contact_management into 74927c5 on OCA:7.0.

bwrsandman · 2014-12-17T14:54:23Z

The Travis CI build passed

🙌

hbrunn · 2014-12-17T15:56:11Z

yay! Concerning _order: search() uses it, and browse() returns records in the order it got in the id list, but read() returns the records as they come from the database.

Anyway: 👍

nbessi · 2014-12-19T07:50:17Z

👍

Base contact by function

hbrunn added the needs review label Aug 25, 2014

bwrsandman force-pushed the partner_contact_management branch from d0ac771 to 32b98d7 Compare August 25, 2014 14:40

nbessi reviewed Aug 27, 2014
View reviewed changes

gurneyalex reviewed Sep 2, 2014
View reviewed changes

gurneyalex added the needs fixing label Sep 2, 2014

bwrsandman force-pushed the partner_contact_management branch 3 times, most recently from b1ed151 to 8ad1ae5 Compare September 2, 2014 15:04

Sandy Carter and others added 2 commits October 31, 2014 18:37

[ADD] base_contact_function module

b43d2e2

Allow to manage contacts by function

[ADD] base_contact_function_partner_firstname

2b0ef42

Add firstname before name. Name is considered like the lastname and we have firstname and lastname on partner view and contact view.

bwrsandman force-pushed the partner_contact_management branch from 2ab513b to 2b0ef42 Compare October 31, 2014 22:37

Extend base_contact_function description

8fbcb1f

bwrsandman added needs review and removed needs fixing labels Nov 3, 2014

bwrsandman removed the work in progress label Nov 12, 2014

Sandy Carter and others added 2 commits November 24, 2014 16:54

Add optional web_m2x_options options

ffc5c2b

Functions for contacts of companies should be created in the Functions Tab. Functions created straight in the Contact form will not end up in the Functions Tab. Does not change behaviour on systems without web_m2x_options

Fix pylint

20d2472

hbrunn reviewed Dec 8, 2014
View reviewed changes

hbrunn added the needs fixing label Dec 8, 2014

Respect limits and remove duplicates

64c0713

bwrsandman removed the needs fixing label Dec 16, 2014

Don't test list order

bff4fbe

nbessi added a commit that referenced this pull request Dec 19, 2014

Merge pull request #20 from savoirfairelinux/partner_contact_management

a120d6e

Base contact by function

nbessi merged commit a120d6e into OCA:7.0 Dec 19, 2014

yvaucher removed the needs review label Dec 19, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Base contact by function #20

Base contact by function #20

bwrsandman commented Aug 4, 2014

coveralls commented Aug 25, 2014

nbessi Aug 27, 2014

coveralls commented Aug 27, 2014

nbessi commented Aug 29, 2014

gurneyalex Sep 2, 2014

coveralls commented Sep 2, 2014

bwrsandman commented Nov 3, 2014

coveralls commented Nov 3, 2014

bwrsandman commented Nov 3, 2014

hbrunn Dec 8, 2014

coveralls commented Dec 8, 2014

hbrunn Dec 8, 2014

hbrunn commented Dec 8, 2014

bwrsandman commented Dec 16, 2014

hbrunn commented Dec 17, 2014

bwrsandman commented Dec 17, 2014

coveralls commented Dec 17, 2014

bwrsandman commented Dec 17, 2014

hbrunn commented Dec 17, 2014

nbessi commented Dec 19, 2014

Base contact by function #20

Base contact by function #20

Conversation

bwrsandman commented Aug 4, 2014

Previous reviews:

coveralls commented Aug 25, 2014

nbessi Aug 27, 2014

Choose a reason for hiding this comment

coveralls commented Aug 27, 2014

nbessi commented Aug 29, 2014

gurneyalex Sep 2, 2014

Choose a reason for hiding this comment

coveralls commented Sep 2, 2014

bwrsandman commented Nov 3, 2014

coveralls commented Nov 3, 2014

bwrsandman commented Nov 3, 2014

hbrunn Dec 8, 2014

Choose a reason for hiding this comment

coveralls commented Dec 8, 2014

hbrunn Dec 8, 2014

Choose a reason for hiding this comment

hbrunn commented Dec 8, 2014

bwrsandman commented Dec 16, 2014

hbrunn commented Dec 17, 2014

bwrsandman commented Dec 17, 2014

coveralls commented Dec 17, 2014

bwrsandman commented Dec 17, 2014

hbrunn commented Dec 17, 2014

nbessi commented Dec 19, 2014