Remove admin guard in header (#329)

Author: iam-flo

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/BadPracticeDetectorScheduler.java

@@ -127,7 +127,7 @@ private void checkUserRoleAndScheduleDetectionAtTime(
                 scheduleDetectionAtTime(pullRequest, scheduledTime, sendBadPracticeDetectionEmail);
             }
         } catch (Exception e) {
-            logger.error("Error while checking user role: {}", e.getMessage());
+            logger.error("Error while checking user role: {}", e.toString());
         }
     }
 
@@ -148,13 +148,16 @@ private void scheduleDetectionAtTime(
 
         if (scheduledTasks.containsKey(pullRequest.getId())) {
             List<ScheduledFuture<?>> scheduledTasksList = scheduledTasks.get(pullRequest.getId());
+            List<ScheduledFuture<?>> tasksToRemove = new ArrayList<>();
             scheduledTasksList.forEach(task -> {
                 if (!task.isDone() && !task.isCancelled()) {
+                    logger.info("Cancelling previous task for pull request: {}", pullRequest.getId());
                     task.cancel(false);
                 } else {
-                    scheduledTasks.get(pullRequest.getId()).remove(task);
+                    tasksToRemove.add(task);
                 }
             });
+            scheduledTasksList.removeAll(tasksToRemove);
         } else {
             scheduledTasks.put(pullRequest.getId(), new ArrayList<>());
         }

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java

@@ -63,6 +63,8 @@ public DetectionResult detectAndSyncBadPractices(PullRequest pullRequest) {
         detectorRequest.setDescription(pullRequest.getBody());
         detectorRequest.setTitle(pullRequest.getTitle());
         detectorRequest.setLifecycleState(lifecycleState.getState());
+        detectorRequest.setRepositoryName(pullRequest.getRepository().getName());
+        detectorRequest.setPullRequestNumber(pullRequest.getNumber());
         if (pullRequest.getBadPracticeSummary() != null) {
             detectorRequest.setBadPracticeSummary(pullRequest.getBadPracticeSummary());
         } else {

server/application-server/src/main/java/de/tum/in/www1/hephaestus/intelligenceservice/model/DetectorRequest.java

@@ -36,6 +36,8 @@
   DetectorRequest.JSON_PROPERTY_BAD_PRACTICES,
   DetectorRequest.JSON_PROPERTY_DESCRIPTION,
   DetectorRequest.JSON_PROPERTY_LIFECYCLE_STATE,
+  DetectorRequest.JSON_PROPERTY_PULL_REQUEST_NUMBER,
+  DetectorRequest.JSON_PROPERTY_REPOSITORY_NAME,
   DetectorRequest.JSON_PROPERTY_TITLE
 })
 @jakarta.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.7.0")
@@ -52,6 +54,12 @@ public class DetectorRequest {
   public static final String JSON_PROPERTY_LIFECYCLE_STATE = "lifecycle_state";
   private String lifecycleState;
 
+  public static final String JSON_PROPERTY_PULL_REQUEST_NUMBER = "pull_request_number";
+  private Integer pullRequestNumber;
+
+  public static final String JSON_PROPERTY_REPOSITORY_NAME = "repository_name";
+  private String repositoryName;
+
   public static final String JSON_PROPERTY_TITLE = "title";
   private String title;
 
@@ -166,6 +174,56 @@ public void setLifecycleState(String lifecycleState) {
     this.lifecycleState = lifecycleState;
   }
 
+  public DetectorRequest pullRequestNumber(Integer pullRequestNumber) {
+    
+    this.pullRequestNumber = pullRequestNumber;
+    return this;
+  }
+
+  /**
+   * Get pullRequestNumber
+   * @return pullRequestNumber
+   */
+  @jakarta.annotation.Nonnull
+  @JsonProperty(JSON_PROPERTY_PULL_REQUEST_NUMBER)
+  @JsonInclude(value = JsonInclude.Include.ALWAYS)
+
+  public Integer getPullRequestNumber() {
+    return pullRequestNumber;
+  }
+
+
+  @JsonProperty(JSON_PROPERTY_PULL_REQUEST_NUMBER)
+  @JsonInclude(value = JsonInclude.Include.ALWAYS)
+  public void setPullRequestNumber(Integer pullRequestNumber) {
+    this.pullRequestNumber = pullRequestNumber;
+  }
+
+  public DetectorRequest repositoryName(String repositoryName) {
+    
+    this.repositoryName = repositoryName;
+    return this;
+  }
+
+  /**
+   * Get repositoryName
+   * @return repositoryName
+   */
+  @jakarta.annotation.Nonnull
+  @JsonProperty(JSON_PROPERTY_REPOSITORY_NAME)
+  @JsonInclude(value = JsonInclude.Include.ALWAYS)
+
+  public String getRepositoryName() {
+    return repositoryName;
+  }
+
+
+  @JsonProperty(JSON_PROPERTY_REPOSITORY_NAME)
+  @JsonInclude(value = JsonInclude.Include.ALWAYS)
+  public void setRepositoryName(String repositoryName) {
+    this.repositoryName = repositoryName;
+  }
+
   public DetectorRequest title(String title) {
 
     this.title = title;
@@ -204,12 +262,14 @@ public boolean equals(Object o) {
         Objects.equals(this.badPractices, detectorRequest.badPractices) &&
         Objects.equals(this.description, detectorRequest.description) &&
         Objects.equals(this.lifecycleState, detectorRequest.lifecycleState) &&
+        Objects.equals(this.pullRequestNumber, detectorRequest.pullRequestNumber) &&
+        Objects.equals(this.repositoryName, detectorRequest.repositoryName) &&
         Objects.equals(this.title, detectorRequest.title);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(badPracticeSummary, badPractices, description, lifecycleState, title);
+    return Objects.hash(badPracticeSummary, badPractices, description, lifecycleState, pullRequestNumber, repositoryName, title);
   }
 
   @Override
@@ -220,6 +280,8 @@ public String toString() {
     sb.append("    badPractices: ").append(toIndentedString(badPractices)).append("\n");
     sb.append("    description: ").append(toIndentedString(description)).append("\n");
     sb.append("    lifecycleState: ").append(toIndentedString(lifecycleState)).append("\n");
+    sb.append("    pullRequestNumber: ").append(toIndentedString(pullRequestNumber)).append("\n");
+    sb.append("    repositoryName: ").append(toIndentedString(repositoryName)).append("\n");
     sb.append("    title: ").append(toIndentedString(title)).append("\n");
     sb.append("}");
     return sb.toString();

server/application-server/src/main/resources/mail-templates/bad-practices-detected.html

@@ -9,10 +9,10 @@
         <span th:text="${badPractice.description}">Bad Practice Description</span>
     </li>
 </ul>
-You can review the bad practices on <a target="_blank" rel="noopener noreferrer nofollow" th:href="${config.clientHost + '/user/' + user.login + '/activity?pullRequest=' + pullRequest.id}">Hephaestus</a>.
+You can review the bad practices on <a target="_blank" rel="noopener noreferrer nofollow" th:href="${config.clientHost + '/best-practices'}">Hephaestus</a>.
 <br>
 <br>
-When you have resolved the bad practices, you can re-run the detection by clicking on the <strong>Detect bad practices</strong> button in the <a target="_blank" rel="noopener noreferrer nofollow" th:href="${config.clientHost + '/user/' + user.login + '/activity?pullRequest=' + pullRequest.id}">Activity</a> dashboard.
+When you have resolved the bad practices, you can re-run the detection by clicking on the <strong>Detect bad practices</strong> button in the <a target="_blank" rel="noopener noreferrer nofollow" th:href="${config.clientHost + '/best-practices'}">Best practices</a> dashboard.
 <hr />
 <div style="text-align: center;font-size: 10px">
     Manage your notification settings <a th:href="${config.clientHost + '/settings'}">here</a>

server/intelligence-service/app/detector/bad_practice_detector.py

@@ -54,7 +54,13 @@ class DetectionResult(BaseModel):
 
 @observe()
 def detect_bad_practices(
-    title, description, lifecycle_state, bad_practice_summary, bad_practices
+    title,
+    description,
+    lifecycle_state,
+    repository_name,
+    pull_request_number,
+    bad_practice_summary,
+    bad_practices,
 ) -> DetectionResult:
 
     callbacks: List[CallbackHandler] = []
@@ -78,6 +84,9 @@ def detect_bad_practices(
     structured_llm = model.with_structured_output(BadPracticeResult)
     response = structured_llm.invoke(prompt, config)
     trace_id = langfuse_context.get_current_trace_id() or ""
+    langfuse_context.update_current_trace(
+        tags=[repository_name, str(pull_request_number)]
+    )
     return DetectionResult(
         bad_practice_summary=response.bad_practice_summary,
         bad_practices=response.bad_practices,

server/intelligence-service/app/routers/detector.py

@@ -12,6 +12,8 @@ class DetectorRequest(BaseModel):
     title: str
     description: str
     lifecycle_state: str
+    repository_name: str
+    pull_request_number: int
     bad_practice_summary: str
     bad_practices: List[BadPractice]
 
@@ -32,6 +34,8 @@ def detect(request: DetectorRequest):
         request.title,
         request.description,
         request.lifecycle_state,
+        request.repository_name,
+        request.pull_request_number,
         request.bad_practice_summary,
         request.bad_practices,
     )

server/intelligence-service/openapi.yaml

@@ -47,13 +47,21 @@ components:
         lifecycle_state:
           title: Lifecycle State
           type: string
+        pull_request_number:
+          title: Pull Request Number
+          type: integer
+        repository_name:
+          title: Repository Name
+          type: string
         title:
           title: Title
           type: string
       required:
       - title
       - description
       - lifecycle_state
+      - repository_name
+      - pull_request_number
       - bad_practice_summary
       - bad_practices
       title: DetectorRequest

webapp/src/app/app.routes.ts

@@ -47,5 +47,6 @@ export const routes: Routes = [
     ]
   },
   { path: 'teams', component: SubteamsComponent, canActivate: [AuthGuard] },
-  { path: 'user/:id/activity', component: ActivityDashboardComponent, canActivate: [AuthGuard] }
+  { path: 'user/:id/best-practices', component: ActivityDashboardComponent, canActivate: [AuthGuard] },
+  { path: 'best-practices', component: ActivityDashboardComponent, canActivate: [AuthGuard] }
 ];

webapp/src/app/core/header/header.component.html

@@ -7,9 +7,11 @@
       </a>
       <span class="text-xs font-semibold mt-1 text-muted-foreground">{{ version }}</span>
     </div>
-    @if (signedIn() && user()?.roles?.includes('admin')) {
-      <a hlmBtn variant="link" routerLink="/workspace">Workspace</a>
-      <a hlmBtn variant="link" [routerLink]="'/user/' + user()!.username + '/activity'">Activity</a>
+    @if (signedIn()) {
+      @if (user()?.roles?.includes('admin')) {
+        <a hlmBtn variant="link" routerLink="/workspace">Workspace</a>
+      }
+      <a hlmBtn variant="link" routerLink="/best-practices">Best practices</a>
       <a hlmBtn variant="link" routerLink="/teams">Teams</a>
     }
   </div>

webapp/src/app/home/activity/activity-dashboard.component.ts

@@ -27,15 +27,15 @@ export class ActivityDashboardComponent {
 
   user = this.securityStore.loadedUser;
 
-  protected userLogin: string | null = null;
+  protected userLogin: string | undefined = undefined;
   protected openedPullRequestId: number | undefined = undefined;
 
   protected numberOfPullRequests = computed(() => this.query.data()?.pullRequests?.length ?? 0);
   protected numberOfBadPractices = computed(() => this.query.data()?.pullRequests?.reduce((acc, pr) => acc + (pr.badPractices?.length ?? 0), 0) ?? 0);
   protected currUserIsDashboardUser = computed(() => this.user()?.username === this.userLogin);
 
   constructor(private route: ActivatedRoute) {
-    this.userLogin = this.route.snapshot.paramMap.get('id');
+    this.userLogin = this.route.snapshot.paramMap.get('id') ?? this.user()?.username;
     this.openedPullRequestId = this.route.snapshot.queryParams['pullRequest'];
   }
 

webapp/src/app/user/pull-request-bad-practice-card/pull-request-bad-practice-card.component.ts

@@ -105,7 +105,7 @@ export class PullRequestBadPracticeCardComponent implements AfterViewInit {
     mutationFn: (prId: number) => lastValueFrom(this.activityService.detectBadPracticesForPullRequest(prId)),
     onSuccess: () => {
       this.queryClient.invalidateQueries({ queryKey: ['activity'] });
-      if (this.collapsibleTrigger.state() === 'closed') {
+      if (this.collapsibleTrigger != undefined && this.collapsibleTrigger.state() === 'closed') {
         this.collapsibleTrigger.toggleCollapsible();
       }
     },