Bug 53006 - remove confusing attributes from school resource, allow setting different share servers
Summary: remove confusing attributes from school resource, allow setting different sha...
Status: CLOSED FIXED
Alias: None
Product: UCS@school
Classification: Unclassified
Component: HTTP-API (Kelvin)
Version: UCS@school 4.4
Hardware: Other Linux
: P5 normal
Target Milestone: ---
Assignee: Daniel Tröder
QA Contact: Tobias Wenzel
URL:
Keywords:
Depends on: 52944
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-30 09:08 CEST by Daniel Tröder
Modified: 2021-06-30 16:14 CEST (History)
1 user (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Customer ID:
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Tröder univentionstaff 2021-03-30 09:08:53 CEST
+++ This bug was initially created as a clone of Bug #52944 +++

Kelvins school resource currently allows only to list/retrieve school objects.
Add the methods to create, modify and delete school objects.
Comment 1 Daniel Tröder univentionstaff 2021-03-30 09:14:54 CEST
Accidentally hit <Enter> before writing the OP text. Here it is:

The value of the single value attributes 'dc_name' and 'dc_name_administrative' is always 'None'. The values that can be stored in them when _creating_ a school are however in the multi-value attributes 'administrative_servers' and 'educational_servers'. That is a) confusing and b) makes them useless.

1. Remove those useless attributes from the resource.

Only one of the values of 'class_share_file_server' and 'home_share_file_server' will be used when creating a school, even if both are set. That is a) confusing and b) makes them less useful as they cannot be changed through Kelvin yet.

2. Allow setting both servers when creating a school.
Comment 2 Tobias Wenzel univentionstaff 2021-04-12 16:31:36 CEST
Code review: looks good, I added a couple of remarks/ suggestions here:

https://git.knut.univention.de/univention/ucsschool/-/commit/bb14092d77cd8917f239e4f3079cfc00dee2b46d

I also started to run the tests. On my VM (multi-server, primary node) the kelvin-api tests were failing - maybe this is because of my vm. I will retry tomorrow. The ucs-school-lib tests pass.
Comment 3 Daniel Tröder univentionstaff 2021-04-13 09:55:06 CEST
Thanks for the review. I replied and made this commit:

[dtroeder/53006_school_create_tests_WIP cce2c8ab1] Bug #53006: QA suggestions
Comment 4 Tobias Wenzel univentionstaff 2021-04-13 16:54:22 CEST
Thanks a lot for the refactoring!

The tests are almost green:

tests/test_route_school.py::test_search_no_filter
was failing in my multi-server env, in my single-server env it passed in the rerun.
Comment 5 Daniel Tröder univentionstaff 2021-04-19 13:01:28 CEST
'dc_name' and 'dc_name_administrative' have been removed and 'administrative_servers' and 'educational_servers' kept.

The following JSON is an example Schools resource:

    {
        "dn": "ou=test,dc=uni,dc=ven",
        "url": "https://<fqdn>/ucsschool/kelvin/v1/schools/test",
        "ucsschool_roles": ["school:school:test"],
        "name": "test",
        "display_name": "Test School",
        "educational_servers": ["cn=dctest-01"],
        "administrative_servers": [],
        "class_share_file_server": "dctest-01",
        "home_share_file_server": "dctest-01"
    }

Most work went into making the test stable.

Commits in 4.4 and feature/kelvin:

[feature/kelvin] 9af220447 Bug #53006: remove superfluous attributes, allow setting different class_share_file_server and home_share_file_server
[feature/kelvin] bd4addfc4 Bug #53006: add tests for schools resource
[feature/kelvin] 4c3fb5530 Bug #53006: improve ucsschool.lib and kelvin tests
[feature/kelvin] 2f99b3b4c Bug #53006: update documentation for school ressource

Docker image 1.4.0 was (re)build.

[4.4] 6b68ae0ae Bug #53006: fix ucslint warnings
[4.4] 0b7ae277b Bug #53006: fix kelvin tests
[4.4] a04a7318a Bug #53006: handle sporadic HTTP 502
[4.4] ea66a68b3 Bug #53006: cleanup

ucs-test-ucsschool (6.0.208)  (uploaded to 4.4 v9 in the test appcenter)
Comment 6 Tobias Wenzel univentionstaff 2021-04-20 11:45:59 CEST
QA → All OK → verify

Code → OK, looks great!
API Change → Works as expected, covered in tests, mentioned in docu

Jenkins → Green
Merge, commits & changelog → OK
Documentation → OK

Docker image 1.4.0 was (re)build.
→ in test-app center
ucs-test-ucsschool has been uploaded

-----
Release QA


Docu release → OK	

version correct → OK
Changelog  has to be updated in `kelvin-api/changelog.rst` `appcenter/README_UPDATE_DE` and `appcenter/README_UPDATE_EN` → OK

new installation
installation on single server → OK
installation in multi server → OK

update 1.3.0 → 1.4.0
single server → OK
multi server → OK

test to create school (post & get)
- single → OK 
- multi-server →  OK
Comment 7 Daniel Tröder univentionstaff 2021-06-30 16:14:00 CEST
The Kelvin API app with the change has been released.