Add deny_permissions, deny_platforms and display_name setting support

cpenv / tk-cpenv

Shotgun Toolkit Application that adds support for cpenv modules.

Other

20 stars 6 forks source link

Add deny_permissions, deny_platforms and display_name setting support #12

Closed RicardoMusch closed 3 years ago

RicardoMusch commented 3 years ago

Adds support for setting:

display_name As great as 'Set Modules' sounds... someone might want to change it up...
deny_permissions Not everyone likes to give users the ability to change environement setups ;)
deny_platforms Handy...

danbradham commented 3 years ago

Just noticed one last thing. The before_app_launch method will still execute on all platforms. It would be great if there was a permissions check at the beginning of before_app_launch to return early if the user is on a denied platform.

RicardoMusch commented 3 years ago

Just noticed one last thing. The before_app_launch method will still execute on all platforms. It would be great if there was a permissions check at the beginning of before_app_launch to return early if the user is on a denied platform.

However we do want that right? We just want to disallow the Set Module app from starting.

Or should deny_platforms encompass the overall setting that CPENV as a whole is denied on that platform?

danbradham commented 3 years ago

You know what, I think it would make sense to rename these to deny_ui_permissions and deny_ui_platforms. I think restricting cpenv to certain platforms would make sense potentially, but that could be handled in other ways. Like adding a platforms field to the Environment entity and exposing that in the Set Modules dialog. So let's keep this PR as you originally intended.

RicardoMusch commented 3 years ago

You know what, I think it would make sense to rename these to deny_ui_permissions and deny_ui_platforms. I think restricting cpenv to certain platforms would make sense potentially, but that could be handled in other ways. Like adding a platforms field to the Environment entity and exposing that in the Set Modules dialog. So let's keep this PR as you originally intended.

Done!

danbradham commented 3 years ago

Excellent. Thanks for contributing!

RicardoMusch commented 3 years ago

Excellent. Thanks for contributing!

Thanks for being my first ever pull request :P