[16.0][IMP] sign_oca: Improvements migrated from 14.0

[PauBForgeFlow]

Migrated PRs:
#8 #17 #18 #19 #20 #21 #24 #26 #27 #30

[OCA-git-bot]

Hi @etobella,
some modules you are maintaining are being modified, check this out!

[victoralmau]

Thank you for doing this work to v16.

Some comments (my functional review is still pending).

[victoralmau]

Some things I have seen do not work in v16:

• The "Sign from template" action that is automatically added in the tree and form view does not work: [14.0][IMP] sign_oca: Add Sign from template action from tree and form view #20
• Systray management does not work: [14.0][IMP] sign_oca: Systray management + user rules + wizard to create requests #8
• The action_signer_sign_url button in the form view of a request should only be shown in the sent state.

Other things I have not been able to check.

[victoralmau]

Some comments to keep in mind:

• Do not add commits that are not yet merged in v14.
• Migration script will not be needed (it has been added in [15.0][MIG] sign_oca: Migration to 15.0 #25).
• Systray does not work, it will be necessary to make the same changes as done in [15.0][MIG] sign_oca: Migration to 15.0 #25 (when merged).

[BernatPForgeFlow]

@victoralmau I'm checking the PR but still seem to find something not completely finished. The problem is that when we generate a signature request from the 'Sign from template' action, it doesn't create the signers. It seems to be something that has already been commented here but it's not quite clear to me: #25 (comment)

From my point of view, in the sign_oca code we should generate by default all the signers defined in the template with the active user that is throwing the action. And later, in any case, it could be inherited from the method that generates the signers to define a different logic. So, the 'Sign from Template' feature should work on its own without the need for another module.

Finally, I would also like to ask what exactly is the 'partner_type' field inside the 'sign.oca.role' model? I don't fully understand how to use it. Is it necessary?

[victoralmau]

Currently everything is correct although I understand the confusion that exists (perhaps the readme should be improved).

I explain a little bit the different use cases:

1- Sign_now

A request will be created defining as signer the user that makes it.

2- Send

A wizard will be created to define the desired signers manually.

3- Sign from template

A wizard will be created that when selecting a template will generate a request with the "corresponding" signers. The concept of signers does not exist in the templates, it is something that generates from the roles and the type (fixed partner or regular expression). You can check #32 to understand it better.

[BernatPForgeFlow]

@victoralmau Ok, I understand how it works and it looks good to me. The only thing that makes me hesitate is that the field is called 'Partner type' as it is not intuitive. How about we change it to 'Partner Selection Policy'? And in addition we add a help explaining a little how it works.

Other than that, what is the use of the 'Empty' option? I don't see any use for it, I would change 'Empty' to 'Active User'. By default, if the role is not configured with another partner selection policy, it would take the active user. Thus, any user can select records from a model and sign them on their behalf.

Lastly, when I use the 'Default' option it seems to fail.

Traceback (most recent call last):
  File "/mnt/data/odoo-addons-dir/sign_oca/wizards/sign_oca_template_generate_multi.py", line 35, in generate
    requests = self._generate()
  File "/mnt/data/odoo-addons-dir/sign_oca/wizards/sign_oca_template_generate_multi.py", line 30, in _generate
    return self.env["sign.oca.request"].create(
  File "<decorator-gen-154>", line 2, in create
  File "/opt/odoo/odoo/api.py", line 415, in _model_create_multi
    return create(self, arg)
  File "/mnt/data/odoo-addons-dir/sign_oca/models/sign_oca_request.py", line 298, in create
    records = super().create(vals_list)
psycopg2.ProgrammingError: can't adapt type 'res.partner'

[victoralmau]

If you want to change the string of the field and add a perfect help, but the empty option should be kept and it is the "most frequent".

If we create a sign.request with the action "Sign from template" from a record (sale.order for example) the user that creates it should NOT be auto-added as signer; the ones that correspond with the defined in the template will be set.

The error you indicate I have not been able to reproduce it, can you indicate the steps to reproduce it and/or add a mini-video? If it is something that happens in 15.0 currently it should be solved.

[BernatPForgeFlow]

Okay thank you! Will add the changes!

If we create a sign.request with the action "Sign from template" from a record (sale.order for example) the user that creates it should NOT be auto-added as signer; the ones that correspond with the defined in the template will be set.

About this, why would you add an item in a template that is filled by a role whose partner selection policy is 'Empty'? It's difficult for me to imagine a situation in which you would configure it like that.
For me, it makes sense that the person who selects the sale order and clicks the action 'Sign from template' is the one who signs the request. It might be useful to validate objects like sale orders, pickings, etc.

A video showing how it is failing for me:
https://github.com/OCA/sign/assets/90243017/6b5d822f-8a63-4130-a9dd-19d70b05a6b6

[PauBForgeFlow]

@BernatPForgeFlow @victoralmau The error that you reported has been fixed. Also, the field partner_type has been renamed to partner_selection_policy.

[victoralmau]

About this, why would you add an item in a template that is filled by a role whose partner selection policy is 'Empty'? It's difficult for me to imagine a situation in which you would configure it like that. For me, it makes sense that the person who selects the sale order and clicks the action 'Sign from template' is the one who signs the request. It might be useful to validate objects like sale orders, pickings, etc.

IMO by default the user who creates it should not be added as a signer. I understand what you comment but there are several things that would not make it very coherent:

• What would be the role for the user (none would be correct).
• If a maintenance user (maintenance_sign_oca) for example creates a sign request from a template, it would NOT be correct to set him as signer, you must set the corresponding ones according to the template (the user of the equipment and a fixed user for example).

IMO it could be improved (in a different PR) sign_oca so that in the wizard from "Sign from template" the signers that will be set with that template are shown, being able to modify them and/or add new ones (for example the user's partner with the desired role).

[BernatPForgeFlow]

LGTM! As @victoralmau recommended I will open a separated PR to propose a new way to work with the 'Partner Selection Policy' in 'Sign from template' action.

[victoralmau]

Sorry, could you add a migration script to rename field partner_type to partner_selection_policy from sign.oca.role?

[BernatPForgeFlow]

Ping @pedrobaeza @etobella

[PauBForgeFlow]

@pedrobaeza This is now ready

[victoralmau]

Is everything correct now? IMO I think it is important not to delay merging and adding these changes in the 17.0 migration.

[pedrobaeza]

/ocabot merge major

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-22-by-pedrobaeza-bump-major, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 27ca9b9. Thanks a lot for contributing to OCA. ❤️