ILIAS icon indicating copy to clipboard operation
ILIAS copied to clipboard

Untranslated role title in header of the role GUI

Open schmitz-ilias opened this issue 2 years ago • 2 comments

@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.

schmitz-ilias avatar Jul 25 '22 09:07 schmitz-ilias

@smeyer-ilias This looks like a change in Services/AccessControl which should also be reviewed by @mstuder as maintainer of Stuff.

klees avatar Aug 02 '22 04:08 klees

@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.

iljalukin avatar Sep 19 '22 08:09 iljalukin

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.

oliversamoila avatar Nov 09 '22 09:11 oliversamoila

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.

alex40724 avatar Apr 28 '23 16:04 alex40724

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

kergomard avatar May 02 '23 07:05 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?)

schmitz-ilias avatar May 04 '23 14:05 schmitz-ilias

Thanks for the update @schmitz-ilias ! I will merge and cherry-pick to 8 and trunk. Best, @kergomard

kergomard avatar May 04 '23 16:05 kergomard