Angular - Contact manager component with API

Question

I am starting with the Angular and I need some support. I have watched some of the tutorials and lot of them are using things differently. I upgraded some tutorial with my own idea, but I am not sure is this right way to do apps or not.

If you can take a look also at pagination and solution, not sure that is the best way to do it. What can I improve in code, what to focus on next?

Git link on full project Git repo

component.ts

import { Observable } from 'rxjs';
import { Router } from '@angular/router';
import { IContact } from './../../models/icontact';
import { Component, OnChanges, OnInit, SimpleChanges } from '@angular/core';
import { ContactService } from 'src/app/services/contact.service';

@Component({
  selector: 'app-contact-manager',
  templateUrl: './contact-manager.component.html',
  styleUrls: ['./contact-manager.component.scss'],
})
export class ContactManagerComponent implements OnInit {
  public loading: boolean = false;
  public contacts: IContact[] = [];
  public loadedContacts: IContact[] = [];
  public errorMessage: string | null = null;
  public contactsFiltered: IContact[] = [];
  public searchString: string = '';

  public currentPage = 1;
  public itemsPerPage = 6;
  public numberOfContacts = 0;
  public numberOfContactsInDB = 0;
  public totalPages = 0;
  public disablePrevous = true;
  public disableNext = true;

  public contacts$: Observable<IContact[]> = {} as Observable<IContact[]>;

  constructor(private contactService: ContactService, public router: Router) {}

  ngOnInit(): void {
    this.getContactsPaginated();
  }

  public getContactsPaginated() {
    this.contactService
      .getContactsPaginated(this.currentPage, this.itemsPerPage)
      .subscribe({
        next: (response) => {
          this.numberOfContactsInDB = +response.headers.get('X-Total-Count')!;
          this.contacts = response.body!;
          this.loadedContacts = response.body!;
          this.contactsFiltered = response.body!;
          this.calculatePages();
          this.changePageButton();
        },
      });
  }

  submitDeleteContact(contactId: string) {
    this.contactService.deleteContact(contactId).subscribe({
      next: (data) => {
        this.router.navigateByUrl('/');
        this.getContactsPaginated();
      },
      error: (error) => {
        this.errorMessage = error;
      },
    });
  }

  searchUser(event: KeyboardEvent) {
    if (this.searchString == '') {
      this.getContactsPaginated();
    } else {
      this.contactService
        .searchByName(this.searchString, this.currentPage, this.itemsPerPage)
        .subscribe((data) => {
          this.numberOfContactsInDB = +data.headers.get('X-Total-Count')!;
          this.contacts = data.body!;
          this.contactsFiltered = data.body!;
          this.loading = false;
          this.calculatePages();
          this.changePageButton();
        });
      this.changePage(1);
    }
  }

  changePage(page: number) {
    this.currentPage = page;
    this.changePageButton();
    this.calculatePages();
    this.contactService
      .searchByName(this.searchString, page, this.itemsPerPage)
      .subscribe((data) => {
        this.contacts = data.body!;
        this.contactsFiltered = data.body!;
        this.loading = false;
        this.calculatePages();
        this.changePageButton();
      });
  }

  changePageButton() {
    if (this.currentPage == 1) {
      this.disablePrevous = true;
    } else {
      this.disablePrevous = false;
    }

    if (this.currentPage == this.totalPages) {
      this.disableNext = true;
    } else {
      this.disableNext = false;
    }
  }

  calculatePages() {
    this.numberOfContacts = this.numberOfContactsInDB;
    this.totalPages = Math.ceil(this.numberOfContacts / this.itemsPerPage);
  }
}

service.ts

import { IContact } from './../models/icontact';
import {
  HttpClient,
  HttpErrorResponse,
  HttpHeaders,
  HttpResponse,
} from '@angular/common/http';
import { Injectable } from '@angular/core';
import { catchError, map, Observable, throwError } from 'rxjs';
import { IGroup } from '../models/igroup';

@Injectable({
  providedIn: 'root',
})
export class ContactService {
  private serverURL: string = 'http://localhost:9000';

  constructor(private httpClient: HttpClient) {}

  public getAllContacts(): Observable<IContact[]> {
    let dataURL: string = `${this.serverURL}/contacts`;
    return this.httpClient
      .get<IContact[]>(dataURL)
      .pipe(catchError(this.handleError));
  }

  public getContactsPaginated(page: number, limit: number) {
    let dataURL: string = `${this.serverURL}/contacts?_page=${page}&_limit=${limit}`;
    return this.httpClient
      .get<IContact[]>(dataURL, { observe: 'response' })
      .pipe(catchError(this.handleError));
  }

  public getContact(contactId: string): Observable<IContact> {
    let dataURL: string = `${this.serverURL}/contacts/${contactId}`;
    return this.httpClient
      .get<IContact>(dataURL)
      .pipe(catchError(this.handleError));
  }

  public createContact(contact: IContact): Observable<IContact> {
    let dataURL: string = `${this.serverURL}/contacts`;
    return this.httpClient
      .post<IContact>(dataURL, contact)
      .pipe(catchError(this.handleError));
  }

  public updateContact(
    contact: IContact,
    contactId: string
  ): Observable<IContact> {
    let dataURL: string = `${this.serverURL}/contacts/${contactId}`;
    return this.httpClient
      .put<IContact>(dataURL, contact)
      .pipe(catchError(this.handleError));
  }

  public deleteContact(contactId: string): Observable<{}> {
    let dataURL: string = `${this.serverURL}/contacts/${contactId}`;
    return this.httpClient
      .delete<{}>(dataURL)
      .pipe(catchError(this.handleError));
  }

  public getAllGroups(): Observable<IGroup[]> {
    let dataURL: string = `${this.serverURL}/groups`;
    return this.httpClient
      .get<IGroup[]>(dataURL)
      .pipe(catchError(this.handleError));
  }
  public getGroup(contact: IContact): Observable<IGroup> {
    let dataURL: string = `${this.serverURL}/groups/${contact.groupId}`;
    return this.httpClient
      .get<IGroup>(dataURL)
      .pipe(catchError(this.handleError));
  }

  public searchByName(searchName: string, page: number, limit: number) {
    let dataURL: string = `${this.serverURL}/contacts?name_like=${searchName}&_limit=6&_page=${page}&_limit=${limit}`;
    return this.httpClient
      .get<IContact[]>(dataURL, { observe: 'response' })
      .pipe(catchError(this.handleError));
  }

  public handleError(error: HttpErrorResponse) {
    let errorMessage: string = '';
    if (error.error instanceof ErrorEvent) {
      errorMessage = 'Client error ' + error.error.message;
    } else {
      errorMessage = 'Server error ' + error.error.message + ' ' + error.status;
    }
    return throwError(errorMessage);
  }

  getAllContactsObservableWay(): Observable<any> {
    return this.httpClient.get(this.serverURL).pipe();
  }
}

Answer 1

Overall your code already has a decent structure to it. Here are some of the more obvious things that I think could be improved, after taking a look at your project. Some are rather small things that you could easily fix, others are things you might want to adopt over time.

Use Angular Router

You shouldn't navigate by url within the angular application.

Change:

this.router.navigateByUrl('/');

To:

this.router.navigate(['..']); // <--- You can also use the route as defined in your app-routing.module.ts

And you will notice way faster routing using the angular router. Your approach will reload the whole app, which is defeating the purpose of a SPA.

Architecture

Your contact manager component handles both the data and presentation in a list. Try working in a container-presentation components approach. Once you have multiple features, you might also want them to have their own module so you can lazy load them.

Container Component

• Manages the state by interacting with service/store
• Contains only minor presentation, like for example a header/title
• Passes state to presentation components using input properties

Presentation Component

• Presents/renders the data, but does not directly interact with state
• Communicates with container component using output properties
• Use child routes to load them into a router-outlet in your container component

After that you might want to consider using the On-Push change detection strategy. Using the inputs for pushing the data to your presentation components will help a lot with getting ready for this. The input trigger the OnPush change detection.

Unsubscribing

Instead of subscribing to the data you get from your back end you could use the async pipe. Right now you are subscribing to your observables, but you never unsubscribe. You could do this in a couple of ways:

• Unsubscribe in ngOnDestroy

ngOnInit() {
    this.sub = this.someEventBus.on(
        Events.SomeEvent, (something => this.something = something))
}

ngOnDestroy() {
    if (this.sub) {
        this.sub.unsubscribe()
    }
}

• AutoUnsubscribe decorator

@AutoUnsubscribe()
export class SomeComponent implements OnDestroy {
    sub: Subscription

    constructor(private someEventBus: SomeEventBusService) { }

    ngOnInit() {
        this.sub = this.someEventBus.on(
            Events.SomeEvent, (something => this.something = something))
    }
}

• SubSink ( yarn add subsink)

subs = new SubSink()

constructor(private someEventBus: SomeEventBusService) { }

ngOnInit() {
    this.subs.sink = this.someEventBus.on(
        Events.SomeEvent, (something => this.something = something))
}

ngOnDestroy() {
    this.subs.unsubscribe()
}

• Use the takeUntil operator to complete observables instead of unsubscribing.

private stopFiring = new Subject()

someObservable$.pipe(
     // Do all the things...
     takeUntil(this.stopFiring)
);

// To complete the observable
this.stopFiring.next()

• Let async pipes handle subscribing / unsubscribing

Async pipe

I really recommend using these and let Angular handle the subscribing/unsubscribing. It takes a little getting used to, but I tend avoid manually subscribing altogether most of the time using this approach.

For example:

Change this:

public getContactsPaginated() {
    this.contactService
      .getContactsPaginated(this.currentPage, this.itemsPerPage)
      .subscribe({
        next: (response) => {
          this.numberOfContactsInDB = +response.headers.get('X-Total-Count')!;
          this.contacts = response.body!;
          this.loadedContacts = response.body!;
          this.contactsFiltered = response.body!;
          this.calculatePages();
          this.changePageButton();
        },
      });
  }

To something like this:

contactsPaginated$ = this.contactService
    .getContactsPaginated(this.currentPage, this.itemsPerPage)
    .pipe(
       tap(response => this.numberOfContactsInDB = +response.headers.get('X-Total-Count')!;)
    )

And in your template:

*ngFor="let contact of (contactsPaginated$ | async).contactsFiltered"

Or use some more rxjs operators to mold the response in the object you want first:

contactsPaginated$ = this.contactService
    .getContactsPaginated(this.currentPage, this.itemsPerPage)
    .pipe(
       tap(response => this.numberOfContactsInDB = +response.headers.get('X-Total-Count')!),
    .pluck('contactsFiltered'),
    .map(obj => /* Your mapping here... */)
    )

This also takes some getting used to, but the more you use it the more you will start to appreciate what you can do with RxJs without cluttering all over your component.

File naming

Naming convention for files holding your models looks like this: contact.model.ts

Styling

The styling of your app is rather simple but pretty, apart from some buttons hugging each other. However, all your styling is in the global styles.scss file. Angular uses encapsulation and you should want to make use of it. Style the presentation components in the stylesheet at the component level. Only actual global styles should go in the styles.scss file. Namely your font, something to normalize your styling and other base styles (like for example your company/house/angular-material default style imports).