Merge pull request DeborahK#1 from SanderElias/refactor/bySander

I did refactor a couple of files. Didn't look at everything. When you have questions, don't hesitate to ask!

APM/src/app/product-categories/product-category.service.ts

@@ -1,37 +1,38 @@
-import { Injectable } from '@angular/core';
 import { HttpClient } from '@angular/common/http';
-
-import { Observable, throwError } from 'rxjs';
-import { catchError, tap } from 'rxjs/operators';
-
+import { Injectable } from '@angular/core';
+import { Observable, ReplaySubject, throwError } from 'rxjs';
+import { catchError, mergeMap, take, tap } from 'rxjs/operators';
 import { ProductCategory } from './product-category';
 
 @Injectable({
   providedIn: 'root'
 })
 export class ProductCategoryService {
+  private refresh = new ReplaySubject<void>();
   private productsUrl = 'api/productCategories';
 
   // All product categories
-  productCategories$: Observable<ProductCategory[]>;
-
-  constructor(private http: HttpClient) { }
+  productCategories$: Observable<ProductCategory[]> = this.refresh.pipe(
+    /** any xxxMap will do, merge is the safest. */
+    mergeMap(() => this.http.get<ProductCategory[]>(this.productsUrl)),
+    /** As its' not completing bcs of the subject, use take to make it behave as usual */
+    take(1),
+    tap({
+      next: data => console.log('getCategories', JSON.stringify(data)),
+      complete: () => console.log('competed request!')
+    }),
+    catchError(this.handleError)
+  );
+
+  constructor(private http: HttpClient) {}
 
   // Refresh the data.
   refreshData(): void {
-    this.start();
+    this.refresh.next();
   }
 
   start() {
-    this.productCategories$ = this.getCategories();
-  }
-
-  private getCategories(): Observable<ProductCategory[]> {
-    return this.http.get<ProductCategory[]>(this.productsUrl)
-      .pipe(
-        tap(data => console.log('getCategories', JSON.stringify(data))),
-        catchError(this.handleError)
-      );
+    this.refreshData();
   }
 
   private handleError(err) {

APM/src/app/products/product-detail/product-detail.component.ts

@@ -8,38 +8,39 @@ import { Product } from '../product';
 import { Supplier } from '../../suppliers/supplier';
 
 @Component({
-    selector: 'pm-product-detail',
-    templateUrl: './product-detail.component.html'
+  selector: 'pm-product-detail',
+  templateUrl: './product-detail.component.html'
 })
 export class ProductDetailComponent implements OnInit {
-    pageTitle = 'Product Detail';
-
-    selectedProductId$: Observable<number | null>;
-    product$: Observable<Product | null>;
-    suppliers$: Observable<Supplier[]>;
-    errorMessage: string;
-
-    constructor(private productService: ProductService) { }
-
-    ngOnInit() {
-        // Watch for changes to the selected product
-        // Get the selected product and display the appropriate heading
-        this.selectedProductId$ = this.productService.selectedProductChanges$
-            .pipe(
-                tap(() => {
-                    this.product$ = this.productService.selectedProduct$;
-                    this.suppliers$ = this.productService.selectedProductSuppliers$;
-                })
-            );
+  pageTitle = 'Product Detail';
+
+  selectedProductId$ = this.productService.selectedProductChanges$;
+  product$ = this.productService.selectedProduct$;
+  suppliers$ = this.productService.selectedProductSuppliers$;
+  errorMessage: string;
+
+  constructor(private productService: ProductService) {}
+
+  ngOnInit() {
+    // Watch for changes to the selected product
+    // Get the selected product and display the appropriate heading
+
+    /** no need for his, the observables in the service take care of it. */
+
+    // this.selectedProductId$ = this.productService.selectedProductChanges$.pipe(
+    //   tap(() => {
+    //     this.product$ = this.productService.selectedProduct$;
+    //     this.suppliers$ = this.productService.selectedProductSuppliers$;
+    //   })
+    // );
+  }
+
+  displayProduct(product: Product): void {
+    // Display the appropriate heading
+    if (product) {
+      this.pageTitle = `Product Detail for: ${product.productName}`;
+    } else {
+      this.pageTitle = 'No product found';
     }
-
-    displayProduct(product: Product): void {
-        // Display the appropriate heading
-        if (product) {
-            this.pageTitle = `Product Detail for: ${product.productName}`;
-        } else {
-            this.pageTitle = 'No product found';
-        }
-    }
-
+  }
 }

APM/src/app/products/product-list/product-list.component.ts

@@ -7,49 +7,41 @@ import { catchError, tap } from 'rxjs/operators';
 import { Product } from '../product';
 import { ProductService } from '../product.service';
 
-
 @Component({
   selector: 'pm-product-list',
   templateUrl: './product-list.component.html'
 })
 export class ProductListComponent implements OnInit {
   pageTitle = 'Products';
   errorMessage: string;
-  products$: Observable<Product[]>;
-  selectedProductId$: Observable<number | null>;
-
-  constructor(private route: ActivatedRoute,
+  products$ = this.productService.productsWithCategory$.pipe(
+    catchError(error => {
+      this.errorMessage = error;
+      return of(null);
+    })
+  );
+  selectedProductId$ = this.productService.selectedProductChanges$;
+
+  constructor(
+    private route: ActivatedRoute,
     private router: Router,
-    private productService: ProductService) { }
+    private productService: ProductService
+  ) {}
 
   ngOnInit(): void {
-
     // Read the parameter from the route - supports deep linking
-    this.route.paramMap.subscribe(
-      params => {
-        const id = +params.get('id');
-        this.productService.changeSelectedProduct(id);
-      }
-    );
-
-    this.selectedProductId$ = this.productService.selectedProductChanges$;
-
-    this.products$ = this.productService.productsWithCategory$
-      .pipe(
-        catchError(error => {
-          this.errorMessage = error;
-          return of(null);
-        })
-      );
+    this.route.paramMap.subscribe(params => {
+      const id = +params.get('id');
+      this.productService.changeSelectedProduct(id);
+    });
   }
 
   onRefresh(): void {
     this.productService.refreshData();
   }
 
   onSelected(productId: number): void {
-      // Modify the URL to support deep linking
+    // Modify the URL to support deep linking
     this.router.navigate(['/products', productId]);
   }
-
 }

APM/src/app/products/product.service.ts

@@ -1,48 +1,76 @@
-import { Injectable } from '@angular/core';
 import { HttpClient } from '@angular/common/http';
-
-import { Observable, throwError, forkJoin, from, BehaviorSubject } from 'rxjs';
-import { catchError, tap, map, mergeMap, toArray, shareReplay } from 'rxjs/operators';
-
-import { Product } from './product';
-import { Supplier } from '../suppliers/supplier';
+import { Injectable } from '@angular/core';
+import { combineLatest, forkJoin, Observable, ReplaySubject, throwError } from 'rxjs';
+import { catchError, map, mergeMap, shareReplay, take, tap } from 'rxjs/operators';
 import { ProductCategoryService } from '../product-categories/product-category.service';
 import { SupplierService } from '../suppliers/supplier.service';
+import { Product } from './product';
+
+
 
 @Injectable({
   providedIn: 'root'
 })
 export class ProductService {
+  private refresh = new ReplaySubject<void>();
+  private selectedProductSource = new ReplaySubject<number>();
   private productsUrl = 'api/products';
   private suppliersUrl = 'api/suppliers';
 
-  // Currently selected product
-  selectedProduct$: Observable<Product>;
-  selectedProductSuppliers$: Observable<Supplier[]>;
-  private selectedProductSource = new BehaviorSubject<number | null>(null);
   selectedProductChanges$ = this.selectedProductSource.asObservable();
 
   // All products
-  products$: Observable<Product[]>;
-  productsWithCategory$: Observable<Product[]>;
+  /** note, all the types are still there, I just don't need to type them out. */
+  products$ = this.refresh.pipe(
+    mergeMap(() => this.http.get<Product[]>(this.productsUrl)),
+    take(1),
+    tap(data => console.log('getProducts: ', JSON.stringify(data))),
+    catchError(this.handleError)
+  );
+
+  productsWithCategory$ = forkJoin([
+    this.products$,
+    this.productCategoryService.productCategories$
+  ]).pipe(
+    map(([products, categories]) =>
+      products.map(
+        p =>
+          ({
+            ...p,
+            category: categories.find(c => p.categoryId === c.id).name
+          } as Product) // <-- note the type here!
+      )
+    ),
+    shareReplay(1)
+  );
 
-  constructor(private http: HttpClient,
+  // Currently selected product
+  selectedProduct$ = combineLatest(
+    this.selectedProductChanges$,
+    this.productsWithCategory$
+  ).pipe(
+    map(([selectedProductId, products]) =>
+      products.find(product => product.id === selectedProductId)
+    ),
+    // Displays this message twice??
+    /** yes, one for each subscription. You might want to share() this. */
+    tap(product => console.log('changeSelectedProduct', product))
+  );
+
+  selectedProductSuppliers$ = this.selectedProduct$.pipe(
+    mergeMap(product =>
+      this.supplierService.getSuppliersByIds(product.supplierIds)
+    )
+  );
+
+  constructor(
+    private http: HttpClient,
     private productCategoryService: ProductCategoryService,
-    private supplierService: SupplierService) { }
+    private supplierService: SupplierService
+  ) {}
 
   // Change the selected product
   changeSelectedProduct(selectedProductId: number | null): void {
-    // This will only be set if it is bound via an async pipe
-    this.selectedProduct$ = this.productsWithCategory$.pipe(
-      map(products => products.find(product => product.id === selectedProductId)),
-      // Displays this message twice??
-      tap(product => console.log('changeSelectedProduct', product))
-    );
-    // This will only be set if it is bound via an async pipe
-    this.selectedProductSuppliers$ = this.selectedProduct$
-      .pipe(
-        mergeMap(product => this.supplierService.getSuppliersByIds(product.supplierIds))
-      );
     this.selectedProductSource.next(selectedProductId);
   }
 
@@ -66,44 +94,29 @@ export class ProductService {
 
   // Refresh the data.
   refreshData(): void {
+    console.log('init refresh')
     this.start();
   }
 
   start() {
     // Start the related services
     this.productCategoryService.start();
-
-    // All products
-    this.products$ = this.getProducts().pipe(
-      shareReplay(1)
-    );
-
-    // Products with categoryId foreign key mapped to category string
-    // [products, categories] uses destructuring to unpack the values from the arrays
-    this.productsWithCategory$ = forkJoin([this.products$, this.productCategoryService.productCategories$]).pipe(
-      map(([products, categories]) =>
-        products.map(p => ({ ...p, 'category': categories.find(c => p.categoryId === c.id).name }))
-      ),
-      shareReplay(1)
-    );
-  }
-
-  private getProducts(): Observable<Product[]> {
-    return this.http.get<Product[]>(this.productsUrl)
-      .pipe(
-        tap(data => console.log('getProducts: ', JSON.stringify(data))),
-        catchError(this.handleError)
-      );
+    this.refresh.next();
   }
 
   // Gets a single product by id
   private getProduct(id: number): Observable<Product> {
-    const url = `${this.productsUrl}/${id}`;
-    return this.http.get<Product>(url)
-      .pipe(
-        tap(data => console.log('getProduct: ', JSON.stringify(data))),
-        catchError(this.handleError)
-      );
+    // const url = `${this.productsUrl}/${id}`;
+    return this.products$.pipe(
+      /**
+       * this will load all products,
+       * perhaps keeping a single http call here is better in some cases.
+       * all depends on requirements. as an Observables sample I like this better ;)
+       */
+      map(productlist => productlist.find(row => row.id === id)),
+      tap(data => console.log('getProduct: ', JSON.stringify(data))),
+      catchError(this.handleError)
+    );
   }
 
   private handleError(err) {
@@ -121,5 +134,4 @@ export class ProductService {
     console.error(err);
     return throwError(errorMessage);
   }
-
 }