Add verbose_name for rules and permissions

[jacklinke]

Adds verbose_name for rules and permissions, resolving #59.

This change should be transparent to any existing users of django-rules, but adds the new capability to add and display verbose names for rules & permissions through the use of new/updated RuleSet instance methods and shortcuts for shared and permissions rule sets.

• add_rule(name, predicate, verbose_name=None): Updated to allow optional verbose_name.
• set_rule(name, predicate, verbose_name=None): Updated to allow optional verbose_name
• rule_verbose_name(name): A new method which returns the verbose_name if it was supplied when adding or setting the rule. Defaults to name if no verbose_name was supplied.
• (similar method names for permissions)

Models can now also optionally add verbose names to permissions using the following conventions:

import rules
from rules.contrib.models import RulesModel

class Book(RulesModel):
    class Meta:
        rules_permissions = {
            "add": rules.is_staff,
            "read": {"pred": rules.is_authenticated, "verbose_name": "Can read this book"},
            "delete": {"pred": rules.is_staff},
        }

This would be equivalent to the following calls::

rules.add_perm("app_label.add_book", rules.is_staff)
rules.add_perm("app_label.read_book", rules.is_authenticated, verbose_name= "Can read this book")
rules.add_perm("app_label.delete_book", rules.is_staff)

Tasks:

• Update code
• Add and run tests
• Update Readme

I welcome any feedback.

[dfunckt]

Great stuff! 👍

My only concern is we're changing the structure of the items stored in rule sets -- RuleSets used to be a map of strings to predicates, whereas now it is going to be a map of strings to dicts. This is technically a breaking change and can catch users by surprise.

Maybe override __getitem__ et al to return the predicate like it did before? The important use cases are using the rule set as a dict (ie. get, set, delete items) and iteration which should continue to behave as before. Tests that ensure that would be much appreciated.

Otherwise this looks like a great addition, thank you for that!

[jacklinke]

Glad this is seen as something beneficial, and thanks for the advice/direction.

I updated the code, returning the predicate if the dict is accessed directly (e.g.: rule_set_instance["permission_name"]), but added the option to prefix/suffix the permission name with 'get_' and '_display' respectively to return the verbose_name (e.g.: rule_set_instance["get_permission_name_display"])

I figured following a similar approach as Django's get_FOO_display() method for getting the display names of choice fields made sense here.

If that sounds sensible, I'll add tests and docs this week to finish this out.

[dfunckt]

@jacklinke apologies for the delayed review. Great stuff 👍

I left a few comments. There also seem to be a few lint/formatting issues highlighted by the checks -- can you please take a look at those too?

[dfunckt]

Can you please rename the pred attribute to predicate?

[dfunckt]

Suggested change

	return self["get_%s_display" % name] or name
	return self["get_%s_display" % name]

This must return None if no verbose name has been provided for the rule and callers can choose whether to fallback to name.

[dfunckt]

All this can just be:

Suggested change

	if isfunction(pred):
	# If a function as passed in (as might be done with legacy django-rules)
	# convert the value to the dictionary form
	pred = {'pred': predicate(pred), 'verbose_name': verbose_name}
	if isinstance(pred, dict):
	# Check if internal pred is already a Predicate, or an basic
	# unwrapped function. Wrap as a Predicate if needed
	if not isinstance(pred['pred'], Predicate):
	pred['pred'] = predicate(pred['pred'])
	if isinstance(pred, dict):
	pred['pred'] = predicate(pred['pred'])
	else:
	pred = {'pred': predicate(pred), 'verbose_name': None}

Or even:

Suggested change

	if isfunction(pred):
	# If a function as passed in (as might be done with legacy django-rules)
	# convert the value to the dictionary form
	pred = {'pred': predicate(pred), 'verbose_name': verbose_name}
	if isinstance(pred, dict):
	# Check if internal pred is already a Predicate, or an basic
	# unwrapped function. Wrap as a Predicate if needed
	if not isinstance(pred['pred'], Predicate):
	pred['pred'] = predicate(pred['pred'])
	if not isinstance(pred, dict):
	pred = {'pred': predicate(pred), 'verbose_name': None}

if we assume anyone passing a dict know what they're doing.

[dfunckt]

All this can just be:

Suggested change

	def _check_name(_name):
	if (not super(RuleSet, self).__contains__(_name)):
	raise KeyError("Provided name '`%s`' not found" % _name)
	if name[0] != '_':
	prefix = "get_"
	suffix = "_display"
	if name.startswith(prefix) and name.endswith(suffix):
	_name = name[len(prefix):-len(suffix)]
	_check_name(_name)
	return super(RuleSet, self).__getitem__(_name)['verbose_name']
	_check_name(name)
	prefix = "get_"
	suffix = "_display"
	if name.startswith(prefix) and name.endswith(suffix):
	_name = name[len(prefix):-len(suffix)]
	return super(RuleSet, self).__getitem__(_name)['verbose_name']

Right?