Role Based Menu Navigation - Bug

Ignition
1

Hello everybody,
I'm using this resource: Role Based Menu Navigation

I found a bug in the resource and I don't know how to fix it, so I hope you can help me.

When you add a view you can set the "Required Roles" property.

image
image759×965 54 KB

So now this is the procedure to make the bug happens:

  1. Add roles to the view
  2. Submit
  3. Remove all the roles to the view
  4. Submit
  5. At this moment the page return this error:
Traceback (most recent call last): File "<transform>", line 53, in transform File "<transform>", line 34, in buildInstance File "<transform>", line 34, in buildInstance File "<transform>", line 7, in buildInstance TypeError: object of type 'bool' has no len()

And the only way to fix this error is to find in the "menu tree" that specific page where the requiredRoles has become a bool and convert to an array

image
image1744×968 57.7 KB

Thank you for the help

2 Likes
2

You should report this to resource's author.

3

The resource is quite old, i doubt it's gonna get updated.
But still worth a try,
Could also show us the code that addes the role, maybe its an easy fix

4

I've already tried to concact the author for another question but without any answer.

Yes, that was my idea, maybe someone can fix it. I will try to understand the code and I will post it.

Thank you all for the help.

5

With more things going to text on 8.3, maybe it would be time to consider some sort of source control functionality for the Ignition Exchange. Then others can submit pull requests to fix bugs or add features to existing Exchange resources.

1 Like
6

The author of this ressource doesn't work anymore at IA so i don't know if she keep it updated. I use that ressource in our standard project. I will check if we made any update to it but will not be before this weekend.

7

Don't worry, in fact, thank you for your availability. I find this resource very useful, although I would like to be able to make a few more modifications, such as being able to choose from different icons (currently, it seems there are only the default ones). I would have also added the icons from Bootstrap as I already have in the designer, but I'm not sure how to add them in the resource. Another idea that comes to mind could be to make it possible to grant access (and visibility) only to specific users rather than by role.

This resource is so useful that it would be a shame not to continue with the updates.

Thanks again

9

I tested the issue with the help of GPT, and here’s what I found:

The error appears in this part of the original code:

The code where the error appears is this:


	def buildInstance(item, options, instances, depth, length, index):
		isChild = True
		selectedValues = []
		subInstances = []
		for roles in range(len(item['requiredRoles'])):
			for opt in range(len(options)):
				if item['requiredRoles'][roles] == options[opt]['label']:
					selectedValues.append(options[opt]['value'])
		if len(item['items']) > 0:
			isChild = False	
			
		#dict matches the view params of Menu Items	
		dict =	{
					"viewName": item['label']['text'], 
					"target": item['target'],
					"dropdownItems": options, 
					"selectedValues": selectedValues,
					"isChild": isChild, 
					"length":length,
					"index":index,
					"depth": depth,
					"tagPath":item['path'],
					"iconPath":item['label']['icon']['path'],
					"requiredRoles": item['requiredRoles']
				}
		instances.append(dict)
					
		if len(item['items']) > 0:
			length = len(item['items'])	
			for j in range(len(item['items'])):
				index = j
				buildInstance(item['items'][j], options, instances, depth+1, length, index)
				

		
	instances = [] 
	items = value
	rolesDict = {}
	roles = system.user.getRoles("default") #get all possible roles from user source
	options= [] #rolesDict will be appended to this to fill in the dropdown options
	depth = 0 
	
	#create dropdown items object
	for r in range(len(roles)):
		rolesDict = {"value":r, "label":roles[r]}
		options.append(rolesDict.copy())
		
	length = len(items)
	for i in range(len(items)):
		index = i
		buildInstance(items[i], options, instances, depth, length, index)
	return instances

The error appears in this part of the original code:

for roles in range(len(item['requiredRoles'])):

The issue is that when you delete all the rolesitem['requiredRoles'] is not an array but a boolean (False ). To handle this, I updated the code as follows:

requiredRoles = item.get('requiredRoles', [])
if isinstance(requiredRoles, bool):
    requiredRoles = []

Here’s the final code I tried, and it seems to work:

		
def buildInstance(item, options, instances, depth, length, index):
    isChild = True
    selectedValues = []
    subInstances = []
    
    # Get the required roles
    requiredRoles = item.get('requiredRoles', [])
    if isinstance(requiredRoles, bool):
        requiredRoles = []  # If it's a boolean, set it to an empty list

    # Check if any of the options match the required roles
    for roles in range(len(requiredRoles)):
        for opt in range(len(options)):
            if requiredRoles[roles] == options[opt]['label']:
                selectedValues.append(options[opt]['value'])
    
    # If the item has child items, it's not a leaf (isChild = False)
    if len(item['items']) > 0:
        isChild = False  

    # Create a dictionary matching the view parameters for Menu Items
    dict = {
        "viewName": item['label']['text'],
        "target": item['target'],
        "dropdownItems": options,
        "selectedValues": selectedValues,
        "isChild": isChild,
        "length": length,
        "index": index,
        "depth": depth,
        "tagPath": item['path'],
        "iconPath": item['label']['icon']['path'],
        "requiredRoles": requiredRoles
    }
    instances.append(dict)

    # If the item has sub-items, recursively process them
    if len(item['items']) > 0:
        length = len(item['items'])  
        for j in range(len(item['items'])):
            index = j
            buildInstance(item['items'][j], options, instances, depth + 1, length, index)

# Main function logic
instances = [] 
items = value  # Assuming 'value' is already defined somewhere in your code
rolesDict = {}
roles = system.user.getRoles("default")  # Get all possible roles from user source
options = []  # rolesDict will be appended to this to fill in the dropdown options
depth = 0

# Create dropdown items object for each role
for r in range(len(roles)):
    rolesDict = {"value": r, "label": roles[r]}
    options.append(rolesDict.copy())

length = len(items)
for i in range(len(items)):
    index = i
    buildInstance(items[i], options, instances, depth, length, index)

return instances

Please give me your feedback about this.

Thank you

10

I would not expect a recursive function to look this way. There should be a base case which ends the recursion, and any number of other cases that result in a recursive call. There are a number of other things that are generally not pythonic.

  1. Anytime you see a for loop with this form: for x in range(len(y)) it can always be written better as for x in y.
  2. In the case where you need both the index and the object you can use the built-in enumerate() function which returns a list of tuples with the index and the object.
  3. In python if you have a for loop that is just appending an item to a list:
for x in y:
    if someCondition:
        someList.append(x)

Then you can write that loop as a comprehension:

someList = [x for x in y if someCondition]

which may look confusing until you learn to read them, but they have the advantage of being more performant.

  1. You should never use built-in names as variable names. Because everything in python is a first class object this: dict = {key:value} is actually redefining the dict() function to this dictionary. So if you tried something like: dict = {key:value} dict(someVariable) the code would fail.
  2. IMO passing an object into a function for the sole purpose of modifying that object is a code smell. You may argue that IA does this with many of their functions, but in those cases they are passing in immutable objects and the function is returning a brand new object, not the same object, but "different". Don't get me wrong, this technique has it's place, but I try to avoid it, because I find it hard to follow. This is one of the reasons I avoid transforms in perspective.

All of that said, I would expect your code to look something like this:

    def buildInstance(item, options, depth, length, index):
        isChild = not item['items']
        selectedValues = [opt['value'] for opt in options for role in item[requiredRoles] if role == opt['label']]
 
        # Create a dictionary matching the view parameters for Menu Items
        if isChild:
	        return {
	                "viewName": item['label']['text'],
	                "target": item['target'],
	                "dropdownItems": options,
	                "selectedValues": selectedValues,
	                "isChild": isChild,
	                "length": length,
	                "index": index,
	                "depth": depth,
	                "tagPath": item['path'],
	                "iconPath": item['label']['icon']['path'],
	                "requiredRoles": item['requiredRoles']
	            }

        # If the item has sub-items, recursively process them
        for index,item in enumerate(item['items']):
            return buildInstance(item, options, depth + 1, len(item['items']), index)

# Main function logic
roles = system.user.getRoles("default")  # Get all possible roles from user source

# Create dropdown items object for each role
options = [{'value':r, 'label':role} for r,role in enumerate(roles)]
return buildInstance(value, options, 0 , len(value), 0)

You will notice that I do not do any checks for the type of item['requiredRoles'], this is because I am using it as a list, and if it happens to be a bool, the loop for role in item['requiredRoles']: will fall through and no items will be appended to selectedValues.

You will also note that I do not use the len() function to determine if the item is a child or not. If a list has a length of 0, then when examined as a conditional statement it will return false. Otherwise, it will return true. This means that isChild = not item['items'] will return true if and only if the length of item['items'] is 0.

In this function, the item being a child item is the base case, so that is where we return the instance, otherwise we call the function recursively.

1 Like
11

Thanks for the reply @lrose, this is really helpful.

I'm going to read calmly and try to implement it!