ILIAS
ILIAS copied to clipboard
Untranslated role title in header of the role GUI
@smeyer-ilias As discussed in #4676, this PR adds the untranslated role title in brackets to the header of role and role template GUIs, when the role (template) is autogenerated. Note that this required a tiny change to an OrgUnit class to retain its previous behavior, as well as a bunch of cs fixes in that class.
@smeyer-ilias This looks like a change in Services/AccessControl
which should also be reviewed by @mstuder as maintainer of Stuff
.
@mstuder Could you please take a look at this? It would be nice to have untranslated role displayed again for assigning the local role when creating registration codes.
Hello everyone, I have only limited hope that we will achieve a result in this way. Do we have another party who could be responsible for a review of the changes in staff? Thank you all.
The code changes are in two components:
Modules/OrgUnit (now maintained by @klees ) Services/AccessControl (now maintained by @kergomard)
@klees Since you removed your assignment, I will not add you again.
Thanks @schmitz-ilias for the PR. Ok, I understand the use-case for this change. I have some doubts about the implementation though: Where does the change in OrgUnits surface? I'm not really familiar with the OrgUnits. Couldn't we just show the complete title there, too? My reasoning: I don't like the introduction of a new function for just one use-case even though right now we are using the "short form" of the Role Title in many places. I also don't think it is the right nomenclature. We are adding "Long" and "Short" willy-nilly all over the place, just because we can not rename the main function. Personally I would like to remove the exception for the OrgUnits and just use the "long form" everywhere.
Thanks and best, @kergomard
@tfamula and I had a closer look at ilOrgUnitStaffGUI
, and it seems like ilOrgUnitStaffGUI::addOtherRolesToToolbar
is an unused remnant of a previous refactoring in OrgUnits
, so no OrgUnit
or Staff
view will be affected by this PR. Accordingly, I dropped getShortPresenationTitle
.
Quick sidenote @klees: it looks like ilOrgUnitStaffGUI
is at most passing on ilCtrl
calls to other classes, and might not do anything at all. It is not needed in Staff
as far as we can tell.
Lastly, this PR was originally intended to also be merged to R7, so I changed the target branch to reflect that. @kergomard , let me know if there is anything else.
(PS: Apologies to all who got notified about me pushing ~250 commits, I might have force-pushed and changed target branches in the wrong order?)
Thanks for the update @schmitz-ilias ! I will merge and cherry-pick to 8 and trunk. Best, @kergomard