[IMP] sale_margin_security: Allows more flexibility in groups

[rousseldenis]

As in some flows, people want to split fields visibility to buyers and salesmen, restore the margin group. As in cost module, there is two groups, add a group for sale margins + edit costs

[OCA-git-bot]

Hi @yajo, @rafaelbn, @sergio-teruel,
some modules you are maintaining are being modified, check this out!

[lmignon]

Granular security is easier to manage.... Thank you @rousseldenis

[rousseldenis]

@OCA/accounting-maintainers Could you link this to :

#222

Thanks

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[yajo]

In #222, we already touched upon this matter. From my perspective, this approach seems to undermine the intended security, as it effectively turns the security into a placebo. Anyone with access to margin will essentially have access to costs, which, in practical terms, means the same thing.

In the previous PRs from OCA/product-attribute#1538 (comment) and #198 (comment), we addressed the inconsistencies and security issues that made the previous status difficult to manage effectively. Thanks to those contributions, the UX has significantly improved, and we now have a simple, intuitive permissions dropdown:

This solution made security clear and functional right out of the box, which I believe was a positive step forward.

If we were to merge this new approach, however, we would return to the need for debug mode to configure security permissions, which could lead to inconsistent states. For example, here are some instances from Runboat:

By contrast, I think the thorough functional validation done by @Gelojr in #198 (review) demonstrates how the current approach ensures consistent behavior across the board.

That said, I still can see that adding an intermediate permission option to the dropdown could be a valuable enhancement, and it could look like this:

1. Read.
2. Edit only in SO.
3. Edit.

I understand there could be use cases where you might want salespeople to edit costs in the SO but not in the product. However, beyond this point, I believe adding more complexity would only lead to confusion and potential misconfigurations, without genuinely improving the system. For that reason, I don't agree with the proposed change in its current form.

In the end, my concern is that increasing complexity may not make the system easier to manage, but rather more prone to mistakes and misconfigurations.

MT-8486

[rafaelbn]

Hello! 😄

We would appreciate your concerns about maintaining flexibility without adding unnecessary complexity or creating a false sense of security. The key challenge is to offer more granular visibility options without making the configuration confusing or difficult to manage.

How do you think we could achieve this balance? For example, introduce the new permission level that give teams the control they need—without requiring debug mode or hidden configurations—while still keeping the user experience clear and straightforward?

The tooltip could be clear to explain the groups so our users don't mistake with them.

I’d really value your input on how to simplify and adapt these settings to better serve everyone’s needs.

I agree with @yajo , and also I can understand that something that for us our our clients don't have any sense for other have it. The goal is to fit everyone.

Thank you! ❤️

[rousseldenis]

Hi,

I didn't see the category in groups configuration.

I will add the category and configuration will show:

[rousseldenis]

@rafaelbn If I understand, your main concern is about user interface. And I understand it. We usually minimize the impact of configuration at that level (using base_user_role module or letting the administrator manages it by itself). 😅

[rousseldenis]

I understand there could be use cases where you might want salespeople to edit costs in the SO but not in the product.

That's not our use case. The reason of that change is about margin visibility (and I know costs people can compute them).

People that could have access to costs could not necessarily have access (direct) to margins.

[rousseldenis]

@rafaelbn The problem to dsiplay a selection is that the groups are not strictly hierarchical.

[yajo]

Indeed. Let's try to get to something more usable and sensible.

Only the costs fields are actually editable, because margins are computed, right? So, the edit mode on margins would make no sense IMHO.

Option A, if you want to split displaying or hiding margins, is to divide between a dropdown and a checkbox. Something like this:

However, IMHO it also doesn't make sense to let somebody actually edit the costs without being able to read the margins. If you have edit permissions, then I can imagine that you need to know the margin to be able to edit the cost appropriately, right? If so, this Option B would fit better:

As you can see, both designs feature a warning when the user is creating an inconsistency. This module is suffixed _security for a reason, and by allowing only one of the 2 read levels, you are only hiding the information, not blocking it. Thus it's important that the admin gets that warning clearly displayed.

WDYT?

[jdidderen-nsi]

I understand the changes and i don't think it hurts anyone to have it.
I agree in principle that having access to the cost gives an idea of the margin.
However, I don't see the issue with the approach proposed by Denis, as the old approach is still possible and the two can co-exist.

For the UX debate, can we simply have the following ?

[yajo]

IIUC #223 (review), your proposal is 4 levels of permissions, each one inheriting the previous one:

1. Nothing.
2. Read costs.
3. Read costs and margin.
4. Edit costs and margin.

Is that correct? Would that be good for you @rousseldenis?

[jdidderen-nsi]

IIUC #223 (review), your proposal is 4 levels of permissions, each one inheriting the previous one:

1. Nothing.
2. Read costs.
3. Read costs and margin.
4. Edit costs and margin.

Is that correct? Would that be good for you @rousseldenis?

Module categories :

• Cost Security (new one)
  • Product Costs (existing one)
  • Sale Margins (new one)

Security groups :

• Product Costs
  • Read (existing one)
  • Edit (existing one)
• Sale Margins
  • Show sale margin (new one)

And the new group has a inherit (implied_ids) on the read security group from product costs.

The edit security group from product costs is isolated and managed manually.

You can check it on the runboat linked to the PR.

[yajo]

Could you please explain to me why would somebody be able to edit costs and yet forbidden to see the margin? Isn't that essential information while editing costs? 🤔

[rafaelbn]

Hello all,

What I talked with @rousseldenis in a Google Meet was this result:

Best regards 😄 🙏🏼 ❤️

@moduon MT-8486