[Feature]: Spieler Listen Endpunkt #6

Merged
SZUT-Mehdi merged 1 commit from TD-34-spieler-listen-endpunkt into trunk 2025-03-05 09:39:18 +00:00
Member

Ticket

TD-34

Beschreibung

Ich habe den endpunkt implementiert, um alle players zu getten. Und ich habe die Tests implementiert, um diesen Endpunkt auf seine funktionalitäten auf ihre korrektheit zu überprüfen.

Weitere Infos

No response

### Ticket TD-34 ### Beschreibung Ich habe den endpunkt implementiert, um alle players zu getten. Und ich habe die Tests implementiert, um diesen Endpunkt auf seine funktionalitäten auf ihre korrektheit zu überprüfen. ### Weitere Infos _No response_
SZUT-Mehdi added 4 commits 2025-02-26 09:33:01 +00:00
TD-34: fixed bugs in tests
Some checks failed
Quality Check / Linting (push) Failing after 1m6s
Quality Check / Validate OAS (push) Successful in 3m35s
Quality Check / Testing (push) Successful in 4m5s
Quality Check / Static Analysis (push) Successful in 4m40s
Quality Check / Validate OAS (pull_request) Successful in 31s
Quality Check / Linting (pull_request) Failing after 49s
Quality Check / Testing (pull_request) Successful in 56s
Quality Check / Static Analysis (pull_request) Successful in 58s
06121ddb93
SZUT-Mehdi added 1 commit 2025-02-26 09:35:03 +00:00
TD-34: fixed main lints
All checks were successful
Quality Check / Validate OAS (push) Successful in 34s
Quality Check / Linting (push) Successful in 1m9s
Quality Check / Testing (push) Successful in 1m14s
Quality Check / Static Analysis (push) Successful in 1m17s
Quality Check / Linting (pull_request) Successful in 1m26s
Quality Check / Static Analysis (pull_request) Successful in 58s
Quality Check / Testing (pull_request) Successful in 54s
Quality Check / Validate OAS (pull_request) Successful in 3m39s
e04371373a
SZUT-Mehdi self-assigned this 2025-02-26 09:36:41 +00:00
SZUT-Rajbir changed title from [Feature]: Spieler Listen Endpunkt to WIP: [Feature]: Spieler Listen Endpunkt 2025-02-26 09:37:52 +00:00
SZUT-Dorian was assigned by SZUT-Mehdi 2025-02-26 09:38:03 +00:00
SZUT-Rajbir was assigned by SZUT-Mehdi 2025-02-26 09:38:04 +00:00
requested reviews from snoweuph, SZUT-Kevin 2025-02-26 09:38:38 +00:00
SZUT-Mehdi changed title from WIP: [Feature]: Spieler Listen Endpunkt to [Feature]: Spieler Listen Endpunkt 2025-02-26 09:39:13 +00:00
snoweuph requested changes 2025-02-26 09:40:29 +00:00
Dismissed
snoweuph left a comment
Owner

image

Die Commit Historie ist nicht Sauber

![image](/attachments/7103cb8b-4aa4-46ad-b663-690ff9e14d06) Die Commit Historie ist nicht Sauber
150 KiB
snoweuph reviewed 2025-02-26 09:42:12 +00:00
@ -39,0 +60,4 @@
Pageable pageable = PageRequest.of(page, pageSize, Sort.by(direction, sortBy));
Page<Player> playerPage = (username != null && !username.trim().isEmpty())
? playerRepository.findByUsernameContainingIgnoreCase(username, pageable)
Owner

Bitte keine Tenaries nutzen, die machen Code unleserlicher, indem sie Logik hinter magic verstecken

Bitte keine Tenaries nutzen, die machen Code unleserlicher, indem sie Logik hinter magic verstecken
SZUT-Dorian marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:42:46 +00:00
@ -38,1 +53,4 @@
}
@Override
public ResponseEntity<List<PlayerApiModel>> getAllPlayers(Integer page, Integer pageSize, String sortBy,
Owner

Bitte einmal Formatieren

Bitte einmal Formatieren
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:44:22 +00:00
api/api.yml Outdated
@ -149,0 +193,4 @@
schema:
type: string
enum: [asc, desc]
default: "asc"
Owner

Enums in OAS sollten kein defautl haben

Enums in OAS sollten kein defautl haben
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:44:36 +00:00
api/api.yml Outdated
@ -149,0 +192,4 @@
required: false
schema:
type: string
enum: [asc, desc]
Owner

Bitte keine abkürzungen.

Bitte keine abkürzungen.
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:46:31 +00:00
api/api.yml Outdated
@ -149,0 +166,4 @@
description: "Returns a paginated list of players, optionally filtered by username."
parameters:
- name: page
in: query
Owner

bitte nutze einen request Body und nicht query parameter dafür

bitte nutze einen request Body und nicht query parameter dafür
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:46:42 +00:00
api/api.yml Outdated
@ -149,0 +172,4 @@
schema:
type: integer
default: 0
- name: pageSize
Owner

bitte nutze einen request Body und nicht query parameter dafür

bitte nutze einen request Body und nicht query parameter dafür
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:46:48 +00:00
api/api.yml Outdated
@ -149,0 +179,4 @@
schema:
type: integer
default: 10
- name: sortBy
Owner

bitte nutze einen request Body und nicht query parameter dafür

bitte nutze einen request Body und nicht query parameter dafür
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:47:00 +00:00
api/api.yml Outdated
@ -149,0 +186,4 @@
schema:
type: string
default: "username"
- name: order
Owner

bitte nutze einen request Body und nicht query parameter dafür

bitte nutze einen request Body und nicht query parameter dafür
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:47:57 +00:00
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from e04371373a to 35b8c33582 2025-02-26 09:48:24 +00:00 Compare
snoweuph reviewed 2025-02-26 09:51:20 +00:00
api/api.yml Outdated
@ -59,2 +59,4 @@
required:
- username
#############################################
# PlayerApiModel #
Owner

Bitte nenne es AdministratablePlayer

API und Model haben nichts in den Namen der Daten zu tuhen, außer wir senden eine API in der API

Bitte nenne es `AdministratablePlayer` API und Model haben nichts in den Namen der Daten zu tuhen, außer wir senden eine API in der API
SZUT-Mehdi marked this conversation as resolved
snoweuph requested changes 2025-02-26 09:55:37 +00:00
Dismissed
@ -39,0 +60,4 @@
Pageable pageable = PageRequest.of(page, pageSize, Sort.by(direction, sortBy));
Page<Player> playerPage = (username != null && !username.trim().isEmpty())
? playerRepository.findByUsernameContainingIgnoreCase(username, pageable)
Owner

Keine Tenaries nutzen, sind zu viel Magic und machen Code unleserlich

Keine Tenaries nutzen, sind zu viel Magic und machen Code unleserlich
SZUT-Mehdi marked this conversation as resolved
@ -39,0 +65,4 @@
List<PlayerApiModel> playersMapped = playerMapperService.mapPlayerToApiPlayers(playerPage.getContent());
return playersMapped.isEmpty() ? ResponseEntity.noContent().build() : ResponseEntity.ok(playersMapped);
Owner

Keine Tenaries nutzen, sind zu viel Magic und machen Code unleserlich

Keine Tenaries nutzen, sind zu viel Magic und machen Code unleserlich
SZUT-Mehdi marked this conversation as resolved
@ -0,0 +10,4 @@
@Component
public class PlayerMapperService {
public List<PlayerApiModel> mapPlayerToApiPlayers(List<Player> players) {
Owner

Bitte in 2 Funktionen splitten

mapPlayerToAdministratablePlayer <-- Für einen Spieler
Für mehrere Spieler -> mapPlayersToAdministratablePlayers, welche die erste benutzt und den for loop hat

Bitte in 2 Funktionen splitten `mapPlayerToAdministratablePlayer` <-- Für einen Spieler Für mehrere Spieler -> `mapPlayersToAdministratablePlayers`, welche die erste benutzt und den for loop hat
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:57:22 +00:00
@ -29,2 +34,4 @@
void setUp() {
playerRepository.deleteAll();
Player player1 = new Player();
Owner

Bitte einen Map und einen Loop nutzen für das Anlegen.

Dies würde den code lesbarer machen

Bitte einen Map und einen Loop nutzen für das Anlegen. Dies würde den code lesbarer machen
SZUT-Mehdi marked this conversation as resolved
snoweuph reviewed 2025-02-26 09:57:40 +00:00
@ -31,0 +50,4 @@
this.playerRepository.save(player2);
System.out.println("LISTE: " + playerRepository.findAll());
Owner

Log entfernen, hat hier nix zu suchen.

Log entfernen, hat hier nix zu suchen.
SZUT-Mehdi marked this conversation as resolved
SZUT-Mehdi added 1 commit 2025-02-26 10:51:50 +00:00
TD-34: Solved comments made by reviewer
Some checks failed
Quality Check / Validate OAS (push) Successful in 30s
Quality Check / Linting (push) Failing after 52s
Quality Check / Testing (push) Successful in 1m4s
Quality Check / Validate OAS (pull_request) Successful in 34s
Quality Check / Static Analysis (push) Successful in 1m37s
Quality Check / Linting (pull_request) Failing after 51s
Quality Check / Testing (pull_request) Successful in 55s
Quality Check / Static Analysis (pull_request) Successful in 59s
2ccc9f14c5
SZUT-Mehdi added 1 commit 2025-02-26 11:14:15 +00:00
TD-34: Solved comments made by reviewer
Some checks failed
Quality Check / Validate OAS (pull_request) Successful in 34s
Quality Check / Validate OAS (push) Successful in 38s
Quality Check / Linting (push) Failing after 1m6s
Quality Check / Testing (push) Successful in 1m18s
Quality Check / Static Analysis (push) Successful in 1m27s
Quality Check / Linting (pull_request) Failing after 1m19s
Quality Check / Static Analysis (pull_request) Successful in 1m19s
Quality Check / Testing (pull_request) Successful in 57s
e388bb0020
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from e388bb0020 to 58c2037443 2025-02-26 11:20:17 +00:00 Compare
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from 58c2037443 to 440160980d 2025-02-26 11:32:16 +00:00 Compare
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from 440160980d to 2740cc53be 2025-02-26 11:37:25 +00:00 Compare
requested review from snoweuph 2025-02-26 11:40:34 +00:00
snoweuph requested changes 2025-02-26 12:46:34 +00:00
Dismissed
api/api.yml Outdated
@ -61,0 +72,4 @@
#############################################
# GetAllPlayersConfigurationData #
#############################################
GetAllPlayersConfigurationData:
Owner

Bitte nenne esPlayerFilter

Bitte nenne es`PlayerFilter`
SZUT-Mehdi marked this conversation as resolved
api/api.yml Outdated
@ -61,0 +78,4 @@
properties:
page:
type: integer
default: 0
Owner

Keine Default Wertte

Keine Default Wertte
SZUT-Mehdi marked this conversation as resolved
api/api.yml Outdated
@ -61,0 +82,4 @@
description: "Page number (zero-based index)."
pageSize:
type: integer
default: 10
Owner

Keine Default Werte

Keine Default Werte
SZUT-Mehdi marked this conversation as resolved
api/api.yml Outdated
@ -61,0 +86,4 @@
description: "Number of players per page."
sortBy:
type: string
default: "username"
Owner

Keine Default Wete

Keine Default Wete
SZUT-Mehdi marked this conversation as resolved
api/api.yml Outdated
@ -61,0 +94,4 @@
description: "Sorting order (asc or desc)."
username:
type: string
description: "Filter players by username (case-insensitive, partial match)."
Owner

page, pagesize, order sollten pflicht felder sein.

page, pagesize, order sollten pflicht felder sein.
SZUT-Mehdi marked this conversation as resolved
api/api.yml Outdated
@ -149,0 +195,4 @@
application/json:
schema:
$ref: "#/components/schemas/GetAllPlayersConfigurationData"
tags:
Owner

Bitte direkt nach der operationID schreiben, für einheitlichkeit

Bitte direkt nach der operationID schreiben, für einheitlichkeit
SZUT-Mehdi marked this conversation as resolved
@ -39,0 +56,4 @@
@Override
public ResponseEntity<List<AdministratablePlayer>> getAllPlayers(GetAllPlayersConfigurationData body) {
var order = body.getOrder();
Owner

Wir nutzen das var Keyword nicht, weil es seine Unsicherheiten hat

Wir nutzen das `var` Keyword nicht, weil es seine Unsicherheiten hat
SZUT-Mehdi marked this conversation as resolved
@ -39,0 +62,4 @@
var sortBy = body.getSortBy();
var username = body.getUsername();
if (order == null) {
Owner

Keine Default Werte für die Order.
Sollte required sein in der API.

Das Frontend wird ja auch einen Default anzeigen und somit setzten.

Es ist also aufgabe des Frontends dieses Festzulegen und nicht des Servers

Keine Default Werte für die Order. Sollte required sein in der API. Das Frontend wird ja auch einen Default anzeigen und somit setzten. Es ist also aufgabe des Frontends dieses Festzulegen und nicht des Servers
SZUT-Mehdi marked this conversation as resolved
@ -39,0 +67,4 @@
}
Sort.Direction direction;
if (order == GetAllPlayersConfigurationData.OrderEnum.ASCENDING) {
Owner

Für dieses mapping, bitte einen Mapper anlegen

Für dieses mapping, bitte einen Mapper anlegen
SZUT-Mehdi marked this conversation as resolved
@ -39,0 +77,4 @@
Page<Player> playerPage;
if (username != null && !username.trim().isEmpty()) {
Owner

Dieser Endpunkt ist immer Paged, deswegen wird dieses If nicht benötigt und die API Spec sollte es auch wieder spiegeln. (Siehe andere Nachricht)

Dieser Endpunkt ist immer Paged, deswegen wird dieses If nicht benötigt und die API Spec sollte es auch wieder spiegeln. (Siehe andere Nachricht)
SZUT-Mehdi marked this conversation as resolved
@ -39,0 +87,4 @@
playerMapperService.mapPlayersToAdministratablePlayers(playerPage.getContent());
if (playersMapped.isEmpty()) {
return ResponseEntity.noContent().build();
Owner

No Content ist ein 204 der für Post und Put Requests genutzt wird und nicht für Get Requests.

Außerdem sollte einfach eine leere Page returned werden, wenn es keine Spieler gibt (Ein Leerer Array ist immer noch ein Valider JSON Array)

No Content ist ein 204 der für Post und Put Requests genutzt wird und nicht für Get Requests. Außerdem sollte einfach eine leere Page returned werden, wenn es keine Spieler gibt (Ein Leerer Array ist immer noch ein Valider JSON Array)
SZUT-Mehdi marked this conversation as resolved
@ -0,0 +11,4 @@
public class PlayerMapperService {
public List<AdministratablePlayer> mapPlayersToAdministratablePlayers(List<Player> players) {
List<AdministratablePlayer> apiPlayers = new ArrayList<>();
Owner

Variable name ist noch falsch

Variable name ist noch falsch
SZUT-Mehdi marked this conversation as resolved
@ -0,0 +13,4 @@
public List<AdministratablePlayer> mapPlayersToAdministratablePlayers(List<Player> players) {
List<AdministratablePlayer> apiPlayers = new ArrayList<>();
for (Player player : players) {
AdministratablePlayer apiPlayer = new AdministratablePlayer();
Owner

Variable name ist noch falsch

Variable name ist noch falsch
SZUT-Mehdi marked this conversation as resolved
@ -31,0 +49,4 @@
} catch (NoSuchAlgorithmException e) {
System.err.println("Error setting password for player: " + username);
}
});
Owner

Statt dem Save oben, hier direkt nach dem loop einmal ein Save und Flush ausführen:

playerRepository.saveAllAndFlush(players.values());
Statt dem Save oben, hier direkt nach dem loop einmal ein Save und Flush ausführen: ```java playerRepository.saveAllAndFlush(players.values()); ```
SZUT-Mehdi marked this conversation as resolved
@ -0,0 +19,4 @@
@Test
void playersExist() throws Exception {
String requestBody = "{" +
Owner

Nope, wir schreiben kein Raw JSON.

https://git.euph.dev/SZUT/ProjectManagmentTool/src/branch/trunk/src/test/java/de/hmmh/pmt/project/CreateTest.java#L102

Mithilfe eines Object Mappers kannst du die passenden Objekte direkt in JSON umwandeln, ohne dabei das Problem zu haben, dass du potenziell invalides JSON schreibst.

Außerdem erhöht es die lesbarkeit des Codes

Nope, wir schreiben kein Raw JSON. https://git.euph.dev/SZUT/ProjectManagmentTool/src/branch/trunk/src/test/java/de/hmmh/pmt/project/CreateTest.java#L102 Mithilfe eines Object Mappers kannst du die passenden Objekte direkt in JSON umwandeln, ohne dabei das Problem zu haben, dass du potenziell invalides JSON schreibst. Außerdem erhöht es die lesbarkeit des Codes
SZUT-Mehdi marked this conversation as resolved
SZUT-Mehdi added 1 commit 2025-03-05 07:48:27 +00:00
TD-34: Endpoint for getting all players and tests
Some checks failed
Quality Check / Validate OAS (push) Successful in 38s
Quality Check / Validate OAS (pull_request) Successful in 37s
Quality Check / Static Analysis (push) Failing after 1m6s
Quality Check / Testing (push) Failing after 1m16s
Quality Check / Linting (push) Successful in 1m16s
Quality Check / Linting (pull_request) Successful in 1m8s
Quality Check / Static Analysis (pull_request) Failing after 1m13s
Quality Check / Testing (pull_request) Failing after 52s
35b538c7ef
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from 35b538c7ef to 1e8c15d52f 2025-03-05 07:54:00 +00:00 Compare
requested review from snoweuph 2025-03-05 07:56:04 +00:00
SZUT-Kevin approved these changes 2025-03-05 09:23:18 +00:00
Dismissed
SZUT-Kevin left a comment
Member

Perfekt, hat mir richtig gut gefallen, musste aber selber lesen. 1/5 Sternen

Perfekt, hat mir richtig gut gefallen, musste aber selber lesen. 1/5 Sternen
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from 1e8c15d52f to 68ac0e6e5d 2025-03-05 09:27:36 +00:00 Compare
SZUT-Mehdi dismissed SZUT-Kevin's review 2025-03-05 09:27:37 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from 68ac0e6e5d to f6ac586f28 2025-03-05 09:29:48 +00:00 Compare
requested reviews from SZUT-Kevin and removed review requests for snoweuph 2025-03-05 09:31:58 +00:00
SZUT-Mehdi force-pushed TD-34-spieler-listen-endpunkt from f6ac586f28 to 417739e6b0 2025-03-05 09:34:07 +00:00 Compare
SZUT-Kevin approved these changes 2025-03-05 09:34:59 +00:00
requested review from snoweuph 2025-03-05 09:37:01 +00:00
refused to review 2025-03-05 09:38:35 +00:00
snoweuph dismissed snoweuph's review 2025-03-05 09:38:53 +00:00
Reason:

veraltet

snoweuph approved these changes 2025-03-05 09:39:07 +00:00
SZUT-Mehdi merged commit 9223823239 into trunk 2025-03-05 09:39:18 +00:00
SZUT-Mehdi deleted branch TD-34-spieler-listen-endpunkt 2025-03-05 09:39:18 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TowerDefence/Server#6
No description provided.