Implement commands system #40
Conversation
Closes alveusgg#32 Closes alveusgg#33 Closes alveusgg#34 Signed-off-by: flakey5 <[email protected]>
| } | ||
|
|
||
| if (permission.group && user in groupMemberships) { | ||
| const userGroup = groupMemberships[user] |
There was a problem hiding this comment.
I would think we'd want a user to be able to be in multiple groups (so we could have a group for full PTZ commands, a group for just the ptzload etc. commands, and so on)?
I think that might be an easier approach that having an intrinsic ranking to groups?
There was a problem hiding this comment.
I would think we'd want to stick to the group rankings that exist or possibly add more in-between tiers rather than have a user exist in multiple groups. The group rankings makes it a lot easier to check what commands a user has access to as it will be the same as all Operators or whatever their rank is.
I cant currently think of any scenario where the current ranking tiers falls short.
There was a problem hiding this comment.
👍 Also happy to stick with the ranking system if that's what folks prefer -- I just found that to be somewhat less intuitive than having each user have an explicit list of what groups and therefor access they have.
| /** | ||
| * @type {Array<import('./types.d.ts').Command>} | ||
| */ | ||
| const commands = aliases.map(alias => ({ |
There was a problem hiding this comment.
I think the command structure should have an aliases prop (during the command loading you can generate an obj/map to do quick lookups) instead of us needing to create multiple identical objects for each alias (and having a single obj per command means we could more easily use the data to power the site commands list etc.)
| @@ -496,6 +496,8 @@ class Axis { | |||
| * | |||
| * `controller.connections.cameras` is an object of Axis camera connections | |||
| * | |||
| * @typedef {Record<typeof config.axisCameras[number], Axis>} CamerasConnection | |||
There was a problem hiding this comment.
A typedef inside the jsdoc for a func seems wrong?
|
@dansza1 @wazix11 @spacevoyager03 @pjeweb would appreciate y'all taking a look at this draft as well and hearing any thoughts you have on it in terms of direction |
I think generally this is a good approach. I'm not the biggest fan of classes in JS but that is besides the point. I have not thought about the permissions in depth yet, even though that seems to be one of the bigger points here. Maybe someone else can think that through ;) There are a bunch of commands with shared code, sometimes only changing one var, like customcamstl, …tr etc. I think it would make sense to allow them as aliases of one command and pass the command name as a CommandArg. Also I would probably move the alias mapping in to the command manager and define them in the exported config as an array or something. |
Co-authored-by: Matt Cowley <[email protected]>
As titled and described in the issues linked.
Opening this as a draft just to get feedback on the approach here before moving forward.
What's currently here:
ptzplayaudioandptzstopaudiocommands for demonstration purposesFor the permission system, it's pretty similar to what's there now with two main differences:
indexOf)This does allow for more granular access to commands, but, it makes giving someone access to say all the
ptzcommands w/o adding them to a group kinda annoying since you'd need to add them to each of the command's permissions individually. A better approach to this might be having permission "nodes" similar to how Minecraft does it (and also might be what #34 is talking about here:?). Then, going back to the example of giving someone access to all the
ptzcommands, we can just make all theptzcommands take the permission nodeptz-bla-bla-bla-something. Then inconfig.jswe define which groups and individual users have a permission node. If a user has that permission node or the group they're in has that node, then they can run the command.Closes #32
Closes #33
Closes #34