-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add functionality to the local dev tools. #1938
Add functionality to the local dev tools. #1938
Conversation
…be created as needed, and for specific users and modules to be selected for the various test scenarios.
…d from any user.
…, using the current defaults if not provided explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, especially the option to add permissions so we don't have to manually update the database to be able to view stats.
Only minor thing I noticed is there are two "add permission" options on the page, though the code for the second one actually removes instead of adds. I also had the idea that being able to remove users might be good, though looking at the endpoints there's not actually anything that does that so it'd probably be more work than it's worth and likely wouldn't get much use anyway.
…sion' - copy and paste fail.
Looks like that was a copy/paste fail. Fixed it and pushed. It might be interesting to see what breaks if users are deleted, but that's currently not expected to ever happen in practice. Like you said, there doesn't appear to be anything anywhere in the code to handle deleting user records - if we wanted to do that, we'd have to remove them from the database by hand regardless of the environment. But since it doesn't seem like it's even possible, I think I'm okay with not having a button for it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, LGTM.
I didn't think about all the other repercussions of deleting a user and the cleanup that it would require, so that's definitely not something that's necessary to add.
Closes #1932.
Apparently the dev tools never had unit tests, so they continue to not have unit tests.