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

Grant installers edit permissions on buildings & nodes #401

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

Andrew-Dickinson
Copy link
Member

It seems odd to single out these two objects. I don't recall why we did that? Might just be a historical thing where we added these models and forgot to update the permissions?

This PR makes installer permissions consistent across all objects

@WillNilges
Copy link
Collaborator

We did this because the Installer role was supposed to be for anyone with a baseline level of trust who would be setting up equipment on a rooftop. Right now, everyone who does that on a regular basis is pretty trustworthy, but I think the idea was that we didn't want that large of a group to mess with MeshDB information. We definitely didn't forget to update the permissions.

I would prefer if we kept this role limited in scope and power.

Copy link
Collaborator

@WillNilges WillNilges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we didn't do this (See my comment)

@Andrew-Dickinson
Copy link
Member Author

Okay but like, they can trash the install, member, and device tables right? And they can't fix errors in a lat/lon position of a building or node. I think this is just going to result in unnecessary frustration. Like either we trust them to edit stuff or we don't, you know. I don't see singling out these two tables as something that offers all that much protection to us I guess

@WillNilges
Copy link
Collaborator

Yeah, fair enough. I guess the installer role was some sort of premature optimization. We can always make more roles if we really feel strongly about it. I think restricting those tables might have been an attempt to prevent someone from potentially breaking info tied to many members (It's less bad if someone accidentally breaks a single Install vs a whole Node, you know?)

@Andrew-Dickinson Andrew-Dickinson enabled auto-merge (squash) July 19, 2024 23:37
@Andrew-Dickinson Andrew-Dickinson merged commit 4a1d71a into main Jul 19, 2024
7 checks passed
@Andrew-Dickinson Andrew-Dickinson deleted the andrew/installer-permissions branch July 27, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants