[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!

[rousseldenis]

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

https://github.com/OCA/margin-analysis/issues/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). 🤖

[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). :sweat_smile:

[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?

[yajo]

IIUC https://github.com/OCA/margin-analysis/pull/223#pullrequestreview-2527818423, 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?

[ghost]

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

[rousseldenis]

Hello all,

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

Best regards 😄 🙏🏼 ❤️

@moduon MT-8486

I finally reverted as the yellow line case is not possible as groups should imply the preceding to be shown as select box.

@yajo @rafaelbn IMHO, if you want to get the margin access and costs in one operation, you should think about using base_user_role module.

[rousseldenis]

At this time, the groups are managed like this:

like suggested

[github-actions[bot]]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.