michaelryanmcneill / shibboleth

Shibboleth plugin for WordPress
https://wordpress.org/plugins/shibboleth/
19 stars 11 forks source link

add option to disable account creation if no mapped roles or default role #59

Closed dandalpiaz closed 4 years ago

dandalpiaz commented 4 years ago

Added a new commit to change the "noaccount" name to "_no_account"; I think that's better. I also made an update to the selected() function to follow the formatting shown at the bottom of this reference page, https://developer.wordpress.org/reference/functions/selected/.

I agree, the "Do NOT" label is pretty harsh sounding - definitely open to other label names. "Prevent account creation"? Or maybe add "(none)" to the label as well to give it juxtaposition to the existing "(none)", e.g. "(none) - prevent account creation". Any other ideas?

I'm pretty new to plugin development, so I appreciate all the feedback!

jrchamp commented 4 years ago

Comparing this to the existing "(none)", I think the parentheses imbues special meaning. As far as meaning, choosing this option will mean that:

Conversely as positives:

Perhaps "(skip 'no role' account creation)" or "(none) and skip '(none)' account creation"? Part of me wants to rename the "(none)" item to "(no role)"...

Hopefully these ideas help somewhat. Ultimately, I think you're on the right track and whatever you decide is probably best. :+1: Thank you for your contributions on this @dandalpiaz !

neverything commented 4 years ago

Tested this pull request on a system I work with and it works as promised, thanks @dandalpiaz. I'll use this version starting next week up until a new release with this included is ready.