search

incodehq / incode-platform

Combines incode.org modules and isisaddons.org into a single set of modules.
http://platform.incode.org
Apache License 2.0
8 stars 9 forks source link

spi-security: Simplify SecurityModuleAppUserRegistrationServiceAbstract #44

Open danhaywood opened 6 years ago

danhaywood commented 6 years ago

from https://github.com/isisaddons-legacy/isis-module-security/issues/46

bilgin:

What is the reason for SecurityModuleAppUserRegistrationServiceAbstract to have 2 abstract methods getInitialRole and getAdditionalInitialRoles?

Why not have only one method that returns Set? It will be easier to override. Otherwise, it makes you think that initial role is different that any any additional role.?

danhaywood commented 6 years ago

dan:

Yep, I agree. Was probably just how it evolved over time.

Obviously it'll be a breaking API change, but I'm less fussy about that so long as the changelog in the README explains how to fix (perhaps worth an additional section in the README to address this).

If you want to create a PR for this, I'll be happy to merge it in.

Thx