Feature/code review suggestions

src/app/home/home.module.ts

@@ -1,8 +1,9 @@
-import { CommonModule } from "@angular/common";
+import { CommonModule, DatePipe } from "@angular/common";
 import { NgModule } from "@angular/core";
 import { CoreModule } from "../core/core.module";
 import { HomeComponent } from "./components/home/home.component";
 import { HomeRoutingModule } from "./home-routing.module";
+import { UnitsService } from "./services/units.service";
 
 @NgModule({
     declarations: [HomeComponent],
@@ -11,6 +12,6 @@ import { HomeRoutingModule } from "./home-routing.module";
         HomeRoutingModule,
         CoreModule
     ],
-    providers: [],
+    providers: [UnitsService, DatePipe],
 })
 export class HomeModule { }

src/app/home/people/components/add-meeting-modal/add-meeting-modal.component.html

@@ -13,6 +13,10 @@ <h3>Add Meeting Notes for {{ personData.name }}</h3>
             formControlName="comments"></textarea>
         </mat-form-field>
 
+        <div *ngIf="!commentsControl?.valid && (commentsControl?.dirty || commentsControl?.touched)">
+            <app-error-box *ngIf="commentsControl?.errors?.['validInput']" errorMessage="{{commentsControl?.errors?.['requiredValue']}}"></app-error-box>
+        </div>
+
         <mat-form-field appearance="fill">
             <mat-label>Questions</mat-label>
             <textarea matInput
@@ -104,5 +108,9 @@ <h3>Add Meeting Notes for {{ personData.name }}</h3>
         </mat-form-field>
 
         <button mat-raised-button [disabled]="!meetingForm.valid">Submit</button>
+
+        <div *ngIf="!meetingForm.valid && (meetingForm.dirty || meetingForm.touched)">
+            <app-error-box errorMessage="Please fill required data"></app-error-box>
+        </div>
     </form>
 </mat-dialog-content>

src/app/home/people/components/add-meeting-modal/add-meeting-modal.component.ts

@@ -6,6 +6,7 @@ import { YesNoToBooleanMapper } from 'src/app/shared/mappers/yes-no.mapper';
 import { AttritionRisk, MeetingNotes, YesNo } from '../../models/meeting-notes.model';
 import { PersonService } from '../../services/person.service';
 import * as moment from 'moment';
+import { inputValidator } from 'src/app/shared/validators/field.validator';
 
 @Component({
   selector: 'app-modal',
@@ -35,7 +36,7 @@ export class AddMeetingModalComponent implements OnDestroy {
     ) {
     this.meetingForm = this.formBuilder.group({
       date: ['', [Validators.required]],
-      comments: [''],
+      comments: ['', null, [inputValidator]],
       questions: [''],
       managerActionItems: [''],
       subordinateActionItems: [''],
@@ -62,6 +63,10 @@ export class AddMeetingModalComponent implements OnDestroy {
     return this.meetingForm.get('comments')!.value;
   }
 
+  get commentsControl() {
+    return this.meetingForm.get("comments");
+  }
+
   get questions() {
     return this.meetingForm.get('questions')!.value;
   }

src/app/home/people/components/edit-meeting/edit-meeting.component.ts

@@ -3,6 +3,10 @@ import { AttritionRisk, MeetingNotes } from '../../models/meeting-notes.model';
 import { FormBuilder, FormGroup } from '@angular/forms';
 import { YesNoToBooleanMapper } from 'src/app/shared/mappers/yes-no.mapper';
 import { Location } from '@angular/common';
+import { debounceTime, switchMap } from 'rxjs';
+import { Store } from '@ngrx/store';
+import { selectUserId } from 'src/app/login/store/login.selectors';
+import { UnitsService } from 'src/app/home/services/units.service';
 
 @Component({
   selector: 'app-edit-meeting',
@@ -12,6 +16,7 @@ import { Location } from '@angular/common';
 export class EditMeetingComponent implements OnInit {
 
   meetingData!: MeetingNotes;
+  notesId?: string;
   meetingForm!: FormGroup;
   cdkAutosizeMinRows = 1;
   cdkAutosizeMaxRows = 4;
@@ -25,10 +30,13 @@ export class EditMeetingComponent implements OnInit {
   constructor(
     private formBuilder: FormBuilder,
     private yesNoMapper: YesNoToBooleanMapper,
-    private location: Location) { }
+    private location: Location,
+    private store: Store,
+    private unitsService: UnitsService) { }
 
   ngOnInit(): void {
     this.meetingData = history.state;
+    this.notesId = this.meetingData.notesId;
     this.meetingForm = this.formBuilder.group({
       comments: [this.meetingData.comments],
       questions: [this.meetingData.questions],
@@ -42,6 +50,40 @@ export class EditMeetingComponent implements OnInit {
       attritionRisk: [this.meetingData.attritionRisk],
       oneToOneReportSent: [this.yesNoMapper.toModel(this.meetingData.oneToOneReportSent)]
     });
+
+    let userId: string | undefined;
+    this.store.select(selectUserId)
+      .subscribe(value => userId = value);
+
+    if (!userId) {
+      throw new Error('userId not found in Store');
+    }
+
+    // Below subscription to valueChanges event was implemented to exercise switchMap operator
+    // Behavior in UI: when user edits Comments form field, request is sent to fetch data from BE 
+    // and update Questions form field if no other Observable is emitted from valueChanges for 2 seconds
+    this.comments.valueChanges
+      .pipe(
+        debounceTime(2000),
+        switchMap(() => this.unitsService.getUnits(userId!))
+      )
+      .subscribe(units => {
+        const meetingData = units
+          .flatMap(unit => unit.people)
+          .flatMap(person => person.meetings)
+          .find(meeting => meeting.notesId === this.notesId)!;
+
+          this.questions.setValue(meetingData.questions);
+      }
+      );
+  }
+
+  get comments() {
+    return this.meetingForm.get('comments')!;
+  }
+
+  get questions() {
+    return this.meetingForm.get('questions')!;
   }
 
   onSubmit() {

src/app/home/people/components/people/people.component.html

@@ -1,5 +1,6 @@
+<button mat-raised-button color="primary" (click)="addNewUnit()">Add new unit</button>
 <ul class="list-group">
-    <li *ngFor="let unit of ($units | async)" class="list-group-item">
+    <li *ngFor="let unit of units" class="list-group-item">
         <app-unit [unitData]="unit"></app-unit>
     </li>
 </ul>

src/app/home/people/components/people/people.component.ts

@@ -1,10 +1,12 @@
 import { Component, OnDestroy, OnInit } from '@angular/core';
-import { Observable, Subscription, of } from 'rxjs';
+import { Subscription, of } from 'rxjs';
 import { Unit } from 'src/app/home/people/models/unit.model';
 import { ModalService } from '../../services/modal.service';
-import { UnitsService } from '../../services/units.service';
+
 import { Store } from '@ngrx/store';
 import { selectUserId } from 'src/app/login/store/login.selectors';
+import { UnitsService } from 'src/app/home/services/units.service';
+import { AttritionRisk } from '../../models/meeting-notes.model';
 
 @Component({
   selector: 'app-people',
@@ -13,14 +15,15 @@ import { selectUserId } from 'src/app/login/store/login.selectors';
 })
 export class PeopleComponent implements OnInit, OnDestroy {
 
-  $units!: Observable<Unit[]>;
+  $units!: Subscription;
+  units: Unit[] = [];
   $userId?: Subscription;
 
   constructor(
     private readonly unitsService: UnitsService,
     private modalService: ModalService,
     private store: Store
-    ) { }
+  ) { }
 
   ngOnInit(): void {
     let userId: string | undefined;
@@ -31,14 +34,61 @@ export class PeopleComponent implements OnInit, OnDestroy {
       throw new Error('userId not found in Store');
     }
 
-    this.$units = this.unitsService.getUnits(userId);
+    this.$units = this.unitsService.getUnits(userId)
+      .subscribe(units => this.units.push(...units));
     this.modalService.subscribe({
-      afterClosed: data => this.$units = userId ? this.unitsService.getUnits(userId) : of([])
+      afterClosed: data => {
+        userId && data ?
+          this.unitsService.getUnits(userId).subscribe(units => this.units = units) :
+          of([...this.units]).subscribe(units => this.units = units)
+      }
     })
   }
 
   ngOnDestroy(): void {
     this.$userId?.unsubscribe();
+    this.$units.unsubscribe();
+  }
+
+  // When method is invoked in the template, it creates a new array with old units and adds one new unit.
+  // UnitComponents which hold multiple PersonComponents have ChangeDetectionStrategy.OnPush
+  // When 'Add new Unit' button is clicked, change detection is run only for new component and not for existing components.
+  // So instead of running change detection multiple times, Angular runs it only once for better performance.
+  addNewUnit() {
+    this.units = [...this.units, this.newUnit];
   }
 
+  get newUnit() {
+    return {
+      id: `${Date.now()}`,
+      name: `Name ${Date.now()}`,
+      people: [
+        {
+          id: `${Date.now()}`,
+          name: `Person ${Date.now()}`,
+          grade: 'T9000',
+          specializations: ['Everything'],
+          location: 'everywhere',
+          meetings: [
+            {
+              date: new Date().toString(),
+              personId: '',
+              notesId: '',
+              comments: '',
+              questions: '',
+              managerActionItems: '',
+              subordinateActionItems: '',
+              importantAgreements: '',
+              satisfaction: '',
+              plans: '',
+              feedback: '',
+              issues: '',
+              attritionRisk: AttritionRisk.HIGH,
+              oneToOneReportSent: false
+            }
+          ]
+        }
+      ]
+    };
+  }
 }

src/app/home/people/components/person/person.component.html

@@ -1,5 +1,5 @@
 <div>
-    <p class="h4" tooltip [data]="personData">{{ personData.name }}</p>
+    <p class="h4" tooltip [data]="personData">{{ personData.name }} {{ runChangeDetection }}</p>
     <div>
         <ul class="list-inline">
             <li *ngFor="let page of pagination.pages; let i = index" class="list-inline-item">

src/app/home/people/components/person/person.component.ts

@@ -1,12 +1,13 @@
-import { Component, Input, OnInit } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core';
 import { AddMeetingModalComponent } from 'src/app/home/people/components/add-meeting-modal/add-meeting-modal.component';
 import { Person } from '../../models/unit.model';
 import { ModalService } from '../../services/modal.service';
 
 @Component({
   selector: 'app-person',
   templateUrl: './person.component.html',
-  styleUrls: ['./person.component.css']
+  styleUrls: ['./person.component.css'],
+  changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class PersonComponent implements OnInit {
 
@@ -25,6 +26,11 @@ export class PersonComponent implements OnInit {
 
   constructor(private modalService: ModalService) { }
 
+  get runChangeDetection() {
+    console.log('Person component - checking view');
+    return '';
+  }
+
   ngOnInit(): void {
     this.initializePagination();
   }

src/app/home/people/components/unit/unit.component.ts

@@ -1,10 +1,11 @@
-import { Component, Input, OnInit } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core';
 import { Unit } from 'src/app/home/people/models/unit.model';
 
 @Component({
   selector: 'app-unit',
   templateUrl: './unit.component.html',
-  styleUrls: ['./unit.component.css']
+  styleUrls: ['./unit.component.css'],
+  changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class UnitComponent implements OnInit {
 

src/app/home/people/people.module.ts

@@ -3,7 +3,6 @@ import { NgModule } from "@angular/core";
 import { PeopleComponent } from "./components/people/people.component";
 import { UnitComponent } from "./components/unit/unit.component";
 import { PeopleRoutingModule } from "./people-routing.module";
-import { UnitsService } from "./services/units.service";
 import { PersonComponent } from './components/person/person.component';
 import { MeetingComponent } from './components/meeting/meeting.component';
 import { NgMaterialModule } from "src/app/ng-material/ng-material.module";
@@ -32,6 +31,6 @@ import { EditMeetingComponent } from './components/edit-meeting/edit-meeting.com
         ReactiveFormsModule,
         SharedModule
     ],
-    providers: [UnitsService],
+    providers: [],
 })
 export class PeopleModule { }

src/app/home/people/services/units.service.ts → src/app/home/services/units.service.ts

@@ -1,13 +1,14 @@
 import { HttpClient } from '@angular/common/http';
 import { Injectable } from '@angular/core';
-import { Observable } from 'rxjs';
+import { Observable, ReplaySubject, of, switchMap } from 'rxjs';
 import { ConfigService } from 'src/app/core/services/config.service';
 import { Unit } from 'src/app/home/people/models/unit.model';
 
 @Injectable()
 export class UnitsService {
 
   private readonly apiBaseUrl;
+  private history$ = new ReplaySubject();
 
   constructor(
     private http: HttpClient,
@@ -17,6 +18,18 @@ export class UnitsService {
     }
 
   getUnits(userId: string): Observable<Unit[]> {
-    return this.http.get<Unit[]>(`${this.apiBaseUrl}/${userId}/units`);
+    return this.http.get<Unit[]>(`${this.apiBaseUrl}/${userId}/units`).pipe(
+      switchMap(units => {
+        this.history$.next(units);
+        return of(units);
+      })
+    );
+  }
+
+  // Method uses ReplaySubject which emits 'historical' values for new subscribers
+  // SummaryComponent uses it to display history of meeting entries count which is updated 
+  // after every request sent to /units endpoint
+  getHistoricalData(subscription: any) {
+    return this.history$.asObservable().subscribe(subscription);
   }
 }

src/app/home/summary/components/summary/summary.component.html

@@ -1 +1,12 @@
-<p>summary works!</p>
+<table class="table">
+    <thead class="thead-dark">
+        <tr>
+            <th scope="col">Number of meetings</th>
+        </tr>
+    </thead>
+    <tbody>
+        <tr *ngFor="let entry of history" scope="row">
+            <td>{{ entry.meetings.length }}</td>
+        </tr>
+    </tbody>
+</table>