Fixes #4608 - Added description field to role

[dhlavac]

No description provided.

[theforeman-bot]

There were the following issues with the commit message:

• 22ed5bf must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project.

More guidelines are available in Coding Standards or on the Foreman wiki.

---

This message was auto-generated by Foreman's prprocessor

[theforeman-bot]

There were the following issues with the commit message:

• 22ed5bf must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project.

More guidelines are available in Coding Standards or on the Foreman wiki.

---

This message was auto-generated by Foreman's prprocessor

[tbrisker]

[test] - this activates the CI testing until you are added to the allowed list.

[dLobatog]

This one was closed already in bugzilla as WONTFIX (https://bugzilla.redhat.com/show_bug.cgi?id=1104176) - what's the use case? It complicates our form for little benefit IMO.

[ares]

The reason was "As per upstream bug, adding description here to roles only would be inappropriate.", in upstream issue the only reason I can find is that it can't be considered as a bug so it was changed to RFE. Redmine issue remained open without further comments.

The use-case that I find useful is writing some description to the role. E.g. default role could have description that it's being shared for all users. Maybe we should put it into the seed file but the question is how to deal with localization. Anyway I'd like to add comments to my roles.

It complicates our form for little benefit IMO.

you mean the form that only has name field on the first tab?

[ares]

[test]

[ares]

@dhlavac could you please rebase?

[theforeman-bot]

Can an existing organization member please verify this patch?

[theforeman-bot]

Can an existing organization member please verify this patch?

[theforeman-bot]

Can an existing organization member please verify this patch?

[ares]

add to PR whitelist

[ares]

@dLobatog are you still against merging and do we need more opinions to decide?

[dLobatog]

On 09/21, Marek Hulán wrote:

@dLobatog are you still against merging and we need more opinions to decide?

I don't think it makes sense to have this or the other description field
PR I saw. The bugzilla associated is closed, and I can't find any logical
use for the field, but I won't block merging it.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#3684 (comment)

Daniel Lobato Garcia

@dLobatog
blog.daniellobato.me
daniellobato.me

GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: https://keybase.io/elobato

[bbuckingham]

Given that our RBAC is very fine grained and to a certain extent requires the user to know some of the internal implementation, I think the description on a role could be useful to users. It seems like in the future, it may be useful to have more default roles defined for the user. So that they could then use them directly or as input to build out roles specific to their needs. If/when that occurred, this could be useful in letting the user know what those default roles are.

[thomasmckay]

I'd +1 adding description too. RBAC can get very complex when used realistically for larger groups of users. Being able to add notes to a role would be a benefit.

[ares]

@dhlavac looks good, it only needs to list the new attribute in app/views/api/v2/roles/base.json.rabl so API lists it, otherwise 👍

[dhlavac]

@ares added

[ares]

Thanks @dhlavac, merged as 5384798. Could you please also create a simple patch for hammer-cli-foreman so it starts displaying the field?

[dhlavac]

@ares sure