Rename "Show Exceptions" checkbox to "Only Show Exceptions" which more actually reflects its function

[luismrsilva]

Describe the bug

When editing tree permissions for a given user (or user group, although not tested), the "Show Exceptions" checkbox appears to behave as if it were "Only Show Exceptions".

In addition, it is checked by default; therefore, by default, only exceptions are shown. Because of this, a user might think that default policies which are not exceptions do not exist.

e.g. suppose the default policy is "Allow". If I want to change it to "Deny" for a tree, I'd have to the uncheck "Show Exceptions" checkbox to see the trees with default value in the list, which does not make sense.

To Reproduce

Steps to reproduce the behavior:

1. Go to Configuration -> Users -> (user) -> Tree Permissions;
2. Notice the "Show Exceptions" checkbox is checked by default;
3. Default policies are not shown. Only exceptions appear, if any.
4. Uncheck the "Show Exceptions" checkbox;
5. All the policies appear, including exceptions.

Expected behavior

When the "Show Exceptions" checkbox is checked, all values should be shown, not just the exceptions.

When presented with a list of "search results" and a checkbox saying "show X", the idea I get as a user is that having that checkbox marked will cause the results to show "X" in addition to other things. However, what is happening is that when the checkbox is marked, only X are shown. Therefore, the checkbox should read "Show Exceptions Only".
Also, I would argue that it should be unchecked by default, as to show everything explicitly.

Screenshots

• "Show exceptions" is checked. Exceptions would appear if there were any. The problem is that policies with the default value are hidden:
  

• When "show exceptions" is unckecked, default value appears for each tree which does not have an expection:
  

Additional context

Tested on 1.2.11 and 1.2.16.

[TheWitness]

From my perspective it's working as designed. If the default is to deny, then an exception would be where the user has access. If the default is to allow, then an exception would be where the user does not have access.

The behavior is right. If you have a problem with the wording, I'm all in for ideas.

[TheWitness]

It's not an 'Access Granted' or 'Access Denied' thing. But maybe it should be. You should write a very precise description of:

1. What the checkbox should be called
2. How it should work if the default is to deny
3. How it should work if the default is to allow

Keep it simple.

[netniV]

I think the confusion comes in that the selection toggles what is displayed rather than including more. This may be one of those situations where either a two value drop down or a pair of on/off buttons are used to toggle between the two? One for further thought.

[luismrsilva]

Thank you for your replies. I'm sorry if the explanation was not clear.

I think the confusion comes in that the selection toggles what is displayed rather than including more.

It's something along those lines, yes.

To be clear:

• A policy is either a value of either Allow or Deny.
• For each tree, we have a policy value.
• A default value can be defined.
• An exception is a tree with a policy value that is different from the default value.

When the list of trees is first shown, "Show Exceptions" is checked.
However, in this case, only exceptions are shown. The tree items for which the default value is used do not appear explicitly in the list. However, the since the checkbox says "Show Exceptions", the expectation is that it will show more things, not less.

This becomes more confusing when the checkbox is unchecked and all tree items appear.
Having the "Show Exceptions" checkbox unchecked would mean that, given its name, no exceptions would be shown.
However, when it is checked, only exceptions are shown; and when it is unchecked, both the exceptions and the trees with default value appear in the list.

Maybe this description was more complicated, I'm sorry if so.
Solving this could be done either by changing the behavior or changing the name. I think it's easier to just change the name, so:

1. What the checkbox should be called

Given the current behavior, it should be called "Only Show Exceptions".

[netniV]

I will agree with that conclusion.

[TheWitness]

I'm okay with adding 'Only'.

[luismrsilva]

Alright, I can do a PR soon.

Also, could you please confirm that the same applies in:
Configuration -> User Groups -> (some group) -> Tree Permissions;

I could also do a PR for that; not sure it's worth opening another issue.

[TheWitness]

Users and groups have common permission interface.

[luismrsilva]

Now that I'm trying to fix it, I've realized that it also applies in "Device Perms" and "Template Perms".

Also, it requires the string to be updated in all supported languages.

[TheWitness]

Your pull request needs to include the updated cacti.pot for translators. The build_gettext.sh script will update it.

[luismrsilva]

@TheWitness the build_gettext.sh script updated a lot of things unrelated to this string. I'm guessing the cacti.pot is not up-to-date with changes to the code. Is this the case? If so, would it make sense to do a PR to update cacti.pot first, and then fix the checkbox thing?

[TheWitness]

Regardless, don't commit any of those changes to your local repo.

[luismrsilva]

Should I commit only the lines changed in cacti.pot that are related to the checkbox thing then (in addition to PHP files)?

[TheWitness]

All the php changes and then the whole cacti.pot after you run the command. Don't forget the CHANGLELOG too.

[luismrsilva]

Ok, I've done that. Is this alright (notice a lot of things were changed there)? Also, I haven't touched the CHANGLELOG yet.
develop...luismrsilva:issue-4056-2

[luismrsilva]

Sorry about the confusion and thank you for fixing it, @TheWitness.