Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change permission selection to checkboxes and make the space bigger #14297

Open
gdprdatasubect opened this issue Nov 18, 2023 · 20 comments
Open
Assignees
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement netbox status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@gdprdatasubect
Copy link
Contributor

gdprdatasubect commented Nov 18, 2023

NetBox version

v3.6.5

Feature type

Change to existing functionality

Proposed functionality

Currently the selection for permissions looks like this:
grafik

This does not seem like a good design choice (A lot of options crammed in a small field, ctrl required to add things to selection) and could IMHO better be handled with individual checkboxes and not using a selection-field in the first place, e.g.:

grafik

etc.

Use case

Improve admin-user-experience, not having to press control to add things to the selection, improving accessibility due to being able to use tab to switch to next field

Database changes

No response

External dependencies

No response

@gdprdatasubect gdprdatasubect added the type: feature Introduction of new functionality to the application label Nov 18, 2023
@jonasnieberle
Copy link

I think it would be very useful if you implement this feature. I struggled the last 15 minutes to select 10 object types. You have to press the Strg key and then scroll down ... then you select the next one and all previous selected object types are deselected.

@DanSheps
Copy link
Member

Sorry, this is non-viable straight out of the gate. We have over 100 models within NetBox that can have permissions assigned to them. We cannot generate 100+ check boxes to allow for this.

Additionally, this also doesn't take into account permissions added by plugins.

@jsenecal
Copy link
Contributor

I think it would be very useful if you implement this feature. I struggled the last 15 minutes to select 10 object types. You have to press the Strg key and then scroll down ... then you select the next one and all previous selected object types are deselected.

Maybe the help text could be more explicit and the widget made bigger, but the current way really is the only logical way.

@jonasnieberle
Copy link

jonasnieberle commented Dec 20, 2023

Ok, yeah you're right 100+ check boxes aren't good to handle. An what about the same method like the filter methods:

image

You can search for the object and can select it and then you see the selected object in the top line. And with the x you can remove the object from the selection. The same ui style is implemented by the user group assignment and the permissions.

image

So we also have the benefit that the same ui styles used 🤔

@jsenecal
Copy link
Contributor

You can search for the object and can select it and then you see the selected object in the top line. And with the x you can remove the object from the selection. The same ui style is implemented by the user group assignment and the permissions.

What do you do when you get 90+ permissions selected ? Wouldnt that break UX ?

@abhi1693
Copy link
Member

How about something similar to this?

image

@DanSheps
Copy link
Member

DanSheps commented Dec 20, 2023

I think #13435 brings up using a dynamic selector (not clear about it from the description IMO though).

@gdprdatasubect
Copy link
Contributor Author

Ok, yeah you're right 100+ check boxes aren't good to handle

Well, to me the current workflow seems worse to handle than checkboxes, so that's why i suggested them.

Personally I think the whole permission workflow is counterintuitive and the ideal solution would be a Permission Constraint text field per available option, since filtering could be more fine grained and would involve creating less groups (or maybe i'm just using it wrongly)

@DanSheps
Copy link
Member

Well, to me the current workflow seems worse to handle than checkboxes, so that's why i suggested them.

Try checking 100 checkboxes because you need to give every permission except for a few (compared to Ctrl-A then Ctrl-click on ones you don't need)

Personally I think the whole permission workflow is counterintuitive and the ideal solution would be a Permission Constraint text field per available option, since filtering could be more fine grained and would involve creating less groups (or maybe i'm just using it wrongly)

There is a permission constraint field, but not everyone can be bothered to learn JSON or they have simpler needs.

Additionally, not every model has every field so you typically have to create multiple permissions for permission constraints anyways.

@jeffgdotorg
Copy link
Contributor

Checkboxes feel untenable, but we could change this control to use a dynamic multi-select widget. That would make it work like the Content Types control from the existing Add Custom Link flow, giving type-to-filter and removing the need to hold down Ctrl / Strg.

image

@jeffgdotorg jeffgdotorg added status: accepted This issue has been accepted for implementation status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Dec 20, 2023
@jeffgdotorg
Copy link
Contributor

Folding #13435 into this issue. The two are similar enough that we think a common approach (dynamic multi-select, see above) will do the trick for both.

@DanSheps
Copy link
Member

Folding #13435 into this issue. The two are similar enough that we think a common approach (dynamic multi-select, see above) will do the trick for both.

It might be a good idea to maybe make it a DynamicModelMultipleChoiceField and see if we can get a selector widget working with it (also make the selector widget work with multi-selection by allowing Ctrl+click to select multiple items).

There would be a little work involved if we wanted to go that route. We would likely need to make ContentType a proxy model of it's own.

@jeremystretch jeremystretch removed the status: accepted This issue has been accepted for implementation label Dec 27, 2023
@DanSheps
Copy link
Member

DanSheps commented Jan 9, 2024

Did we ever decide on which field we should use for this? I am kind of coming around to @abhi1693's idea as any other could get rather messy with the amount of objects we have.

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label May 15, 2024
@jonasnieberle
Copy link

Did we ever decide on which field we should use for this? I am kind of coming around to @abhi1693's idea as any other could get rather messy with the amount of objects we have.

How can we procceed with this topic? From my site the idea from @abhi1693 ist the best? The other fields can/will break the UI if you select 100+ permissions. Or maybe some other ideas from other software?

@jeremystretch
Copy link
Member

@JonasEinfach this issue needs a volunteer to implement it. Would you like to own it?

@jonasnieberle
Copy link

jonasnieberle commented May 15, 2024

@jeremystretch Im not familar with developing in netbox, but I can try :)

At first look, it looks like that's only a change in the forms of the permission object.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation pending closure Requires immediate attention to avoid being closed for inactivity labels May 15, 2024
@jeremystretch jeremystretch added the complexity: medium Requires a substantial but not unusual amount of effort to implement label May 22, 2024
@jonasnieberle
Copy link

jonasnieberle commented May 26, 2024

Today I checked the forms.py for the ObjectPermission.

So that's the actual python code:

    object_types = ContentTypeMultipleChoiceField(
        label=_('Object types'),
        queryset=ObjectType.objects.all(),
        limit_choices_to=OBJECTPERMISSION_OBJECT_TYPES,
        widget=forms.SelectMultiple(attrs={'size': 6})
    )

If Im right you limit the choice of form rendering with the constants OBJECTPERMISSION_OBJECT_TYPES?

In the form we use the ContentTypeMultipleChoiceField. I searched in the netbox github for all forms which use this form as well. For example I found that the CustomFieldForm also use this form.

object_types = ContentTypeMultipleChoiceField(

The funny thing - in the custom field form we have definitely a better UI form to select / search / remove / see the content types.

image

But the ObjectPermission Form have the "not well to handle" Form :D

image

The widget attribute forms.SelectMultiple(attrs={'size': 6}) overwrite the default form ...
I commented the widget attribute and we have the better multiple select form.

image

The form can also handle >80 selected objects.
Tbh who use permissions with more than 80 content type objects? It's definitley smarter to use multiple permissions and assign them to a group ...

image

From my site the UI experience is definitely better without widget attribute forms.SelectMultiple(attrs={'size': 6})

What do you think? @jeremystretch @DanSheps

@netbox-community netbox-community deleted a comment from PaulTorm Jul 9, 2024
@netbox-community netbox-community deleted a comment from jonasnieberle Jul 9, 2024
@PieterL75
Copy link
Contributor

Cant his FR also include the list view of the user permissions ?
Its a pretty long page when all but some objects are selected
image

Maybe do the same as ip/vlans on interfaces, just display a nbr of there are more than 5

@jeremystretch jeremystretch added the netbox label Nov 1, 2024 — with Linear
@itslychee
Copy link

Hey was wondering if there was any update on this!

@DanSheps

Sorry, this is non-viable straight out of the gate. We have over 100 models within NetBox that can have permissions assigned to them. We cannot generate 100+ check boxes to allow for this.

Could you elaborate on this? How does adding another element to each item added to the list of entries and iterating over that to see if an entry is checked not viable? Genuinely just curious

I'd say the solution proposed by @jonasnieberle is probably the cleanest and best solution for this, but hoping there could be some feedback from a member.

thank you for netbox!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement netbox status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

9 participants