Merge pull request #593 from puzzle/bug/578-scoring-styling

Bug/578 scoring styling

backend/src/main/java/ch/puzzle/okr/dto/overview/OverviewTeamDto.java

@@ -1,4 +1,4 @@
 package ch.puzzle.okr.dto.overview;
 
-public record OverviewTeamDto(Long id, String name, boolean writable) {
+public record OverviewTeamDto(Long id, String name, boolean writable, boolean hasInActiveOrganisations) {
 }

backend/src/main/java/ch/puzzle/okr/mapper/OverviewMapper.java

@@ -2,6 +2,7 @@
 
 import ch.puzzle.okr.dto.overview.*;
 import ch.puzzle.okr.models.overview.Overview;
+import ch.puzzle.okr.service.business.OrganisationBusinessService;
 import org.springframework.stereotype.Component;
 import org.springframework.web.server.ResponseStatusException;
 
@@ -16,6 +17,11 @@
 
 @Component
 public class OverviewMapper {
+    private final OrganisationBusinessService organisationBusinessService;
+
+    public OverviewMapper(OrganisationBusinessService organisationBusinessService) {
+        this.organisationBusinessService = organisationBusinessService;
+    }
 
     public List<OverviewDto> toDto(List<Overview> overviews) {
         List<OverviewDto> overviewDtos = new ArrayList<>();
@@ -60,8 +66,11 @@ private OverviewDto createOverviewDto(Overview overview) {
         if (isValidId(overview.getOverviewId().getObjectiveId())) {
             objectives.add(createObjectiveDto(overview));
         }
-        return new OverviewDto(new OverviewTeamDto(overview.getOverviewId().getTeamId(), overview.getTeamName(),
-                overview.isWriteable()), objectives);
+        return new OverviewDto(
+                new OverviewTeamDto(overview.getOverviewId().getTeamId(), overview.getTeamName(),
+                        overview.isWriteable(),
+                        organisationBusinessService.teamHasInActiveOrganisations(overview.getOverviewId().getTeamId())),
+                objectives);
     }
 
     private OverviewObjectiveDto createObjectiveDto(Overview overview) {

backend/src/main/java/ch/puzzle/okr/service/business/OrganisationBusinessService.java

@@ -54,6 +54,11 @@ public List<Organisation> getActiveOrganisations() {
         return persistenceService.getActiveOrganisations();
     }
 
+    public boolean teamHasInActiveOrganisations(Long teamId) {
+        return !getOrganisationsByTeam(teamId).stream()
+                .filter(organisation -> organisation.getState() == OrganisationState.INACTIVE).toList().isEmpty();
+    }
+
     public List<Organisation> getOrganisationsByTeam(Long teamId) {
         return persistenceService.getOrganisationsByTeamId(teamId);
     }

backend/src/test/java/ch/puzzle/okr/controller/OverviewControllerIT.java

@@ -5,6 +5,7 @@
 import ch.puzzle.okr.models.overview.Overview;
 import ch.puzzle.okr.models.overview.OverviewId;
 import ch.puzzle.okr.service.authorization.OverviewAuthorizationService;
+import ch.puzzle.okr.service.business.OrganisationBusinessService;
 import org.hamcrest.Matchers;
 import org.hamcrest.core.Is;
 import org.junit.jupiter.api.Test;
@@ -40,6 +41,8 @@ class OverviewControllerIT {
     private MockMvc mvc;
     @MockBean
     private OverviewAuthorizationService overviewAuthorizationService;
+    @MockBean
+    OrganisationBusinessService organisationBusinessService;
     @SpyBean
     private DashboardMapper dashboardMapper;
     @SpyBean

backend/src/test/java/ch/puzzle/okr/mapper/OverviewMapperTest.java

@@ -6,7 +6,12 @@
 import ch.puzzle.okr.dto.overview.OverviewKeyResultOrdinalDto;
 import ch.puzzle.okr.models.overview.Overview;
 import ch.puzzle.okr.models.overview.OverviewId;
+import ch.puzzle.okr.service.business.OrganisationBusinessService;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.web.server.ResponseStatusException;
 
 import java.util.List;
@@ -16,8 +21,13 @@
 import static org.junit.jupiter.api.Assertions.*;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;
 
+@ExtendWith(MockitoExtension.class)
 class OverviewMapperTest {
-    private final OverviewMapper overviewMapper = new OverviewMapper();
+    @Mock
+    private OrganisationBusinessService organisationBusinessService;
+
+    @InjectMocks
+    private OverviewMapper overviewMapper;
 
     @Test
     void toDtoShouldReturnEmptyListWhenNoTeamFound() {

backend/src/test/java/ch/puzzle/okr/service/business/OrganisationBusinessServiceTest.java

@@ -17,6 +17,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;
 
@@ -34,6 +36,9 @@ class OrganisationBusinessServiceTest {
     private static final Organisation organisationThree = Organisation.Builder.builder().withId(2L)
             .withOrgName("org_three").withState(OrganisationState.ACTIVE).build();
 
+    private static final Organisation organisationInActive = Organisation.Builder.builder().withId(2L)
+            .withOrgName("org_in_active").withState(OrganisationState.INACTIVE).build();
+
     @Mock
     LdapTemplate ldapTemplate;
 
@@ -78,4 +83,18 @@ void getOrganisationsByTeamId() {
         organisationBusinessService.getOrganisationsByTeam(1L);
         verify(organisationPersistenceService, times(1)).getOrganisationsByTeamId(1L);
     }
+
+    @Test
+    void teamHasInActiveOrganisations() {
+        when(organisationPersistenceService.getOrganisationsByTeamId(anyLong()))
+                .thenReturn(List.of(organisationInActive, organisationOne, organisationTwo));
+        assertTrue(organisationBusinessService.teamHasInActiveOrganisations(1L));
+    }
+
+    @Test
+    void teamHasNoInActiveOrganisations() {
+        when(organisationPersistenceService.getOrganisationsByTeamId(anyLong()))
+                .thenReturn(List.of(organisationOne, organisationTwo, organisationThree));
+        assertFalse(organisationBusinessService.teamHasInActiveOrganisations(1L));
+    }
 }

frontend/src/app/shared/custom/scoring/scoring.component.html

@@ -13,7 +13,7 @@
         </span>
       </div>
 
-      <div>
+      <div [ngStyle]="{ display: stretched ? 'none' : 'block' }">
         <span class="okr-score">
           <span class="child" #fail [attr.data-testId]="'fail'"></span>
         </span>
@@ -24,6 +24,7 @@
           <span class="child" #target [attr.data-testId]="'target'"></span>
         </span>
       </div>
+      <div [ngStyle]="{ display: stretched ? 'block' : 'none' }" class="stretch-div">&nbsp;</div>
     </div>
 
     <div class="okr-stretch d-flex flex-column align-items-center mt-1">

frontend/src/app/shared/custom/scoring/scoring.component.spec.ts

@@ -1,16 +1,10 @@
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { ScoringComponent } from './scoring.component';
-import {
-  keyResultMetricMinScoring,
-  keyResultMetricMinScoringInversion,
-  keyResultOrdinalMinScoring,
-} from '../../testData';
+import { keyResultMetricMinScoring, keyResultOrdinalMinScoring } from '../../testData';
 import { Router } from '@angular/router';
 import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { By } from '@angular/platform-browser';
 import { Zone } from '../../types/enums/Zone';
-import { KeyresultMin } from '../../types/model/KeyresultMin';
-import { KeyResultMetricMin } from '../../types/model/KeyResultMetricMin';
 
 describe('ScoringComponent', () => {
   let component: ScoringComponent;
@@ -42,7 +36,7 @@ describe('ScoringComponent', () => {
     });
 
     it('should fill out star if target percentage is over 100', () => {
-      component.targetPercent = 101;
+      component.stretched = true;
       component.ngAfterViewInit();
       expect(component.iconPath).toBe('filled');
     });
@@ -124,7 +118,6 @@ describe('ScoringComponent', () => {
       [{ zoneValue: Zone.FAIL, fail: 100, commit: 0, target: 0 }],
       [{ zoneValue: Zone.COMMIT, fail: 100, commit: 100, target: 0 }],
       [{ zoneValue: Zone.TARGET, fail: 100, commit: 100, target: 100 }],
-      [{ zoneValue: Zone.STRETCH, fail: 100, commit: 100, target: 101 }],
     ])('should set percentages correctly', (object: any) => {
       //Reset component
       component.targetPercent = 0;

frontend/src/app/shared/custom/scoring/scoring.component.ts

@@ -30,6 +30,7 @@ export class ScoringComponent implements OnInit, AfterViewInit, OnChanges {
   commitPercent: number = 0;
   targetPercent: number = 0;
   labelPercentage: Observable<number>;
+  stretched: boolean = false;
 
   @ViewChild('fail')
   private failElement: ElementRef<HTMLSpanElement> | undefined = undefined;
@@ -55,21 +56,22 @@ export class ScoringComponent implements OnInit, AfterViewInit, OnChanges {
   calculatePercentageOrdinal() {
     switch (this.keyResult.lastCheckIn?.value) {
       case Zone.STRETCH:
-        this.failPercent = 100;
-        this.commitPercent = 100;
-        this.targetPercent = 101;
+        this.stretched = true;
         break;
       case Zone.TARGET:
         this.failPercent = 100;
         this.commitPercent = 100;
         this.targetPercent = 100;
+        this.stretched = false;
         break;
       case Zone.COMMIT:
         this.failPercent = 100;
         this.commitPercent = 100;
+        this.stretched = false;
         break;
       case Zone.FAIL:
         this.failPercent = 100;
+        this.stretched = false;
         break;
     }
   }
@@ -82,20 +84,21 @@ export class ScoringComponent implements OnInit, AfterViewInit, OnChanges {
       this.labelPercentage = of(percentage);
       switch (true) {
         case percentage >= 100:
-          this.failPercent = 100;
-          this.commitPercent = 100;
-          this.targetPercent = 101;
+          this.stretched = true;
           break;
         case percentage > 70:
+          this.stretched = false;
           this.failPercent = 100;
           this.commitPercent = 100;
           this.targetPercent = (100 / 30) * (percentage - 70);
           break;
         case percentage > 30:
+          this.stretched = false;
           this.failPercent = 100;
           this.commitPercent = (100 / 40) * (percentage - 30);
           break;
         default:
+          this.stretched = false;
           this.failPercent = (100 / 30) * percentage;
       }
     }
@@ -106,19 +109,25 @@ export class ScoringComponent implements OnInit, AfterViewInit, OnChanges {
       case this.targetPercent > 100:
         return 'score-stretch';
       case this.targetPercent > 0:
-        this.targetElement!.nativeElement.classList.add('border-right');
+        this.setBorder(this.targetElement!);
         return 'score-green';
       case this.commitPercent > 0:
-        this.commitElement!.nativeElement.classList.add('border-right');
+        this.setBorder(this.commitElement!);
         return 'score-yellow';
       case this.failPercent > 0:
-        this.failElement!.nativeElement.classList.add('border-right');
+        this.setBorder(this.failElement!);
         return 'score-red';
       default:
         return null;
     }
   }
 
+  setBorder(element: ElementRef<HTMLSpanElement>) {
+    if (this.keyResult.keyResultType != 'ordinal') {
+      element.nativeElement.classList.add('border-right');
+    }
+  }
+
   ngAfterViewInit(): void {
     //Define width of scoring elements
     this.failElement!.nativeElement.style.width = this.failPercent + '%';
@@ -141,7 +150,7 @@ export class ScoringComponent implements OnInit, AfterViewInit, OnChanges {
     }
 
     //Fill out icon if target percent has reached 100 percent or more
-    if (this.targetPercent > 100) {
+    if (this.stretched) {
       this.iconPath = 'filled';
       this.changeDetectionRef.detectChanges();
     }

frontend/src/app/shared/services/organisation.service.ts

@@ -2,7 +2,6 @@ import { Injectable } from '@angular/core';
 import { HttpClient } from '@angular/common/http';
 import { Organisation } from '../types/model/Organisation';
 import { Observable } from 'rxjs';
-import { optionalValue } from '../common';
 
 @Injectable({
   providedIn: 'root',

frontend/src/app/shared/types/model/TeamMin.ts

@@ -2,4 +2,5 @@ export interface TeamMin {
   id: number;
   name: string;
   writable: boolean;
+  hasInActiveOrganisations: boolean;
 }

frontend/src/app/team/team.component.html

@@ -11,7 +11,7 @@
     />
     <div *ngIf="this.hasAdminAccess | async" class="d-flex flex-row align-items-center">
       <mat-icon
-        *ngIf="this.hasInActiveOrganisation | async"
+        *ngIf="OVEntity.team.hasInActiveOrganisations"
         class="team-actions cursor-pointer ms-2 text-danger"
         [matTooltip]="'This team has inactive organisations'"
         >error_outline</mat-icon

frontend/src/app/team/team.component.spec.ts

@@ -82,26 +82,10 @@ describe('TeamComponent', () => {
     expect(button).toBeFalsy();
   });
 
-  it('should set has inactive teams to false', async () => {
-    jest
-      .spyOn(organisationServiceMock, 'getOrganisationsByTeamId')
-      .mockReturnValue(of([organisationActive, organisationActive]));
-    component.checkIfTeamHasInActiveOrganisations();
-    expect(component.hasInActiveOrganisation.value).toBeFalsy();
-  });
-
   it('should open Teamdialog and make call to dialog object', async () => {
     jest.spyOn(dialogMock, 'open').mockReturnValue({ afterClosed: () => of(team1) });
     component.openEditTeamDialog(teamMin1);
     expect(dialogMock.open).toHaveBeenCalled();
     expect(refreshDataServiceMock.markDataRefresh).toHaveBeenCalled();
   });
-
-  it('should set has inactive teams to true', async () => {
-    jest
-      .spyOn(organisationServiceMock, 'getOrganisationsByTeamId')
-      .mockReturnValue(of([organisationInActive, organisationInActive]));
-    component.checkIfTeamHasInActiveOrganisations();
-    expect(component.hasInActiveOrganisation.value).toBeTruthy();
-  });
 });

frontend/src/app/team/team.component.ts

@@ -1,4 +1,4 @@
-import { AfterViewInit, ChangeDetectionStrategy, Component, Input } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
 import { OverviewEntity } from '../shared/types/model/OverviewEntity';
 import { MatDialog } from '@angular/material/dialog';
 import { ObjectiveFormComponent } from '../shared/dialog/objective-dialog/objective-form.component';
@@ -9,49 +9,25 @@ import { KeyResultDialogComponent } from '../shared/dialog/key-result-dialog/key
 import { trackByFn } from '../shared/common';
 import { TeamManagementComponent } from '../shared/dialog/team-management/team-management.component';
 import { TeamMin } from '../shared/types/model/TeamMin';
-import { OrganisationService } from '../shared/services/organisation.service';
-import { OrganisationState } from '../shared/types/enums/OrganisationState';
 
 @Component({
   selector: 'app-team',
   templateUrl: './team.component.html',
   styleUrls: ['./team.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class TeamComponent implements AfterViewInit {
+export class TeamComponent {
   private overviewEntity$ = new BehaviorSubject<OverviewEntity>({} as OverviewEntity);
   protected readonly trackByFn = trackByFn;
 
   @Input()
   hasAdminAccess!: ReplaySubject<boolean>;
-  hasInActiveOrganisation = new BehaviorSubject<boolean>(false);
 
   constructor(
     private dialog: MatDialog,
     private refreshDataService: RefreshDataService,
-    private organisationService: OrganisationService,
   ) {}
 
-  ngAfterViewInit(): void {
-    this.hasAdminAccess.subscribe((result) => {
-      if (result) {
-        this.checkIfTeamHasInActiveOrganisations();
-      }
-    });
-  }
-
-  checkIfTeamHasInActiveOrganisations() {
-    this.overviewEntity$.subscribe((result) => {
-      if (result.team) {
-        this.organisationService.getOrganisationsByTeamId(result.team.id).subscribe((organisations) => {
-          this.hasInActiveOrganisation.next(
-            organisations.filter((organisation) => organisation.state != OrganisationState.ACTIVE).length > 0,
-          );
-        });
-      }
-    });
-  }
-
   @Input()
   get overviewEntity(): BehaviorSubject<OverviewEntity> {
     return this.overviewEntity$;