From 2504b75b87e9e359127fe7320e91b8fda4c97a2f Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 03:38:57 +0200 Subject: [PATCH 01/14] Leaderboard After + Before --- server/application-server/openapi.yaml | 29 ++++++++++++------- .../leaderboard/LeaderboardController.java | 8 +++-- .../leaderboard/LeaderboardService.java | 20 +++++++++---- .../src/main/resources/application.yml | 2 +- .../openapi/api/leaderboard.service.ts | 21 +++++++++++--- .../api/leaderboard.serviceInterface.ts | 4 ++- .../modules/openapi/model/pull-request.ts | 2 +- .../app/core/modules/openapi/model/user.ts | 4 +-- webapp/src/app/home/home.component.ts | 15 +++++++++- 9 files changed, 77 insertions(+), 28 deletions(-) diff --git a/server/application-server/openapi.yaml b/server/application-server/openapi.yaml index 2b04452c..17a4aa9d 100644 --- a/server/application-server/openapi.yaml +++ b/server/application-server/openapi.yaml @@ -94,6 +94,19 @@ paths: tags: - leaderboard operationId: getLeaderboard + parameters: + - name: before + in: query + required: false + schema: + type: string + format: date-time + - name: after + in: query + required: false + schema: + type: string + format: date-time responses: "200": description: OK @@ -241,9 +254,8 @@ components: type: string state: type: string - description: |- - State of the PullRequest. - Does not include the state of the merge. + description: "State of the PullRequest.\r\n Does not include the state of\ + \ the merge." enum: - CLOSED - OPEN @@ -397,15 +409,12 @@ components: description: Display name of the user. url: type: string - description: |- - Unique URL to the user's profile. - Not the website a user can set in their profile. + description: "Unique URL to the user's profile.\r\n Not the website a user\ + \ can set in their profile." avatarUrl: type: string - description: |- - URL to the user's avatar. - If unavailable, a fallback can be generated from the login, e.g. on Github: - https://github.com/{login}.png + description: "URL to the user's avatar.\r\n If unavailable, a fallback can\ + \ be generated from the login, e.g. on Github:\r\n https://github.com/{login}.png" type: type: string description: Type of the user. Used to distinguish between users and bots. diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java index ed32ba59..aafb9a8f 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java @@ -1,10 +1,13 @@ package de.tum.in.www1.hephaestus.leaderboard; +import java.time.OffsetDateTime; import java.util.List; +import java.util.Optional; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @RestController @@ -18,7 +21,8 @@ public LeaderboardController(LeaderboardService leaderboardService) { } @GetMapping - public ResponseEntity> getLeaderboard() { - return ResponseEntity.ok(leaderboardService.createLeaderboard()); + public ResponseEntity> getLeaderboard(@RequestParam Optional before, + @RequestParam Optional after) { + return ResponseEntity.ok(leaderboardService.createLeaderboard(before, after)); } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java index 33cbbb30..3e1deabd 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java @@ -8,6 +8,7 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -36,14 +37,17 @@ public LeaderboardService(UserService userService) { this.userService = userService; } - public List createLeaderboard() { + public List createLeaderboard(Optional before, Optional after) { logger.info("Creating leaderboard dataset"); List users = userService.getAllUsers(); - logger.info("Leaderboard has " + users.size() + " users"); - OffsetDateTime cutOffTime = new Date(System.currentTimeMillis() - 1000 * 60 * 60 * 24 * timeframe) - .toInstant().atOffset(ZoneOffset.UTC); + OffsetDateTime afterCutOff = after.isPresent() ? after.get() + : new Date(System.currentTimeMillis() - 1000 * 60 * 60 * 24 * timeframe).toInstant() + .atOffset(ZoneOffset.UTC); + + logger.info("Leaderboard has " + users.size() + " users with cut-off time " + afterCutOff + " and before time " + + before); List leaderboard = users.stream().map(user -> { if (user.getType() != UserType.USER) { @@ -55,8 +59,8 @@ public List createLeaderboard() { Set commentSet = new HashSet<>(); user.getReviews().stream() - .filter(review -> (review.getCreatedAt() != null && review.getCreatedAt().isAfter(cutOffTime)) - || (review.getUpdatedAt() != null && review.getUpdatedAt().isAfter(cutOffTime))) + .filter(review -> isInTimeframe(review.getCreatedAt(), before, afterCutOff) + || isInTimeframe(review.getUpdatedAt(), before, afterCutOff)) .forEach(review -> { if (review.getPullRequest().getAuthor().getLogin().equals(user.getLogin())) { return; @@ -99,6 +103,10 @@ public List createLeaderboard() { return leaderboard; } + private boolean isInTimeframe(OffsetDateTime date, Optional before, OffsetDateTime after) { + return date != null && (before.isPresent() && date.isAfter(before.get()) || date.isBefore(after)); + } + /** * Calculates the score for a given pull request. * Possible values: 1, 3, 7, 17, 33. diff --git a/server/application-server/src/main/resources/application.yml b/server/application-server/src/main/resources/application.yml index aebef22e..324d01dd 100644 --- a/server/application-server/src/main/resources/application.yml +++ b/server/application-server/src/main/resources/application.yml @@ -33,7 +33,7 @@ springdoc: monitoring: runOnStartup: true # Fetching timeframe in days - timeframe: 7 + timeframe: 1 repository-sync-cron: "0 0 * * * *" repositories: ls1intum/Artemis diff --git a/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts b/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts index 7f01bf4a..4fc3001d 100644 --- a/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts +++ b/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts @@ -96,13 +96,25 @@ export class LeaderboardService implements LeaderboardServiceInterface { } /** + * @param before + * @param after * @param observe set whether or not to return the data Observable as the body, response or events. defaults to returning the body. * @param reportProgress flag to report request and response progress. */ - public getLeaderboard(observe?: 'body', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>; - public getLeaderboard(observe?: 'response', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; - public getLeaderboard(observe?: 'events', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; - public getLeaderboard(observe: any = 'body', reportProgress: boolean = false, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable { + public getLeaderboard(before?: string, after?: string, observe?: 'body', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>; + public getLeaderboard(before?: string, after?: string, observe?: 'response', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; + public getLeaderboard(before?: string, after?: string, observe?: 'events', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; + public getLeaderboard(before?: string, after?: string, observe: any = 'body', reportProgress: boolean = false, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable { + + let localVarQueryParameters = new HttpParams({encoder: this.encoder}); + if (before !== undefined && before !== null) { + localVarQueryParameters = this.addToHttpParams(localVarQueryParameters, + before, 'before'); + } + if (after !== undefined && after !== null) { + localVarQueryParameters = this.addToHttpParams(localVarQueryParameters, + after, 'after'); + } let localVarHeaders = this.defaultHeaders; @@ -144,6 +156,7 @@ export class LeaderboardService implements LeaderboardServiceInterface { return this.httpClient.request>('get', `${this.configuration.basePath}${localVarPath}`, { context: localVarHttpContext, + params: localVarQueryParameters, responseType: responseType_, withCredentials: this.configuration.withCredentials, headers: localVarHeaders, diff --git a/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts b/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts index dcd0ca27..c41edbf2 100644 --- a/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts +++ b/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts @@ -27,7 +27,9 @@ export interface LeaderboardServiceInterface { /** * * + * @param before + * @param after */ - getLeaderboard(extraHttpRequestParams?: any): Observable>; + getLeaderboard(before?: string, after?: string, extraHttpRequestParams?: any): Observable>; } diff --git a/webapp/src/app/core/modules/openapi/model/pull-request.ts b/webapp/src/app/core/modules/openapi/model/pull-request.ts index f1fa06ad..9de86033 100644 --- a/webapp/src/app/core/modules/openapi/model/pull-request.ts +++ b/webapp/src/app/core/modules/openapi/model/pull-request.ts @@ -23,7 +23,7 @@ export interface PullRequest { title: string; url: string; /** - * State of the PullRequest. Does not include the state of the merge. + * State of the PullRequest. Does not include the state of the merge. */ state: PullRequest.StateEnum; additions?: number; diff --git a/webapp/src/app/core/modules/openapi/model/user.ts b/webapp/src/app/core/modules/openapi/model/user.ts index 8e8311bd..371ca15c 100644 --- a/webapp/src/app/core/modules/openapi/model/user.ts +++ b/webapp/src/app/core/modules/openapi/model/user.ts @@ -29,11 +29,11 @@ export interface User { */ name?: string; /** - * Unique URL to the user\'s profile. Not the website a user can set in their profile. + * Unique URL to the user\'s profile. Not the website a user can set in their profile. */ url: string; /** - * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png + * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png */ avatarUrl?: string; /** diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index fe173d4d..3c7f1702 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -1,4 +1,5 @@ import { Component, inject } from '@angular/core'; +import { ActivatedRoute } from '@angular/router'; import { injectQuery } from '@tanstack/angular-query-experimental'; import { LeaderboardService } from 'app/core/modules/openapi/api/leaderboard.service'; import { LeaderboardComponent } from 'app/home/leaderboard/leaderboard.component'; @@ -13,9 +14,21 @@ import { lastValueFrom } from 'rxjs'; export class HomeComponent { leaderboardService = inject(LeaderboardService); + // timeframe for leaderboard + // example: 2024-09-19T10:15:30+01:00 + protected after: string | undefined = undefined; + protected before: string | undefined = undefined; + + constructor() { + inject(ActivatedRoute).queryParamMap.subscribe((params) => { + this.after = params.get('after')?.replace(' ', '+') ?? undefined; + this.before = params.get('before')?.replace(' ', '+') ?? undefined; + }); + } + query = injectQuery(() => ({ queryKey: ['leaderboard'], - queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard()), + queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before, this.after, undefined, undefined, undefined)), gcTime: Infinity })); } From 3ab086f8a6dec4c1cbaa9363331409ba00f90f23 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 03:45:42 +0200 Subject: [PATCH 02/14] Remove unused parameters --- webapp/src/app/home/home.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index 3c7f1702..5f719f83 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -28,7 +28,7 @@ export class HomeComponent { query = injectQuery(() => ({ queryKey: ['leaderboard'], - queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before, this.after, undefined, undefined, undefined)), + queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before, this.after)), gcTime: Infinity })); } From 833a34004520c1700eb977cd7eca56ac3644d1f4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 20 Sep 2024 01:53:26 +0000 Subject: [PATCH 03/14] chore: update API specs and client --- server/application-server/openapi.yaml | 16 ++++++++++------ .../core/modules/openapi/model/pull-request.ts | 2 +- .../src/app/core/modules/openapi/model/user.ts | 4 ++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/server/application-server/openapi.yaml b/server/application-server/openapi.yaml index 17a4aa9d..56eafb49 100644 --- a/server/application-server/openapi.yaml +++ b/server/application-server/openapi.yaml @@ -254,8 +254,9 @@ components: type: string state: type: string - description: "State of the PullRequest.\r\n Does not include the state of\ - \ the merge." + description: |- + State of the PullRequest. + Does not include the state of the merge. enum: - CLOSED - OPEN @@ -409,12 +410,15 @@ components: description: Display name of the user. url: type: string - description: "Unique URL to the user's profile.\r\n Not the website a user\ - \ can set in their profile." + description: |- + Unique URL to the user's profile. + Not the website a user can set in their profile. avatarUrl: type: string - description: "URL to the user's avatar.\r\n If unavailable, a fallback can\ - \ be generated from the login, e.g. on Github:\r\n https://github.com/{login}.png" + description: |- + URL to the user's avatar. + If unavailable, a fallback can be generated from the login, e.g. on Github: + https://github.com/{login}.png type: type: string description: Type of the user. Used to distinguish between users and bots. diff --git a/webapp/src/app/core/modules/openapi/model/pull-request.ts b/webapp/src/app/core/modules/openapi/model/pull-request.ts index 9de86033..f1fa06ad 100644 --- a/webapp/src/app/core/modules/openapi/model/pull-request.ts +++ b/webapp/src/app/core/modules/openapi/model/pull-request.ts @@ -23,7 +23,7 @@ export interface PullRequest { title: string; url: string; /** - * State of the PullRequest. Does not include the state of the merge. + * State of the PullRequest. Does not include the state of the merge. */ state: PullRequest.StateEnum; additions?: number; diff --git a/webapp/src/app/core/modules/openapi/model/user.ts b/webapp/src/app/core/modules/openapi/model/user.ts index 371ca15c..8e8311bd 100644 --- a/webapp/src/app/core/modules/openapi/model/user.ts +++ b/webapp/src/app/core/modules/openapi/model/user.ts @@ -29,11 +29,11 @@ export interface User { */ name?: string; /** - * Unique URL to the user\'s profile. Not the website a user can set in their profile. + * Unique URL to the user\'s profile. Not the website a user can set in their profile. */ url: string; /** - * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png + * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png */ avatarUrl?: string; /** From b83c893ce7b7b8d4784f286fd43bd4924a9e7ad7 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 11:16:06 +0200 Subject: [PATCH 04/14] Revert timeframe --- server/application-server/src/main/resources/application.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/application-server/src/main/resources/application.yml b/server/application-server/src/main/resources/application.yml index 324d01dd..aebef22e 100644 --- a/server/application-server/src/main/resources/application.yml +++ b/server/application-server/src/main/resources/application.yml @@ -33,7 +33,7 @@ springdoc: monitoring: runOnStartup: true # Fetching timeframe in days - timeframe: 1 + timeframe: 7 repository-sync-cron: "0 0 * * * *" repositories: ls1intum/Artemis From 171a58bbffc1ad3aebd861f9611bf3ab20600576 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 11:21:10 +0200 Subject: [PATCH 05/14] Add query key params for leaderboard --- webapp/src/app/home/home.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index 5f719f83..4d69bcbe 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -27,7 +27,7 @@ export class HomeComponent { } query = injectQuery(() => ({ - queryKey: ['leaderboard'], + queryKey: ['leaderboard', { after: this.after, before: this.before }], queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before, this.after)), gcTime: Infinity })); From ad71f1611fc661afe8654d67f11bd66a2a3427d9 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 11:59:29 +0200 Subject: [PATCH 06/14] Use signals for after and before --- webapp/src/app/home/home.component.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index 4d69bcbe..309f810f 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -1,9 +1,10 @@ -import { Component, inject } from '@angular/core'; +import { Component, computed, inject } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { injectQuery } from '@tanstack/angular-query-experimental'; import { LeaderboardService } from 'app/core/modules/openapi/api/leaderboard.service'; import { LeaderboardComponent } from 'app/home/leaderboard/leaderboard.component'; import { lastValueFrom } from 'rxjs'; +import { toSignal } from '@angular/core/rxjs-interop'; @Component({ selector: 'app-home', @@ -16,19 +17,14 @@ export class HomeComponent { // timeframe for leaderboard // example: 2024-09-19T10:15:30+01:00 - protected after: string | undefined = undefined; - protected before: string | undefined = undefined; - - constructor() { - inject(ActivatedRoute).queryParamMap.subscribe((params) => { - this.after = params.get('after')?.replace(' ', '+') ?? undefined; - this.before = params.get('before')?.replace(' ', '+') ?? undefined; - }); - } + private readonly route = inject(ActivatedRoute); + private queryParams = toSignal(this.route.queryParamMap, { requireSync: true }); + protected after = computed(() => this.queryParams().get('after')?.replace(' ', '+') ?? undefined); + protected before = computed(() => this.queryParams().get('before')?.replace(' ', '+') ?? undefined); query = injectQuery(() => ({ queryKey: ['leaderboard', { after: this.after, before: this.before }], - queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before, this.after)), + queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before(), this.after())), gcTime: Infinity })); } From dbf2b02eac7cd4a9efe6c84234b3a9df3854d50b Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 12:03:32 +0200 Subject: [PATCH 07/14] Reorder after and before --- server/application-server/openapi.yaml | 20 ++++++++----------- .../leaderboard/LeaderboardController.java | 6 +++--- .../leaderboard/LeaderboardService.java | 2 +- .../openapi/api/leaderboard.service.ts | 18 ++++++++--------- .../api/leaderboard.serviceInterface.ts | 4 ++-- .../modules/openapi/model/pull-request.ts | 2 +- .../app/core/modules/openapi/model/user.ts | 4 ++-- webapp/src/app/home/home.component.ts | 2 +- 8 files changed, 27 insertions(+), 31 deletions(-) diff --git a/server/application-server/openapi.yaml b/server/application-server/openapi.yaml index 56eafb49..2b7aa415 100644 --- a/server/application-server/openapi.yaml +++ b/server/application-server/openapi.yaml @@ -95,13 +95,13 @@ paths: - leaderboard operationId: getLeaderboard parameters: - - name: before + - name: after in: query required: false schema: type: string format: date-time - - name: after + - name: before in: query required: false schema: @@ -254,9 +254,8 @@ components: type: string state: type: string - description: |- - State of the PullRequest. - Does not include the state of the merge. + description: "State of the PullRequest.\r\n Does not include the state of\ + \ the merge." enum: - CLOSED - OPEN @@ -410,15 +409,12 @@ components: description: Display name of the user. url: type: string - description: |- - Unique URL to the user's profile. - Not the website a user can set in their profile. + description: "Unique URL to the user's profile.\r\n Not the website a user\ + \ can set in their profile." avatarUrl: type: string - description: |- - URL to the user's avatar. - If unavailable, a fallback can be generated from the login, e.g. on Github: - https://github.com/{login}.png + description: "URL to the user's avatar.\r\n If unavailable, a fallback can\ + \ be generated from the login, e.g. on Github:\r\n https://github.com/{login}.png" type: type: string description: Type of the user. Used to distinguish between users and bots. diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java index aafb9a8f..eb4ff149 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java @@ -21,8 +21,8 @@ public LeaderboardController(LeaderboardService leaderboardService) { } @GetMapping - public ResponseEntity> getLeaderboard(@RequestParam Optional before, - @RequestParam Optional after) { - return ResponseEntity.ok(leaderboardService.createLeaderboard(before, after)); + public ResponseEntity> getLeaderboard(@RequestParam Optional after, + @RequestParam Optional before) { + return ResponseEntity.ok(leaderboardService.createLeaderboard(after, before)); } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java index 3e1deabd..4698fafd 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java @@ -37,7 +37,7 @@ public LeaderboardService(UserService userService) { this.userService = userService; } - public List createLeaderboard(Optional before, Optional after) { + public List createLeaderboard(Optional after, Optional before) { logger.info("Creating leaderboard dataset"); List users = userService.getAllUsers(); diff --git a/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts b/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts index 4fc3001d..34a2c7cd 100644 --- a/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts +++ b/webapp/src/app/core/modules/openapi/api/leaderboard.service.ts @@ -96,25 +96,25 @@ export class LeaderboardService implements LeaderboardServiceInterface { } /** - * @param before * @param after + * @param before * @param observe set whether or not to return the data Observable as the body, response or events. defaults to returning the body. * @param reportProgress flag to report request and response progress. */ - public getLeaderboard(before?: string, after?: string, observe?: 'body', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>; - public getLeaderboard(before?: string, after?: string, observe?: 'response', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; - public getLeaderboard(before?: string, after?: string, observe?: 'events', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; - public getLeaderboard(before?: string, after?: string, observe: any = 'body', reportProgress: boolean = false, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable { + public getLeaderboard(after?: string, before?: string, observe?: 'body', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>; + public getLeaderboard(after?: string, before?: string, observe?: 'response', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; + public getLeaderboard(after?: string, before?: string, observe?: 'events', reportProgress?: boolean, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable>>; + public getLeaderboard(after?: string, before?: string, observe: any = 'body', reportProgress: boolean = false, options?: {httpHeaderAccept?: 'application/json', context?: HttpContext, transferCache?: boolean}): Observable { let localVarQueryParameters = new HttpParams({encoder: this.encoder}); - if (before !== undefined && before !== null) { - localVarQueryParameters = this.addToHttpParams(localVarQueryParameters, - before, 'before'); - } if (after !== undefined && after !== null) { localVarQueryParameters = this.addToHttpParams(localVarQueryParameters, after, 'after'); } + if (before !== undefined && before !== null) { + localVarQueryParameters = this.addToHttpParams(localVarQueryParameters, + before, 'before'); + } let localVarHeaders = this.defaultHeaders; diff --git a/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts b/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts index c41edbf2..19a131c7 100644 --- a/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts +++ b/webapp/src/app/core/modules/openapi/api/leaderboard.serviceInterface.ts @@ -27,9 +27,9 @@ export interface LeaderboardServiceInterface { /** * * - * @param before * @param after + * @param before */ - getLeaderboard(before?: string, after?: string, extraHttpRequestParams?: any): Observable>; + getLeaderboard(after?: string, before?: string, extraHttpRequestParams?: any): Observable>; } diff --git a/webapp/src/app/core/modules/openapi/model/pull-request.ts b/webapp/src/app/core/modules/openapi/model/pull-request.ts index f1fa06ad..9de86033 100644 --- a/webapp/src/app/core/modules/openapi/model/pull-request.ts +++ b/webapp/src/app/core/modules/openapi/model/pull-request.ts @@ -23,7 +23,7 @@ export interface PullRequest { title: string; url: string; /** - * State of the PullRequest. Does not include the state of the merge. + * State of the PullRequest. Does not include the state of the merge. */ state: PullRequest.StateEnum; additions?: number; diff --git a/webapp/src/app/core/modules/openapi/model/user.ts b/webapp/src/app/core/modules/openapi/model/user.ts index 8e8311bd..371ca15c 100644 --- a/webapp/src/app/core/modules/openapi/model/user.ts +++ b/webapp/src/app/core/modules/openapi/model/user.ts @@ -29,11 +29,11 @@ export interface User { */ name?: string; /** - * Unique URL to the user\'s profile. Not the website a user can set in their profile. + * Unique URL to the user\'s profile. Not the website a user can set in their profile. */ url: string; /** - * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png + * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png */ avatarUrl?: string; /** diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index 309f810f..b890dc72 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -24,7 +24,7 @@ export class HomeComponent { query = injectQuery(() => ({ queryKey: ['leaderboard', { after: this.after, before: this.before }], - queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.before(), this.after())), + queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.after(), this.before())), gcTime: Infinity })); } From cc94b30646006361afb9ac6132b919c726ab86d7 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 12:07:45 +0200 Subject: [PATCH 08/14] Switch after and before --- .../in/www1/hephaestus/leaderboard/LeaderboardService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java index 4698fafd..1c4de0d2 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java @@ -59,8 +59,8 @@ public List createLeaderboard(Optional after, Set commentSet = new HashSet<>(); user.getReviews().stream() - .filter(review -> isInTimeframe(review.getCreatedAt(), before, afterCutOff) - || isInTimeframe(review.getUpdatedAt(), before, afterCutOff)) + .filter(review -> isInTimeframe(review.getCreatedAt(), afterCutOff, before) + || isInTimeframe(review.getUpdatedAt(), afterCutOff, before)) .forEach(review -> { if (review.getPullRequest().getAuthor().getLogin().equals(user.getLogin())) { return; @@ -103,7 +103,7 @@ public List createLeaderboard(Optional after, return leaderboard; } - private boolean isInTimeframe(OffsetDateTime date, Optional before, OffsetDateTime after) { + private boolean isInTimeframe(OffsetDateTime date, OffsetDateTime after, Optional before) { return date != null && (before.isPresent() && date.isAfter(before.get()) || date.isBefore(after)); } From 81fedd106fa8aac36b7ca0faa09f5b30c45328a6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 20 Sep 2024 12:16:32 +0000 Subject: [PATCH 09/14] chore: update API specs and client --- server/application-server/openapi.yaml | 16 ++++++++++------ .../core/modules/openapi/model/pull-request.ts | 2 +- .../src/app/core/modules/openapi/model/user.ts | 4 ++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/server/application-server/openapi.yaml b/server/application-server/openapi.yaml index 2b7aa415..0c8f2307 100644 --- a/server/application-server/openapi.yaml +++ b/server/application-server/openapi.yaml @@ -254,8 +254,9 @@ components: type: string state: type: string - description: "State of the PullRequest.\r\n Does not include the state of\ - \ the merge." + description: |- + State of the PullRequest. + Does not include the state of the merge. enum: - CLOSED - OPEN @@ -409,12 +410,15 @@ components: description: Display name of the user. url: type: string - description: "Unique URL to the user's profile.\r\n Not the website a user\ - \ can set in their profile." + description: |- + Unique URL to the user's profile. + Not the website a user can set in their profile. avatarUrl: type: string - description: "URL to the user's avatar.\r\n If unavailable, a fallback can\ - \ be generated from the login, e.g. on Github:\r\n https://github.com/{login}.png" + description: |- + URL to the user's avatar. + If unavailable, a fallback can be generated from the login, e.g. on Github: + https://github.com/{login}.png type: type: string description: Type of the user. Used to distinguish between users and bots. diff --git a/webapp/src/app/core/modules/openapi/model/pull-request.ts b/webapp/src/app/core/modules/openapi/model/pull-request.ts index 9de86033..f1fa06ad 100644 --- a/webapp/src/app/core/modules/openapi/model/pull-request.ts +++ b/webapp/src/app/core/modules/openapi/model/pull-request.ts @@ -23,7 +23,7 @@ export interface PullRequest { title: string; url: string; /** - * State of the PullRequest. Does not include the state of the merge. + * State of the PullRequest. Does not include the state of the merge. */ state: PullRequest.StateEnum; additions?: number; diff --git a/webapp/src/app/core/modules/openapi/model/user.ts b/webapp/src/app/core/modules/openapi/model/user.ts index 371ca15c..8e8311bd 100644 --- a/webapp/src/app/core/modules/openapi/model/user.ts +++ b/webapp/src/app/core/modules/openapi/model/user.ts @@ -29,11 +29,11 @@ export interface User { */ name?: string; /** - * Unique URL to the user\'s profile. Not the website a user can set in their profile. + * Unique URL to the user\'s profile. Not the website a user can set in their profile. */ url: string; /** - * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png + * URL to the user\'s avatar. If unavailable, a fallback can be generated from the login, e.g. on Github: https://github.com/{login}.png */ avatarUrl?: string; /** From a62318d79aefd651394c33f8fd4af6c4645bd87f Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 23:55:48 +0200 Subject: [PATCH 10/14] Remove gcTime Infinity --- webapp/src/app/home/home.component.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index b890dc72..0f4bff47 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -24,7 +24,6 @@ export class HomeComponent { query = injectQuery(() => ({ queryKey: ['leaderboard', { after: this.after, before: this.before }], - queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.after(), this.before())), - gcTime: Infinity + queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.after(), this.before())) })); } From 9034afcba9eb8afbb87b5fba12df76a4bf1b3ca7 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 20 Sep 2024 23:56:47 +0200 Subject: [PATCH 11/14] Use signals for query key --- webapp/src/app/home/home.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index 0f4bff47..6f43fea2 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -23,7 +23,7 @@ export class HomeComponent { protected before = computed(() => this.queryParams().get('before')?.replace(' ', '+') ?? undefined); query = injectQuery(() => ({ - queryKey: ['leaderboard', { after: this.after, before: this.before }], + queryKey: ['leaderboard', { after: this.after(), before: this.before() }], queryFn: async () => lastValueFrom(this.leaderboardService.getLeaderboard(this.after(), this.before())) })); } From b76ed6dd13098a415d894cefe82d0188814a333e Mon Sep 17 00:00:00 2001 From: GODrums Date: Sun, 22 Sep 2024 01:50:23 +0200 Subject: [PATCH 12/14] Remove timestamp from leaderboard dates --- .../leaderboard/LeaderboardController.java | 8 ++++--- .../leaderboard/LeaderboardService.java | 22 ++++++++++--------- webapp/src/app/home/home.component.ts | 6 ++--- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java index eb4ff149..12d7d922 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardController.java @@ -1,9 +1,10 @@ package de.tum.in.www1.hephaestus.leaderboard; -import java.time.OffsetDateTime; +import java.time.LocalDate; import java.util.List; import java.util.Optional; +import org.springframework.format.annotation.DateTimeFormat; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; @@ -21,8 +22,9 @@ public LeaderboardController(LeaderboardService leaderboardService) { } @GetMapping - public ResponseEntity> getLeaderboard(@RequestParam Optional after, - @RequestParam Optional before) { + public ResponseEntity> getLeaderboard( + @RequestParam @DateTimeFormat(pattern = "yyyy-MM-dd") Optional after, + @RequestParam @DateTimeFormat(pattern = "yyyy-MM-dd") Optional before) { return ResponseEntity.ok(leaderboardService.createLeaderboard(after, before)); } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java index 1c4de0d2..852ac6b7 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java @@ -1,10 +1,11 @@ package de.tum.in.www1.hephaestus.leaderboard; +import java.time.LocalDate; +import java.time.LocalDateTime; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Comparator; -import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -37,17 +38,17 @@ public LeaderboardService(UserService userService) { this.userService = userService; } - public List createLeaderboard(Optional after, Optional before) { + public List createLeaderboard(Optional after, Optional before) { logger.info("Creating leaderboard dataset"); List users = userService.getAllUsers(); - OffsetDateTime afterCutOff = after.isPresent() ? after.get() - : new Date(System.currentTimeMillis() - 1000 * 60 * 60 * 24 * timeframe).toInstant() - .atOffset(ZoneOffset.UTC); + LocalDateTime afterCutOff = after.isPresent() ? after.get().atStartOfDay() + : LocalDate.now().minusDays(timeframe).atStartOfDay(); + Optional beforeCutOff = before.map(date -> date.plusDays(1).atStartOfDay()); logger.info("Leaderboard has " + users.size() + " users with cut-off time " + afterCutOff + " and before time " - + before); + + beforeCutOff); List leaderboard = users.stream().map(user -> { if (user.getType() != UserType.USER) { @@ -59,12 +60,12 @@ public List createLeaderboard(Optional after, Set commentSet = new HashSet<>(); user.getReviews().stream() - .filter(review -> isInTimeframe(review.getCreatedAt(), afterCutOff, before) - || isInTimeframe(review.getUpdatedAt(), afterCutOff, before)) + .filter(review -> isInTimeframe(review.getCreatedAt(), afterCutOff, beforeCutOff)) .forEach(review -> { if (review.getPullRequest().getAuthor().getLogin().equals(user.getLogin())) { return; } + PullRequestReviewDTO reviewDTO = new PullRequestReviewDTO(review.getId(), review.getCreatedAt(), review.getUpdatedAt(), review.getSubmittedAt(), review.getState()); @@ -103,8 +104,9 @@ public List createLeaderboard(Optional after, return leaderboard; } - private boolean isInTimeframe(OffsetDateTime date, OffsetDateTime after, Optional before) { - return date != null && (before.isPresent() && date.isAfter(before.get()) || date.isBefore(after)); + private boolean isInTimeframe(OffsetDateTime date, LocalDateTime after, Optional before) { + return date != null && (before.isPresent() && date.isBefore(before.get().atOffset(ZoneOffset.UTC)) + || date.isAfter(after.atOffset(ZoneOffset.UTC))); } /** diff --git a/webapp/src/app/home/home.component.ts b/webapp/src/app/home/home.component.ts index 6f43fea2..09a5efa3 100644 --- a/webapp/src/app/home/home.component.ts +++ b/webapp/src/app/home/home.component.ts @@ -16,11 +16,11 @@ export class HomeComponent { leaderboardService = inject(LeaderboardService); // timeframe for leaderboard - // example: 2024-09-19T10:15:30+01:00 + // example: 2024-09-19 private readonly route = inject(ActivatedRoute); private queryParams = toSignal(this.route.queryParamMap, { requireSync: true }); - protected after = computed(() => this.queryParams().get('after')?.replace(' ', '+') ?? undefined); - protected before = computed(() => this.queryParams().get('before')?.replace(' ', '+') ?? undefined); + protected after = computed(() => this.queryParams().get('after') ?? undefined); + protected before = computed(() => this.queryParams().get('before') ?? undefined); query = injectQuery(() => ({ queryKey: ['leaderboard', { after: this.after(), before: this.before() }], From d59215069ca8f70f883b90c6ea1545a5d0c1ef44 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 22 Sep 2024 02:12:46 +0000 Subject: [PATCH 13/14] chore: update API specs and client --- server/application-server/openapi.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/application-server/openapi.yaml b/server/application-server/openapi.yaml index 0c8f2307..a193b0d2 100644 --- a/server/application-server/openapi.yaml +++ b/server/application-server/openapi.yaml @@ -100,13 +100,13 @@ paths: required: false schema: type: string - format: date-time + format: date - name: before in: query required: false schema: type: string - format: date-time + format: date responses: "200": description: OK From 63c434d0306602b68c244fa63db202cfcff1dfba Mon Sep 17 00:00:00 2001 From: GODrums Date: Sun, 22 Sep 2024 04:41:36 +0200 Subject: [PATCH 14/14] Time filter directly in SQL --- .../codereview/user/UserRepository.java | 9 +++++++++ .../hephaestus/codereview/user/UserService.java | 6 ++++++ .../leaderboard/LeaderboardService.java | 15 +++++---------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java index ebba4e74..ba998f9d 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java @@ -1,5 +1,6 @@ package de.tum.in.www1.hephaestus.codereview.user; +import java.time.OffsetDateTime; import java.util.List; import java.util.Optional; @@ -39,4 +40,12 @@ SELECT new UserDTO(u.id, u.login, u.email, u.name, u.url) JOIN FETCH u.reviews """) List findAllWithRelations(); + + @Query(""" + SELECT u + FROM User u + JOIN FETCH u.reviews re + WHERE re.createdAt BETWEEN :after AND :before + """) + List findAllInTimeframe(@Param("after") OffsetDateTime after, @Param("before") OffsetDateTime before); } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java index 1913c8ef..1a5f3e9b 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java @@ -1,5 +1,6 @@ package de.tum.in.www1.hephaestus.codereview.user; +import java.time.OffsetDateTime; import java.util.List; import java.util.Optional; @@ -31,4 +32,9 @@ public List getAllUsers() { logger.info("Getting all users"); return userRepository.findAll().stream().toList(); } + + public List getAllUsersInTimeframe(OffsetDateTime after, OffsetDateTime before) { + logger.info("Getting all users in timeframe between " + after + " and " + before); + return userRepository.findAllInTimeframe(after, before); + } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java index 852ac6b7..fbb495f6 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java @@ -41,16 +41,17 @@ public LeaderboardService(UserService userService) { public List createLeaderboard(Optional after, Optional before) { logger.info("Creating leaderboard dataset"); - List users = userService.getAllUsers(); - LocalDateTime afterCutOff = after.isPresent() ? after.get().atStartOfDay() : LocalDate.now().minusDays(timeframe).atStartOfDay(); Optional beforeCutOff = before.map(date -> date.plusDays(1).atStartOfDay()); - logger.info("Leaderboard has " + users.size() + " users with cut-off time " + afterCutOff + " and before time " - + beforeCutOff); + List users = userService.getAllUsersInTimeframe(afterCutOff.atOffset(ZoneOffset.UTC), + beforeCutOff.map(b -> b.atOffset(ZoneOffset.UTC)).orElse(OffsetDateTime.now())); + + logger.info("Found " + users.size() + " users for the leaderboard"); List leaderboard = users.stream().map(user -> { + // ignore non-users, e.g. bots if (user.getType() != UserType.USER) { return null; } @@ -60,7 +61,6 @@ public List createLeaderboard(Optional after, Optio Set commentSet = new HashSet<>(); user.getReviews().stream() - .filter(review -> isInTimeframe(review.getCreatedAt(), afterCutOff, beforeCutOff)) .forEach(review -> { if (review.getPullRequest().getAuthor().getLogin().equals(user.getLogin())) { return; @@ -104,11 +104,6 @@ public List createLeaderboard(Optional after, Optio return leaderboard; } - private boolean isInTimeframe(OffsetDateTime date, LocalDateTime after, Optional before) { - return date != null && (before.isPresent() && date.isBefore(before.get().atOffset(ZoneOffset.UTC)) - || date.isAfter(after.atOffset(ZoneOffset.UTC))); - } - /** * Calculates the score for a given pull request. * Possible values: 1, 3, 7, 17, 33.