[12.0][ADD] auth_ldap_group_sync: alternative to users_ldap_groups

[alexey-pelykh]

[alexey-pelykh]

@nikul-serpentcs @hbrunn @dreispt I'd like to kindly ask for a review of this PR

[nikul-serpentcs]

Improve Code

[alexey-pelykh]

@nikul-serpentcs Thanks! Ready for re-review

[dreispt]

Since this seems to be proposing an alternative implementation to an existing feature, it begs the question:

• In what cases would this module be preferred, and what cases should users_ldap_groups be preferred?
• Why can't the distinctive features of this implementation be added to the existing module?

[alexey-pelykh]

@dreispt This module would be preferred in case Odoo permissions replicate LDAP group membership of single LDAP group granting multiple permissions in Odoo, with users_ldap_groups the configuration would be hard to maintain. While users_ldap_groups allows to configure attribute-based permission management sacrificing UX for flexibility.

I did an attempt to add these features in #46 yet reverted ti just-migration as original module lost its simplicity and flexibility or would become unmaintainable due to lots of if/else to support original behavior.

[hbrunn]

you can have both: Add a computed field to the original module that reads the original field and maps it to your simplified configuration and the inverse. Then declare a group that you use to restrict a view override which replaces the original field with the computed field. So just by adding/removing a user to/from the group, you can switch which field is shown. Make your group a standard implied group of base.group_user, and give the possibility to turn that off, something like 'show advanced configuration'.

[alexey-pelykh]

@hbrunn thanks for the suggestion! Adding this feature to original module was my original approach and in the end it proved itself to be overcomplicating the original codebase. Making an extra module that workarounds its purpose in order to be an extension of another module has no value IMHO. Thus if end-user needs LDAP-group-based permission management, there's one module, while advanced LDAP-attribute-based permission management there's another module.

[dreispt]

Some thoughts:

From a functional PoV, I can see this as a friendly UI over users_ldap_groups. I'm afraid that the feature duplication generates confusion for the people choosing modules from the OCA.

In an ideal world this module would wrap the users_ldap_groups technical features, hide advanced fields, and automate the filling of these technical fields. A security Group would be available to enable/make visible the advanced features, allowing finer control.

[alexey-pelykh]

Good point, I'd suggest to describe use-cases for both modules in readme. From code and functional perspective it would be hard to reuse users_ldap_groups codebase to implement these features, while keeping original behaviour intact for users who migrate.