Merge pull request #101 from UMM-CSci-3601/angular-signals-from-michael

Incorporate Michael Small's improvements

client/cypress/e2e/add-user.cy.ts

@@ -92,8 +92,11 @@ describe('Add user', () => {
 
       page.addUser(user);
 
-      // New URL should end in the 24 hex character Mongo ID of the newly added user
-      cy.url()
+      // New URL should end in the 24 hex character Mongo ID of the newly added user.
+      // We'll wait up to 10 seconds for this these `should()` assertions to succeed.
+      // Hopefully that long timeout will help ensure that our Cypress tests pass in
+      // GitHub Actions, where we're often running on slow VMs.
+      cy.url({ timeout: 10000 })
         .should('match', /\/users\/[0-9a-fA-F]{24}$/)
         .should('not.match', /\/users\/new$/);
 
@@ -115,7 +118,7 @@ describe('Add user', () => {
         age: 30,
         company: null, // The company being set to null means nothing will be typed for it
         email: 'test@example.com',
-        role: 'editor'
+        role: 'editor',
       };
 
       page.addUser(user);

client/src/app/app.component.html

@@ -29,7 +29,7 @@
   <mat-sidenav-content>
     <mat-toolbar color="primary">
       <button type="button" aria-label="Toggle sidenav" mat-icon-button
-      (click)="drawer.toggle()" onclick="this.blur()" class="sidenav-button">
+      (click)="drawer.toggle()" onclick="blur()" class="sidenav-button">
         <mat-icon aria-label="Side nav toggle icon">menu</mat-icon>
       </button>
       <span class="app-title">{{title}}</span>

client/src/app/app.component.scss

@@ -6,11 +6,11 @@
   width: 256px;
 }
 
-.sidenav .mat-toolbar {
+.sidenav mat-toolbar {
   background: inherit;
 }
 
-.mat-toolbar.mat-primary {
+mat-toolbar.mat-primary {
   position: sticky;
   top: 0;
   z-index: 1;

client/src/app/company-card/company-card.component.html

@@ -1,14 +1,14 @@
-@if (this.company) {
+@if (company) {
 <mat-card>
   <mat-card-header>
-    <mat-card-title class="company-card-name">{{ this.company._id }}</mat-card-title>
+    <mat-card-title class="company-card-name">{{ company._id }}</mat-card-title>
     <mat-card-subtitle class="company-card-employee-count">
-      ({{ this.company.count }} {{ (this.company.count === 1) ? 'employee' : 'employees' }})
+      ({{ company.count }} {{ (company.count === 1) ? 'employee' : 'employees' }})
     </mat-card-subtitle>
   </mat-card-header>
   <mat-card-content>
     <ul class="company-card-users">
-      @for (user of this.company.users; track user._id) {
+      @for (user of company.users; track user._id) {
         <li><span class="company-card-user-name">{{ user.name }}</span></li>
       }
     </ul>

client/src/app/company-list/company-list.component.html

@@ -2,10 +2,10 @@
   <div class="flex-1">
     <h1 class="company-list-title">Companies</h1>
     <div class="flex-row">
-      @if (this.companies()) {
+      @if (companies()) {
         <div class="flex-1">
           <div class="company-cards-container flex-row gap-8 flex-wrap">
-            @for (company of this.companies(); track company._id) {
+            @for (company of companies(); track company._id) {
               <app-company-card class="company-card" [company]="company" ></app-company-card>
             }
           </div>

client/src/app/company-list/company-list.component.ts

@@ -1,4 +1,4 @@
-import { Component, Signal } from '@angular/core';
+import { Component, inject, Signal } from '@angular/core';
 import { toSignal } from '@angular/core/rxjs-interop';
 import { CompanyCardComponent } from '../company-card/company-card.component';
 import { UserService } from '../users/user.service';
@@ -12,9 +12,7 @@ import { Company } from './company';
   styleUrl: './company-list.component.scss'
 })
 export class CompanyListComponent {
-  companies: Signal<Company[]>;
+  private userService = inject(UserService);
 
-  constructor(private userService: UserService) {
-    this.companies = toSignal(this.userService.getCompanies());
-  }
+  companies: Signal<Company[]> = toSignal(this.userService.getCompanies());
 }

client/src/app/users/user-card.component.html

@@ -1,13 +1,13 @@
-@if (this.user) {
+@if (user()) {
   <mat-card class="user-card">
     <mat-card-header>
-      @if (this.user.avatar) {
-        <img mat-card-avatar [src]="this.user.avatar" alt="Avatar for {{this.user.name}}" class="user-avatar">
+      @if (user().avatar) {
+        <img mat-card-avatar [src]="user().avatar" alt="Avatar for {{user().name}}" class="user-avatar">
       }
-      <mat-card-title class="user-card-name">{{ this.user.name }}</mat-card-title>
-      <mat-card-subtitle class="user-card-company">{{ this.user.company }}</mat-card-subtitle>
+      <mat-card-title class="user-card-name">{{ user().name }}</mat-card-title>
+      <mat-card-subtitle class="user-card-company">{{ user().company }}</mat-card-subtitle>
     </mat-card-header>
-    @if (!this.simple) {
+    @if (!simple()) {
       <mat-card-content>
         <mat-list>
           <mat-list-item>
@@ -17,27 +17,27 @@
             We had to put both the `role` and `age` entries on a single line. Otherwise
             we got an annoying leading space in front of the values.
             -->
-            <p matListItemLine class="user-card-role">{{ this.user.role }}</p>
+            <p matListItemLine class="user-card-role">{{ user().role }}</p>
           </mat-list-item>
           <mat-divider [inset]=true></mat-divider>
           <mat-list-item>
             <mat-icon matListItemIcon>schedule</mat-icon>
             <p matListItemTitle>Age</p>
-            <p matListItemLine class="user-card-age">{{ this.user.age }}</p>
+            <p matListItemLine class="user-card-age">{{ user().age }}</p>
           </mat-list-item>
           <mat-divider [inset]=true></mat-divider>
           <mat-list-item>
-            <a href="mailto:{{this.user.email}}"  matListItemIcon><mat-icon>mail</mat-icon></a>
+            <a href="mailto:{{user().email}}"  matListItemIcon><mat-icon>mail</mat-icon></a>
             <p matListItemTitle>Email</p>
-            <a matListItemLine class="user-card-email" href="mailto:{{this.user.email}}">{{this.user.email}}</a>
-            <a href="mailto:{{this.user.email}}" matListItemMeta mat-icon-button><mat-icon>open_in_new</mat-icon></a>
+            <a matListItemLine class="user-card-email" href="mailto:{{user().email}}">{{user().email}}</a>
+            <a href="mailto:{{user().email}}" matListItemMeta mat-icon-button><mat-icon>open_in_new</mat-icon></a>
           </mat-list-item>
         </mat-list>
       </mat-card-content>
     }
-    @if (this.simple) {
+    @if (simple()) {
       <mat-card-actions>
-        <button mat-button data-test="viewProfileButton" [routerLink]="['/users', this.user._id]">View Profile</button>
+        <button mat-button data-test="viewProfileButton" [routerLink]="['/users', user()._id]">View Profile</button>
       </mat-card-actions>
     }
 </mat-card>

client/src/app/users/user-card.component.spec.ts

@@ -3,34 +3,33 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { UserCardComponent } from './user-card.component';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import { MatCardModule } from '@angular/material/card';
+import { input } from '@angular/core';
 
 describe('UserCardComponent', () => {
   let component: UserCardComponent;
   let fixture: ComponentFixture<UserCardComponent>;
 
   beforeEach(waitForAsync(() => {
     TestBed.configureTestingModule({
-    imports: [
-        BrowserAnimationsModule,
-        MatCardModule,
-        UserCardComponent
-    ]
-})
-    .compileComponents();
+      imports: [BrowserAnimationsModule, MatCardModule, UserCardComponent],
+    }).compileComponents();
   }));
 
   beforeEach(() => {
     fixture = TestBed.createComponent(UserCardComponent);
     component = fixture.componentInstance;
-    component.user = {
-      _id: 'chris_id',
-      name: 'Chris',
-      age: 25,
-      company: 'UMM',
-      email: 'chris@this.that',
-      role: 'admin',
-      avatar: 'https://gravatar.com/avatar/8c9616d6cc5de638ea6920fb5d65fc6c?d=identicon'
-    };
+    TestBed.runInInjectionContext(() => {
+      component.user = input({
+        _id: 'chris_id',
+        name: 'Chris',
+        age: 25,
+        company: 'UMM',
+        email: 'chris@this.that',
+        role: 'admin',
+        avatar:
+          'https://gravatar.com/avatar/8c9616d6cc5de638ea6920fb5d65fc6c?d=identicon',
+      });
+    });
     fixture.detectChanges();
   });
 

client/src/app/users/user-card.component.ts

@@ -1,4 +1,4 @@
-import { Component, Input } from '@angular/core';
+import { Component, input } from '@angular/core';
 import { User } from './user';
 import { RouterLink } from '@angular/router';
 import { MatButtonModule } from '@angular/material/button';
@@ -15,8 +15,6 @@ import { MatIconModule } from '@angular/material/icon';
     imports: [MatCardModule, MatButtonModule, MatListModule, MatIconModule, RouterLink]
 })
 export class UserCardComponent {
-
-  @Input() user: User;
-  @Input() simple?: boolean = false;
-
+  user = input.required<User>();
+  simple = input(false);
 }

client/src/app/users/user-list.component.html

@@ -13,14 +13,14 @@
           <mat-form-field class="input-field">
             <mat-label>Name</mat-label>
             <input matInput data-test="userNameInput" placeholder="Filter by name"
-              [(ngModel)]="userName" (input)="updateFilter()">
+              [(ngModel)]="userName">
             <mat-hint>Filtered on client</mat-hint>
           </mat-form-field>
 
           <mat-form-field class="input-field">
             <mat-label>Company</mat-label>
             <input matInput data-test="userCompanyInput" placeholder="Filter by company"
-              [(ngModel)]="userCompany" (input)="updateFilter()">
+              [(ngModel)]="userCompany">
             <mat-hint>Filtered on client</mat-hint>
           </mat-form-field>
         </div>
@@ -31,13 +31,13 @@
           <mat-form-field class="input-field">
             <mat-label>Age</mat-label>
             <input matInput data-test="userAgeInput" type="number" placeholder="Filter by age"
-              min="0" max="200" [(ngModel)]="userAge" (input)="getUsersFromServer()">
+              min="0" max="200" [(ngModel)]="userAge">
             <mat-hint>Filtered on server</mat-hint>
           </mat-form-field>
 
           <mat-form-field class="input-field">
             <mat-label>Role</mat-label>
-            <mat-select (selectionChange)="getUsersFromServer()" [(ngModel)]="userRole" data-test="userRoleSelect">
+            <mat-select [(ngModel)]="userRole" data-test="userRoleSelect">
               <mat-option>--</mat-option>
               <mat-option value="admin">Admin</mat-option>
               <mat-option value="editor">Editor</mat-option>
@@ -62,15 +62,15 @@
   </div>
 
   <div class="flex-row">
-    @if (serverFilteredUsers) {
+    @if (serverFilteredUsers()) {
       <div class="flex-1" >
         <!-- Switch between card and list view based on the viewType variable, set above in the mar-radio-group -->
         <div>
-          @switch (viewType) {
+          @switch (viewType()) {
             <!-- Card grid view -->
             @case ('card') {
               <div class="user-cards-container flex-row gap-8 flex-wrap">
-                @for (user of filteredUsers; track user._id) {
+                @for (user of filteredUsers(); track user._id) {
                   <app-user-card [simple]="true" class="user-card" [user]="user" ></app-user-card>
                 }
               </div>
@@ -81,9 +81,9 @@
                 <mat-card-content>
                   <mat-nav-list class="user-nav-list">
                     <h3 mat-subheader>Users</h3>
-                    @for (user of this.filteredUsers; track user._id) {
+                    @for (user of filteredUsers(); track user._id) {
                       <a mat-list-item [routerLink]="['/users', user._id]" class="user-list-item">
-                        @if (this.user.avatar) {
+                        @if (user.avatar) {
                           <img matListItemAvatar [src]="user.avatar" alt="Avatar for {{ user.name }}">
                         }
                         <span matListItemTitle mat-line class="user-list-name"> {{user.name}} </span>

client/src/app/users/user-list.component.scss

@@ -12,7 +12,7 @@
   margin-bottom: 10px;
 }
 
-.mat-radio-button {
+mat-radio-button {
   margin: 0 12px;
 }
 

client/src/app/users/user-list.component.spec.ts

@@ -51,7 +51,7 @@ describe('User list', () => {
     imports: [COMMON_IMPORTS, UserListComponent, UserCardComponent],
     // providers:    [ UserService ]  // NO! Don't provide the real service!
     // Provide a test-double instead
-    providers: [{ provide: UserService, useValue: new MockUserService() }]
+    providers: [{ provide: UserService, useValue: new MockUserService() }],
 });
   });
 
@@ -79,23 +79,23 @@ describe('User list', () => {
   }));
 
   it('contains all the users', () => {
-    expect(userList.serverFilteredUsers.length).toBe(3);
+    expect(userList.serverFilteredUsers().length).toBe(3);
   });
 
   it('contains a user named \'Chris\'', () => {
-    expect(userList.serverFilteredUsers.some((user: User) => user.name === 'Chris')).toBe(true);
+    expect(userList.serverFilteredUsers().some((user: User) => user.name === 'Chris')).toBe(true);
   });
 
   it('contain a user named \'Jamie\'', () => {
-    expect(userList.serverFilteredUsers.some((user: User) => user.name === 'Jamie')).toBe(true);
+    expect(userList.serverFilteredUsers().some((user: User) => user.name === 'Jamie')).toBe(true);
   });
 
   it('doesn\'t contain a user named \'Santa\'', () => {
-    expect(userList.serverFilteredUsers.some((user: User) => user.name === 'Santa')).toBe(false);
+    expect(userList.serverFilteredUsers().some((user: User) => user.name === 'Santa')).toBe(false);
   });
 
   it('has two users that are 37 years old', () => {
-    expect(userList.serverFilteredUsers.filter((user: User) => user.age === 37).length).toBe(2);
+    expect(userList.serverFilteredUsers().filter((user: User) => user.age === 37).length).toBe(2);
   });
 });
 
@@ -125,7 +125,7 @@ describe('Misbehaving User List', () => {
     imports: [COMMON_IMPORTS, UserListComponent],
     // providers:    [ UserService ]  // NO! Don't provide the real service!
     // Provide a test-double instead
-    providers: [{ provide: UserService, useValue: userServiceStub }]
+    providers: [{ provide: UserService, useValue: userServiceStub }],
 });
   });
 
@@ -139,22 +139,16 @@ describe('Misbehaving User List', () => {
     });
   }));
 
-  it('generates an error if we don\'t set up a UserListService', () => {
-    const mockedMethod = spyOn(userList, 'getUsersFromServer').and.callThrough();
-    // Since calling either getUsers() or getUsersFiltered() return
-    // Observables that then throw exceptions, we don't expect the component
-    // to be able to get a list of users, and serverFilteredUsers should
-    // be undefined.
-    expect(userList.serverFilteredUsers)
-      .withContext('service can\'t give values to the list if it\'s not there')
-      .toBeUndefined();
-    expect(userList.getUsersFromServer)
-      .withContext('will generate the right error if we try to getUsersFromServer')
-      .toThrow();
-    expect(mockedMethod)
-      .withContext('will be called')
-      .toHaveBeenCalled();
-    expect(userList.errMsg)
+  it("generates an error if we don't set up a UserListService", () => {
+    // If the service fails, we expect the `serverFilteredUsers` signal to
+    // be an empty array of users.
+    expect(userList.serverFilteredUsers())
+      .withContext("service can't give values to the list if it's not there")
+      .toEqual([]);
+    // We also expect the `errMsg` signal to contain the "Problem contacting…"
+    // error message. (It's arguably a bit fragile to expect something specific
+    // like this; maybe we just want to expect it to be non-empty?)
+    expect(userList.errMsg())
       .withContext('the error message will be')
       .toContain('Problem contacting the server – Error Code:');
       console.log(userList.errMsg);