Improve the OrchardCore.Tenants.FeatureProfiles feature

[MikeAlhayek]

Is your feature request related to a problem? Please describe.

If a profile is renamed, It'll break existing tenants since the name is stored in the appsettings.json file.

Also, other modules are not able to contribute into the FeatureProfiles setup. For example, I want write a billing module that would calculate the monthly cost for the tenant. I could add price field for each profile then sum the total price for billing.

Describe the solution you'd like

1. Add an id and store the id in appsettings.json this way renaming a profile does not break anything
2. Allow the user to have multiple profiles more on than explained in #10962
3. Ultimately, user DisplayDriver to render the CRUD to allow other modules to contribute into the captured data.

If the above is acceptable, I can submit a PR to handle this request. I already implemented 1 & 2. I should be able to implement 3 as well.

[Skrypt]

Is there no permission/role required to enable/disable features in a tenant? I suggest you look at that.

[ns8482e]

@CrestApps Adding id won't solve the real problem. - IMHO rename should not be allowed IF the profile is assigned to any tenant.

[deanmarcussen]

Changing it to an id sounds breaking (and of little benefit).

suggest just making the name non editable (if it wasn’t done that way originally that’s my mistake)

multi select sounds ok as long as it is non breaking. I assume you’ll have to make them comma separated, as we can only support storing strings in the shell sources. It’s a little weird to have a comma separated string stored in json instead of an array, but could live with that.

drivers. Yes, possible, but runs good into the issue of only storing strings, which could make them a bit odd

[MikeAlhayek]

@deanmarcussen it should not break anything if we treat existing name as id when id is not stored. This show not break anything and stay backward compatible.

One question I do have, why is FeatureProfilesValidationProvider part of OrcharcCore project instead of OrchardCore.Tenants? I would think it should be also registered in the OrchardCore.Tenants.FeatureProfiles startup. Should I move it?

[MikeAlhayek]

Adding id won't solve the real problem. - IMHO rename should not be allowed IF the profile is assigned to any tenant.

@ns8482e Not sure why you don't think it'll solve the problem. IMHO, naming could change and the app should be fixable to allow it. Please review the implementation to see if it'll solve the problem or not. Here is the branch that contains the complete implementation https://github.com/CrestApps/OrchardCore/commits/CreateFeatureProfiles

[ns8482e]

@CrestApps two reasons

• You are changing the reference value from name to id will lead to breaking changes as @deanmarcussen suggested.
• You are offloading the issue to ID property instead of Name and not actually resolve concerned issue.

The real issue is once the reference is in use - you can't modify - think Name is like technical name. ( same issue exists with Part - if you change technical name it will break all your existing content )

[ns8482e]

Another solution is have it read only once created. If you need to change the name - retire previous one and recreate new.

[MikeAlhayek]

@ns8482e I think the problem with that is you'll have to find any tenant that is using an old tenant and change the settings for it. The implementation I put together should not impact old stuff even if you rename old stuff. The current name is being treated as the id even if you change the name to something else which does not require the update of the tenant profiles.

For example, if you have a an old profile called "Test123" and you want to rename it to "Final Test", the Id will be "Test123" and the name will be "Final Test". So regardless what you change the name to, the original name will be the id. If you create a new profile, a new Id will be generated. So it should not really break anything.

[sebastienros]

Fien to be able to change the name as long as it's not breaking. For custom properties and drivers, I don't see the need for the added complexity.

[MikeAlhayek]

PR #11841 would allow the user to use multiple feature profiles. Additionally, we create id that is used for each profile instead of the name since we know changing the name will break the profiles. This was done as per suggestion from @sebastienros with no drivers. Also, this PR does not break existing setup since we treat current name as the id.