Bug 50773 - Make Classlist Partially Configurable
Make Classlist Partially Configurable
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: UMC - Class lists
unspecified
Other Linux
: P5 normal (vote)
: UCS@school 4.4 v5-errata
Assigned To: Tobias Wenzel
Ole Schwiegert
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2020-02-04 12:27 CET by Christian Völker
Modified: 2020-06-25 10:27 CEST (History)
4 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: 2: Improvement: Would be a product improvement
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 3: A User would likely not purchase the product
User Pain: 0.069
Enterprise Customer affected?: Yes
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2020013121000452
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Völker univentionstaff 2020-02-04 12:27:13 CET
When generating the classlists customer likes to use his own attributes for the "class" field.

He uses the concept of a "main class" (as extended attribute) and therefore prefers to have this class in the field instead of a more or less randomly selected class.

Ideally having a UCRV defining the attribute name so it would not confuse end users.
Comment 1 Nico Stöckigt univentionstaff 2020-02-05 12:07:12 CET
The Use-Case differs between Germany and Suisse. While in Germany students are normally in a single class and are assigned to courses which also my differ separately (e.g. spanish grade a, spanisch grade b and spanish grade c) in Suisse the courses are assigned to classes and students are assigned to multiple classes instead to courses.
This might be a bit confusing but I guess this makes this FR a Bug.
Comment 2 Christian Völker univentionstaff 2020-02-11 17:03:18 CET
Wishes by customer:

a) (mindestens) die Spalte "Klasse" (welche aus heutiger Sicht, siehe auch den
von eröffneten Bug https://forge.univention.org/bugzilla/show_bug.cgi?id=50790,
nicht wirklich die gewünschten Ergebnisse liefert) sollte konfigurierbar sein.

b) Damit ist gemeint, dass die Spalte ERSETZT werden kann (nicht einfach eine
    zusätzliche Spalte)

c) Wir möchten ein beliebiges Attribut von der Person in die
    Spalte einsetzen

d) diese Konfigurierbarkeit sollte Teil vom Produkt werden (keine projektspezifische Änderung, die bei jedem Upgrade potentiell Schmerzen bereitet).
Comment 3 Tobias Wenzel univentionstaff 2020-06-11 14:07:13 CEST
There now exists a ucr-v ucsschool/umc/lists/class/attributes with default value
firstname Firstname,lastname Lastname,Class Class,username Username

The first value (except Class) ist a udm property, the second is the desired column name in the csv output. If Class is set, it holds the value of the selected class-name (works also without). Lists are concatenated. At this point all properties are possible. Feel free to reopen if we need a blacklist. 
If a non-existent udm property was chosen, an umc error is thrown which displays the first wrong one.

I also added a section in the manual which explains this .> jenkins branch-test for .
There exists a ucs-test which tests this for random udm properties which are already set and a unit-test which test the csv-file creation with random values.


[twenzel/50773_make_classlists_configurable] 7af51d537 Bug #50773: Unit-test for csv-creation
[twenzel/50773_make_classlists_configurable] 1cea3d4c6 Bug #50773: documentation
[twenzel/50773_make_classlists_configurable] 6da029371 Bug #50773: ucs-test and missing ucr.load
[twenzel/50773_make_classlists_configurable] 77e128dd2 Bug #50773: Test part 2
[twenzel/50773_make_classlists_configurable] 686daad45 Bug #50773: po file
[twenzel/50773_make_classlists_configurable] 272a0368d Bug #50773: displayName -> username & ucs-test
[twenzel/50773_make_classlists_configurable] cc8c67ecd Bug #50773: Raise umc_error with wrong udm-property
[twenzel/50773_make_classlists_configurable] 4500e5aca Bug #50773: add ucr-v description and read list in classlist module
Comment 4 Ole Schwiegert univentionstaff 2020-06-14 23:02:06 CEST
Okay the functionality looks good to me so far. Tested valid, invalid and unset UCRV.

Manual: OK

Following things remain to do: Rename 51_ucs-school-configurable-schoollists to 51_ucs-school-configurable-schoollists.py (So it will be catched by pre-commit)

Format code (maybe before merging + rebase)

51_ucs-school-schoollists-csv.py has the right idea, but it does not use the code to be tested at all, thus making it irrelevant. For best unit testing I would recommend extracting the CVS creating logic out of the def csv_list, import that function and test with it.

Please dont do value = student.get_udm_object(ldap_user_read)[attr] in every iteration of the loop. Extract the get_udm_object out of the loop.

The translation of the server side error message did not work for me. Please check.

The UCRV and its description was not shown  with ucr search after installing the package. Please check.
Comment 5 Tobias Wenzel univentionstaff 2020-06-15 16:18:55 CEST
Thanks for the QA!

[twenzel/50773_make_classlists_configurable] 6629de3db Bug #50773: QA Suggestions


The translation of the server side error message did not work for me. Please check.
-> please double-check, worked on my machine.
Comment 6 Tobias Wenzel univentionstaff 2020-06-16 15:33:21 CEST
As discussed, I added the improvements. Thanks for the suggestions:

- using UCSTestConfigRegistry
- no ucr.load (not needed)
- check csv content

[twenzel/50773_make_classlists_configurable] 8927426ae Bug #50773: Implemented further suggestions
Comment 7 Daniel Tröder univentionstaff 2020-06-16 23:45:30 CEST
OK: code
OK: documentation
OK: manual test
OK: new automated tests

Please merge, build package and create the advisory.
Comment 8 Tobias Wenzel univentionstaff 2020-06-17 09:53:16 CEST
Merged twenzel/50773_make_classlists_configurable to 4.4
(tests are passing after build)

[4.4] 9a44d75cf Bug #50773: yaml version
[4.4] 3d536c3c0 Bug #50773: Added changelog & yaml
[4.4] 32cb2511a Bug #50773: Fixed mergeconflicts
[4.4] 93dd62c7a Bug #50773: Formatting
[4.4] 8927426ae Bug #50773: Implemented further suggestions
[4.4] 6629de3db Bug #50773: QA Suggestions
[4.4] 7af51d537 Bug #50773: Unit-test for csv-creation
[4.4] 1cea3d4c6 Bug #50773: documentation
[4.4] 6da029371 Bug #50773: ucs-test and missing ucr.load
[4.4] 77e128dd2 Bug #50773: Test part 2
[4.4] 686daad45 Bug #50773: po file
[4.4] 272a0368d Bug #50773: displayName -> username & ucs-test
[4.4] cc8c67ecd Bug #50773: Raise umc_error with wrong udm-property
[4.4] 4500e5aca Bug #50773: add ucr-v description and read list in classlist module


Package: ucs-school-umc-lists
Version: 2.0.0-6A~4.4.0.202006170938
Branch: ucs_4.4-0
Scope: ucs-school-4.4

Package: ucs-test-ucsschool
Version: 6.0.113A~4.4.0.202006170945
Branch: ucs_4.4-0
Scope: ucs-school-4.4
Comment 9 Ole Schwiegert univentionstaff 2020-06-17 09:59:40 CEST
Changelog&Advisory: OK
Package installs: OK
Comment 10 Tobias Wenzel univentionstaff 2020-06-25 10:27:24 CEST
UCS@school 4.5 v5 has been released (errata update to the release).

http://docs.software-univention.de/changelog-ucsschool-4.4v5-de.html

If this error occurs again, please clone this bug.