Integrate partner_contact_address to cooperator module #350

robinkeunen · 2022-07-27T12:53:10Z

Goal
Remove partner_contact_address dependencies of cooperator since it is not in the oca.
partner_contact_address adds a representative type to partners. Other types are

class Partner(models.Model):

    type = fields.Selection(
        [('contact', 'Contact'),
         ('invoice', 'Invoice address'),
         ('delivery', 'Shipping address'),
         ('other', 'Other address'),
         ("private", "Private Address"),
        ], string='Address Type',

Suggestion

My first idea was

add the selection type to cooperator module
remove the dependency of cooperator to partner_contact_address
uninstall the module at lesptitspots
delete the module partner_contact_address from cie-addons.

BUT

While doing that, I realise that cooperator module also

adds boolean field representative
adds computed boolean field representative_of_member_company

Questions to the analysts before I start

is there a difference between the the boolean field and the partner type ? In the code, they seem to be used interchangeably.
if no difference, which field should we keep ?
is there in version 12+ a way to mark a contact as representative in Odoo CE (Community Edition) ? If so, maybe we should remove both the boolean field and the representative type.

Other information

the boolean field was added in January 2018
the type selection was added in June 2018
in BEES database, there are 48 partners with representative==True. 10 of those are not of type "cooperator".
in synergie database, all representative partners are of type "cooperator"

The representative_of_member_company field is used to open the view "Company representative" from the menu. We should keep it.

Miscellaneous notes

partner_contact_address is only installed on databases that use cooperator except for lesptitspots.
Most contacts of lesptitspots are representative which does not make much sense. It's probably a mistake since "representative" is made the default value to the partner type field by the partner_contact_address module.

The text was updated successfully, but these errors were encountered:

polchampion · 2022-08-16T13:58:40Z

@robinkeunen I don't have a full picture yet, but here's below what I understand.

But first, is this work on v12, v14 or both at the same time? This module was just ported to v14, so I guess a refactoring would need to affect v14 as well.

* is there a difference between the the boolean field and the partner type ? In the code, they seem to be used interchangeably.

For me, there is a difference by design: "adding a contact as representative" is meant as a general feature and put in the module partner_contact_address (nothing to do with cooperator), while the representative boolean flags the legal representative of a company for the purposes of cooperator (limited to 1 legal representative).
The question is, 'is there a need for the representative type as in the partner_contact_address module on its own ?'.
I'm not sure it is.
BEES uses the ability to have multiple representatives for one company, but I imagine they could do the same with a normal "contact' type. Difficult to know if the representative type is used because it's the by-default type, as you said. (note : from v14, it's not default anymore)

* if no difference, which field should we keep ?

If there is no need for a 'representative' type outside of cooperator, and if it's okay to limit the representative to 1 per company, and if it's possible to prevent a user from manually adding a contact of type 'representative', then the representative boolean is probably useless. We should probably check that with clients and test different scenarii - for example how to handle a change of representative ?

* is there in version 12+ a way to mark a contact as representative in Odoo CE (Community Edition) ? If so, maybe we should remove both the boolean field and the `representative` type.

Not that I could find. However, if the need behind the representative type is to have an address, there seems to be a module that does this (is this why partner_contact_address cannot be proposed to OCA? - if not that, why?)

The representative_of_member_company field is used to open the view "Company representative" from the menu. We should keep it.

I don't understand what that is.

partner_contact_address is only installed on databases that use cooperator except for lesptitspots. Most contacts of lesptitspots are representative which does not make much sense. It's probably a mistake since "representative" is made the default value to the partner type field by the partner_contact_address module.

Let's ask someone who knows more about lespetitspots.

robinkeunen · 2022-08-17T08:34:39Z

@polchampion

But first, is this work on v12, v14 or both at the same time? This module was just ported to v14, so I guess a refactoring would need to affect v14 as well.

Yes, it will have to affect both versions.

is there a difference between the the boolean field and the partner type ? In the code, they seem to be used interchangeably.

For me, there is a difference by design: "adding a contact as representative" is meant as a general feature and put in the module partner_contact_address (nothing to do with cooperator), while the representative boolean flags the legal representative of a company for the purposes of cooperator (limited to 1 legal representative).

From what you say, I still don't see the difference. It seems to me these are two technical solutions to address the same functional problem. Moreover, cooperator depends on partner_contact_address.

As is, there are no limits whatsoever to the number of representative in the code. I would have thought that we would need one to know whom to address the communication to but I seem to be wrong on that.

The question is, 'is there a need for the representative type as in the partner_contact_address module on its own ?'. I'm not sure it is. BEES uses the ability to have multiple representatives for one company, but I imagine they could do the same with a normal "contact' type. Difficult to know if the representative type is used because it's the by-default type, as you said. (note : from v14, it's not default anymore)

I don't think there is a need since partner_contact_address is only used on databases that use cooperator except for lesptitspots which doesn't seem to use it correctly (cf lower comment).

if no difference, which field should we keep ?

If there is no need for a 'representative' type outside of cooperator, and if it's okay to limit the representative to 1 per company, and if it's possible to prevent a user from manually adding a contact of type 'representative', then the representative boolean is probably useless. We should probably check that with clients and test different scenarii - for example how to handle a change of representative ?

As said above, there are no constraints at this time on the number of representatives for a company. Both the boolean and the type are used throughout the code.

is there in version 12+ a way to mark a contact as representative in Odoo CE (Community Edition) ? If so, maybe we should remove both the boolean field and the representative type.

Not that I could find. However, if the need behind the representative type is to have an address, there seems to be a module that does this (is this why partner_contact_address cannot be proposed to OCA? - if not that, why?)

This is no exactly what the partner_contact_address_default module does. It sets up a default delivery and invoice address on partners. If I were to push this to the OCA, I would rename the module partner_representative or something like that. I don't want to put it on the OCA because I don't see the use outside of cooperator.

The representative_of_member_company field is used to open the view "Company representative" from the menu. We should keep it.

I don't understand what that is.

It's a view in the Cooperator Application that displays all the representative contacts of member companies.

vdewulf · 2022-08-17T08:41:57Z

@robinkeunen and @polchampion
I didn't ready all your text, I have too much to do ritght now.
My knoledge about the topic:

representative boolean was meant to create the company subscription and being able to link an existing person from the database to a company
representative type was meant to remove the side effect of the "normal" link between a company and an individual in Odoo, whici is that the personal data are removed and replaced by the company ones. So a individual coop lost his personal info if he was also representative for a company becoming cooperator.

For "lesptitspots", Cath did the implementation and I don't know why a module was or was not installed, and I don't know why it was made by default a representative type. They have an odoo consultant volunteer, I might have tried some things. I don't think we need to spend a lot of time on their database (the consultant told me he would advise to not touch anymore the functionalities of the database because they have no money).

victor-champonnois · 2022-09-01T13:38:40Z

Recap discussion with @polchampion

Let's split the problem in two parts and process sequentially:

Remove the dependency and include the representative type to cooperator. The only issue is the lesptitpots DB: a lot of contact have a "representative" address type, which would break if the partner_contact_address module is removed. The solution would be just to change their address type to "Personal address" for instance, with an SQL query.
- We should check that it does not break anything ? -> Searching in the codebase where partner's "type" is used.
- Maybe we should check with @cathLemb because she did the implementation ?
Considering a cooperator refactoring, for two reasons:
- The "representative" boolean and the "representative" address type are maybe redundant?
  - We need to see how these two variables are used in the code.
  - For instance could we use the "Personal address" address type, remove the representative address type and keep the representative boolean?
- The address type is "representative" by default, so any contact that is not a child contact has a "representative" address type. (to see this go to Contact in a DB with cooperator installed and filter by address type == representative). This is misleading and could cause problem if the address type is used in the code.
This is a complicated issue that would demand a lot of work. It is still obscure to me.

robinkeunen · 2022-09-01T13:56:37Z

Part 1 is enough for now. Part 2 can be put in ROADMAP.rst and and in an issue for the time being.

I'm 95% sure lesptitspots is not aware of this module.

victor-champonnois · 2022-09-01T14:01:26Z

1. Remove the dependency and include the representative type to cooperator. The only issue is the lesptitpots DB: a lot of contact have a "representative" address type, which would break if the partner_contact_address module is removed. The solution would be just to change their address type to "Personal address" for instance, with an SQL query.
   
   * We should check that it does not break anything ? -> Searching in the codebase where partner's "type" is used.

I checked in all the *partner.py files and the type doesn't seem to be used in any logic. Because the field name "type" is so generic, I could not check in all the codebase.

I ran update res_partner set type = 'private' where type = 'representative'; on the lesptitspots-test-representative DB. @polchampion Could you check if it breaks something ?

victor-champonnois · 2022-09-02T07:30:24Z

PR in 12.0:
#362
coopiteasy/addons#261

PR in 14.0:
#363
coopiteasy/addons#262

polchampion · 2022-09-08T09:32:14Z

@victor-champonnois Can't access lesptitspots-test-representative: the html on the login page is broken and I get a white screen after logging in

victor-champonnois · 2022-09-08T13:36:34Z

@polchampion it's fixed !

polchampion · 2022-09-09T06:52:53Z

@victor-champonnois LGTM
Tested on lesptitspots-representative

Open a contact
Link a personal address to another personal address as parent/child
Default address type is personal address for new contacts and linked contacts

victor-champonnois · 2022-09-13T15:01:53Z

@polchampion should I put the deploy note in a task ?
Not ready !!!
Deploy note:

merge the two PR :
[12.0][REF] include representative address type in cooperator #362
[REM] partner_contact_address module (refactored in cooperator) addons#261
update cie-repositories+run gitaggregate
update all with restart (the module partner_contact_address will now be in "to update" state)
run the following script on all DBs (after the update all and restart !)
edit : new version:

import logging

_logger = logging.getLogger(__name__)

# undefined name 'env'
env = env  # noqa: F821

module_to_uninstall_names = [
    "partner_contact_address",
]
modules_to_uninstall = env["ir.module.module"].search(
    [
        (
            "name",
            "in",
            module_to_uninstall_names,
        ),
        (
            "state",
            "!=",
            "uninstalled",
        ),
    ]
)
for module in modules_to_uninstall:
    _logger.info("uninstall %s", module.name)
    try:
        module.button_immediate_uninstall()
    except Exception:
        _logger.exception("failed to uninstall %s", module.name)
env.cr.commit()

robinkeunen · 2022-09-13T15:22:30Z

@victor-champonnois If ran through the shell, it will need a env.cr.commit()

victor-champonnois · 2022-09-14T08:40:55Z

@polchampion
We need to be careful with this one because it is the same kind of stuff that broke the bees DB when we deployed the komunigi refactoring. Except now we know that the script must be run after the updates :-)

I ran the script on all DB in test.
It fails on some backup DBs (e.g. spp-test-theo-2022-02-28 ,oufticoop-test-T8621, coopiteasy-test-2022-05-31), but not for up to date DBs.
Could you test for a few DBs that partner_contact_address is effectively uninstalled and that everything is OK (cooperator still there, no apparent data loss) ?

polchampion · 2022-09-15T07:53:40Z

@victor-champonnois Sur lasource-test, j'ai :

créé une demande de souscitpion pour un partenaire 'test test' : le contact créé est une adresse de type contact --> ok
créé une demande de souscription pour une société 'Société de test' : j'ai une erreur copiée ci-dessous.

 Error code: 200 
 Error message: Odoo Server Error 
 Error data message:
 Vous ne pouvez pas créer de hiérarchie de partenaires récursive.
None 
 Error data debug:
 Traceback (most recent call last):
  File "/home/odoo12/src/odoo/odoo/http.py", line 656, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/home/odoo12/src/odoo/odoo/http.py", line 314, in _handle_exception
    raise pycompat.reraise(type(exception), exception, sys.exc_info()[2])
  File "/home/odoo12/src/odoo/odoo/tools/pycompat.py", line 87, in reraise
    raise value
  File "/home/odoo12/src/odoo/odoo/http.py", line 698, in dispatch
    result = self._call_function(**self.params)
  File "/home/odoo12/src/odoo/odoo/http.py", line 346, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/home/odoo12/src/odoo/odoo/service/model.py", line 98, in wrapper
    return f(dbname, *args, **kwargs)
  File "/home/odoo12/src/odoo/odoo/http.py", line 339, in checked_call
    result = self.endpoint(*a, **kw)
  File "/home/odoo12/src/odoo/odoo/http.py", line 941, in __call__
    return self.method(*args, **kw)
  File "/home/odoo12/src/odoo/odoo/http.py", line 519, in response_wrap
    response = f(*args, **kw)
  File "/home/odoo12/src/odoo/addons/web/controllers/main.py", line 971, in call_button
    action = self._call_kw(model, method, args, {})
  File "/home/odoo12/src/odoo/addons/web/controllers/main.py", line 959, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/home/odoo12/src/odoo/odoo/api.py", line 759, in call_kw
    return _call_kw_multi(method, model, args, kwargs)
  File "/home/odoo12/src/odoo/odoo/api.py", line 746, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/home/odoo12/src/coopiteasy/obeesdoo/beesdoo_easy_my_coop/models/subscription_request.py", line 42, in validate_subscription_request
    invoice = super().validate_subscription_request()[0]
  File "/home/odoo12/src/coopiteasy/vertical-cooperative/cooperator/models/subscription_request.py", line 760, in validate_subscription_request
    contact.write({"parent_id": partner.id, "representative": True})
  File "/home/odoo12/src/coopiteasy/obeesdoo/eater/models/partner.py", line 45, in write
    return super(Partner, self).write(values)
  File "/home/odoo12/src/coopiteasy/obeesdoo/beesdoo_shift/models/res_partner.py", line 166, in write
    result = super(ResPartner, self).write(vals)
  File "/home/odoo12/src/odoo/odoo/addons/base/models/res_partner.py", line 564, in write
    result = result and super(Partner, self).write(vals)
  File "/home/odoo12/src/odoo/addons/mail/models/mail_thread.py", line 321, in write
    result = super(MailThread, self).write(values)
  File "/home/odoo12/src/odoo/addons/mail/models/mail_activity.py", line 621, in write
    return super(MailActivityMixin, self).write(vals)
  File "/home/odoo12/src/odoo/odoo/models.py", line 3366, in write
    self._write(store_vals)
  File "/home/odoo12/src/odoo/odoo/models.py", line 3502, in _write
    self._validate_fields(vals)
  File "/home/odoo12/src/odoo/odoo/models.py", line 1128, in _validate_fields
    check(self)
  File "/home/odoo12/src/odoo/odoo/addons/base/models/res_partner.py", line 347, in _check_parent_id
    raise ValidationError(_('You cannot create recursive Partner hierarchies.'))
odoo.exceptions.ValidationError: ('Vous ne pouvez pas créer de hiérarchie de partenaires récursive.', None)```

victor-champonnois · 2022-09-15T12:58:32Z

@polchampion le problème arrive déjà en 12.0. Je créé une issue à part là dessus.

This was referenced Jul 28, 2022

Move modules to the OCA #359

Open

General project management - komunigi - 2 - technical axis #204

Open

victor-champonnois mentioned this issue Sep 15, 2022

Error when validating a company sub request related to an existing cooperator #371

Closed

2 tasks

github-grap-bot closed this as completed in #362 Sep 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate partner_contact_address to cooperator module #350

Integrate partner_contact_address to cooperator module #350

robinkeunen commented Jul 27, 2022 •

edited

Loading

polchampion commented Aug 16, 2022

robinkeunen commented Aug 17, 2022

vdewulf commented Aug 17, 2022

victor-champonnois commented Sep 1, 2022 •

edited

Loading

robinkeunen commented Sep 1, 2022

victor-champonnois commented Sep 1, 2022

victor-champonnois commented Sep 2, 2022

polchampion commented Sep 8, 2022

victor-champonnois commented Sep 8, 2022

polchampion commented Sep 9, 2022 •

edited by victor-champonnois

Loading

victor-champonnois commented Sep 13, 2022 •

edited

Loading

robinkeunen commented Sep 13, 2022

victor-champonnois commented Sep 14, 2022 •

edited

Loading

polchampion commented Sep 15, 2022 •

edited

Loading

victor-champonnois commented Sep 15, 2022

Integrate partner_contact_address to cooperator module #350

Integrate partner_contact_address to cooperator module #350

Comments

robinkeunen commented Jul 27, 2022 • edited Loading

polchampion commented Aug 16, 2022

robinkeunen commented Aug 17, 2022

vdewulf commented Aug 17, 2022

victor-champonnois commented Sep 1, 2022 • edited Loading

robinkeunen commented Sep 1, 2022

victor-champonnois commented Sep 1, 2022

victor-champonnois commented Sep 2, 2022

polchampion commented Sep 8, 2022

victor-champonnois commented Sep 8, 2022

polchampion commented Sep 9, 2022 • edited by victor-champonnois Loading

victor-champonnois commented Sep 13, 2022 • edited Loading

robinkeunen commented Sep 13, 2022

victor-champonnois commented Sep 14, 2022 • edited Loading

polchampion commented Sep 15, 2022 • edited Loading

victor-champonnois commented Sep 15, 2022

robinkeunen commented Jul 27, 2022 •

edited

Loading

victor-champonnois commented Sep 1, 2022 •

edited

Loading

polchampion commented Sep 9, 2022 •

edited by victor-champonnois

Loading

victor-champonnois commented Sep 13, 2022 •

edited

Loading

victor-champonnois commented Sep 14, 2022 •

edited

Loading

polchampion commented Sep 15, 2022 •

edited

Loading