Univention Bugzilla – Bug 52422
Add backend configuration to computer room administration
Last modified: 2021-03-24 14:09:19 CET
School administrators need to be able to choose between ITalc and Veyon during administration of computerrooms
Implemented on oschwieg/4.4/52422 - schoolless role veyon-backend was added - lib (RoleMixin) was modified to respect "-" school context type values correctly - Modified UMC module to be able to choose between italc and veyon as backend. Documentation is handled in a separate US Tests are following.
Testcases were implemented in 25_room_management_module_veyon_settings.py
Please REOPEN for merge&build
* The "role_workgroup_share" was made a tuple, typo? * In test_veyon_*setting(): please move the validation of the LDAP object to univention.testing.ucsschool.schoolroom.ComputerRoom.verify_ldap() and use that. * When creating a new room, none of the two options of the radio button is enabled. IMHO the default (iTalc) should be enabled. Please verify with the PO, that this is desired. * When creating a new room, and none of the two options of the radio button is enabled the object cannot be saved and no error is displayed. It gives the impression that the "save" button does not work. An error message should be displayed. In case a default setting is enabled as suggested in the point above, this problem will go away on its own.
I won't be able to green light this, as long as all Jenkins tests related to exams and computer rooms are failing with: ---- CookieConflictError: There are multiple cookies with name, 'UMCSessionId' ---
I have not a clue how I managed to make a tuple out of that role string... REVERTED A change in one file was not committed, sorry. ITALC is selected by default and thus it is not possible to reach the wizard without an option being selected for the radio buttons. Regarding the tests: I do not agree with the concept of the "verify_ldap_object" method. This function is intended to check for the correctness of the entire LDAP object. This means if anything with the object is wrong, but this particular feature with the backend role works, the test fails. IMHO this is not desired. The utils.verify_ldap_object() function is also not properly suited for the test, since I can only verify ldap attributes in a way, that I need to know the entire makeup of one attribute. In case of ucsschoolRole this means that I need to construct ALL role strings that are to be expected to ensure the function does not raise an Exception. Even if I am able to do that now, this means that at some point this test could fail, because in an unrelated Bug/Feature a new role was added that my test does not expect and thus fails. IMHO also not desired. If you disagree I am happy to resolve this in a face to face discussion. I am currently trying to figure out the jenkins problems with the cookies.
Merge Request on internal gitlab: https://git.knut.univention.de/univention/ucsschool/-/merge_requests/2
(In reply to Ole Schwiegert from comment #6) > I have not a clue how I managed to make a tuple out of that role string... > REVERTED OK: code change was reverted. > A change in one file was not committed, sorry. ITALC is selected by default > and thus it is not possible to reach the wizard without an option being > selected for the radio buttons. OK: room backend setting is now always set when opening the UMC module. > Regarding the tests: I do not agree with the concept of the > "verify_ldap_object" method. This function is intended to check for the > correctness of the entire LDAP object. This means if anything with the > object is wrong, but this particular feature with the backend role works, > the test fails. IMHO this is not desired. > The utils.verify_ldap_object() function is also not properly suited for the > test, since I can only verify ldap attributes in a way, that I need to know > the entire makeup of one attribute. In case of ucsschoolRole this means that > I need to construct ALL role strings that are to be expected to ensure the > function does not raise an Exception. Even if I am able to do that now, this > means that at some point this test could fail, because in an unrelated > Bug/Feature a new role was added that my test does not expect and thus > fails. IMHO also not desired. If you disagree I am happy to resolve this in > a face to face discussion. OK: I understand your reasoning. I'm fine with your decision to keep the test checking only the tested state. BTW: the test "25_room_management_module.py" now fails because »u'italc': True« is now part of »current attributes«, but not of »expected attributes« - exactly your reasoning! REOPEN: Anyway… I'll have to reopen this because the failing test 25_room_management_module.py". OK: manual tests: creating and changing rooms works and stores the setting in LDAP as expected. OK code and execution: automated test 25_room_management_module_veyon_settings OK: 8/10 test in "90_ucsschool/*room*" succeed. The failing ones are errors in the tests (bad IP address generatin in "22_computerroom_two_rooms_settings_interference.py" on my VM and the above problem in "25_room_management_module.py"). I have approved and executed the merge request, as I deem the production code OK and only see problems in the tests. Please add changelogs, build the three packages (ucs-school-lib, ucs-school-umc-rooms, ucs-test-ucsschool) and create the advisories.
Fixed the 25_* test by fixing the schoolroom module. Package: ucs-test-ucsschool Version: 6.0.166A~4.4.0.202011301737 Branch: ucs_4.4-0 Scope: ucs-school-4.4 Package: ucs-school-lib Version: 12.2.4A~4.4.0.202011301741 Branch: ucs_4.4-0 Scope: ucs-school-4.4 Package: ucs-school-umc-rooms Version: 16.1.0-5A~4.4.0.202011301742 Branch: ucs_4.4-0 Scope: ucs-school-4.4 Advisories added.
OK: 25_room_management_module*.py OK: Jenkins test OK: package builds and advisories
The package ucs-school-lib including changes from this bug were already released with the UCS@school 4.4v8 erratum on the 10.02.2021.
UCS@school 4.4 v9 has been released. https://docs.software-univention.de/changelog-ucsschool-4.4v9-de.html If this error occurs again, please clone this bug.