Een groepcluster vullen met 1 request #252
Conversation
|
Ik ga hier nog een backend test voor schrijven, dan ga ik de PR open zetten |
|
@arnedierick ik zal anders de testen wel schrijven. Kheb basically alle andere testen ook aangepast (geupgrade) in mijn komende PR dus kneem deze er wel gwn bij shouldn't take long |
dit is geen ding, die count is gwn een (lowkey overbodig at this point) tellertje da bijhoudt hoeveel groepen er zijn. Das geen maximum fzo. Alleja ig da de route wel bestaande groepen opvult ma dan zou ik eerder checken of die groepen valid zijn en gekoppeld aan de cluster |
Dit zou wel moeten gebeuren |
| GroupClusterJson clusterJson = (GroupClusterJson) response.getBody(); | ||
| if(clusterJson == null){ | ||
| return ResponseEntity.status(HttpStatus.NOT_FOUND).body("Group cluster could not be found"); | ||
| } |
There was a problem hiding this comment.
Dit lijkt me overbodig aangezien je de GroupClusterEntity al hebt
There was a problem hiding this comment.
Ik doe dit omdat een GroupClusterEntity niet de groep-info bevat om alle groepen te verwijderen
There was a problem hiding this comment.
GroupRepository.findAllByClusterId(ClusterId) zou ik hier dan voor gebruiken
| if(clusterFillJson.getClusterGroupMembers().keySet().size() > clusterJson.groupCount()){ | ||
| return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("provided more groups than are allowed in the cluster"); | ||
| } |
| if(clusterFillJson.getClusterGroupMembers().values().stream().anyMatch(members -> members.length > clusterJson.capacity())){ | ||
| return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("you made a group with too many members"); |
There was a problem hiding this comment.
Ik zou dus voor elke groep checken in de clusterFillJson checken of
- de groep een valid groep is & gekoppeld aan die cluster
- er niet meer members zijn dan de maxSize
- alle users valid users zijn die in de course zitten
Eenmaal dit voor alle groepen id request gecheckt is kunnen ze toegevoegd worden zoals je dit reeds doet
Oeps, ik heb dit ondertussen al gedaan. je kan die test aanpassen als je hem wil veranderen |
|
Ik zal de requested changes implementeren na de vergadering |
…s it isn't up to date anymore
|
Ok, de gevraagde changes zijn nu geïmplementeerd. We checken of de groep deel is van de cluster, en of alle geselecteerde users geldige studenten zijn van dat vak. De test is wel niet meer up to date. Ik heb ze in commentaar gezet. Ze zou vrij gemakkelijk aan te passen moeten zijn. Ik denk dat een nieuwe review kan gebeuren |
|
Top, ik zal hiernaar kijken wanneer ik klaar ben met de andere testen te schrijven (zal waarschijnlijk pas morgen zijn tho) |
|
Top, de changes die je gedaan hebt maken de route veiliger en makkelijker om te gebruiken. Je mag mergen! |
Ik heb een route toegevoegd /api/clusters/{clusterid}/fill die de groepen van een bestaande cluster volledig overschrijft. Het is belangrijk om op te merken dat dit de hele cluster reset. Het gaat als volgt: