-1

This is the home page of my app. It gets redirected to if someone tries to access a page when they aren't logged in.

import { Component, OnInit } from '@angular/core';
import { Router } from '@angular/router';
import { Observable } from 'rxjs/Observable';
import 'rxjs/Rx'; 

import { AuthenticationService } from '../_services/authentication.service';

@Component({
  selector: 'app-home',
  templateUrl: './home.component.html',
  styleUrls: ['./home.component.css']
})
export class HomeComponent implements OnInit {
  model: any = { username: "", password: "" };
  loading = false;
  error = '';
  isLoggedIn = false;
  public currentUser: any = null;
  constructor(private _auth: AuthenticationService, private router: Router) { }

  ngOnInit() {
    if(this._auth.isLoggedIn()) {
      this.isLoggedIn = true;
      this.router.navigate(['/census']);
    }
    else {

    }

    this.currentUser = this._auth.getCurrentUser().subscribe(result => {
      this.currentUser = result;
    });

    /*let timer = Observable.timer(29000, 29000);
    timer.subscribe(t => {

      //this.refresh();
    });*/
  }

  login() {
        this.loading = true;
        this._auth.login(this.model.username, this.model.password)
            .subscribe(result => {
                if (result === true) {
                    // login successful
                    this.isLoggedIn = true;
                    this.router.navigate(['/census']);
                } else {
                    // login failed
                    this.error = 'Username or password is incorrect';
                    this.loading = false;
                }
            });
    }

}

I have the same problem on any page I try redirecting to. The line that has

this.currentUser = this._auth.getCurrentUser().subscribe(result => {
  this.currentUser = result;
});

The error is

error_handler.js:54 EXCEPTION: Uncaught (in promise): Error: Error in :0:0 caused by: this._auth.getCurrentUser(...).subscribe is not a function

Gives me an error. It only gives an error if I redirected to the page using router.navigate(['/']);

I've tried navigateByUrl and get the same result. This is on the top of every page, and I get the error any time I redirect to the page.

This is the getCurrentUser method from the AuthenticationService

    getCurrentUser(): Observable<any> {
        if (this.currentUser == null) {
            let headers = new Headers({ 'Authorization': 'Bearer ' + localStorage.getItem('token') });
            let options = new RequestOptions({ headers: headers });
            return this.http.get(  this._api.apiUrl + 'users/0', options)
                .map((response: Response) => {
                    this.currentUser = response.json();
                    return this.currentUser;
                });
        }
        else {
            return this.currentUser;
        }
  }
Jhorra
  • 6,233
  • 21
  • 69
  • 123
  • you should import the authProvider in every component(or do it in some shared service). In every component you have to pass your auth object to the constructor to make it available in your (newly instantiated after a redirect) component – R_Ice Jul 04 '17 at 21:13
  • @Rienk I updated my question. I do import it on every page and include it in my constructor as shown above. – Jhorra Jul 04 '17 at 21:16
  • Please post the full component this is too little information. The only thing I dont understand is the dot (./) in your route target: this._router.navigate(['./encounter-billing/' + index]); – R_Ice Jul 04 '17 at 21:27
  • @Rienk I pasted the entire code from a shorter page. I have the same problem on any page that I redirect to. – Jhorra Jul 04 '17 at 21:33
  • `this._auth.getCurrentUser(...).subscribe is not a function` - that seems clear. What does your `getCurrentUser` function look like? –  Jul 04 '17 at 21:39
  • @hdk I added the code for that, but hear the distinction I'm making. That method works fine. If I have a href html redirect it works fine. Every time. It only doesn't work if I redirect programmatically using the router. – Jhorra Jul 04 '17 at 21:43
  • Friendly advice, if you want to prevent users from accessing routes of your app if they are not logged in. You should read and learn about Guards https://angular.io/guide/router#milestone-5-route-guards – Fabio Antunes Jul 04 '17 at 21:47
  • 1
    You're not returning an observable from `getCurrentUser` when `this.currentUser` is not `null`, only when it is `null`. –  Jul 04 '17 at 21:48
  • @hdk That's it! Since I'm redirecting it doesn't need to call it down again and doesn't return anything. If you post that as the answer I'll mark it accepted. – Jhorra Jul 04 '17 at 21:49
  • @FabioAntunes I am using authGuard on my routes, that was a poor choice of wording on my part. – Jhorra Jul 04 '17 at 21:49
  • @hdk I am returning something though if it's not null, I guess it's that I'm returning something that's not subscribable? How would I change my return to make it something subscribable? – Jhorra Jul 04 '17 at 21:51
  • 1
    Just change `return this.currentUser;` to `return Observable.of(this.currentUser);` –  Jul 04 '17 at 21:52
  • @hdk Thanks, if you post is as an answer I'll mark it accepted. – Jhorra Jul 04 '17 at 21:58

1 Answers1

1

Rather than returning the user object when it already exists, transform it with the Observable of operator to return an Observable.

Change return this.currentUser; to return Observable.of(this.currentUser); in the Authentication Service.

Also this is a bit off:

this.currentUser = this._auth.getCurrentUser().subscribe(result => {
  this.currentUser = result;
});

currentUser is being set to the Subscription of getCurrentUser() and then subsequently the result (subscribe() returns a Subscription).

  • How would you recommend I change it? – Jhorra Jul 04 '17 at 22:07
  • The subscription? The typical pattern is to save the subscription (in a separate property) and then unsubscribe in `ngOnDestroy` to ensure it's removed. –  Jul 04 '17 at 22:17
  • Could you point me to an example? I want to make sure I'm doing it right. – Jhorra Jul 04 '17 at 23:47
  • 1
    Something like this answer: https://stackoverflow.com/a/34926698/3244479 –  Jul 04 '17 at 23:52
  • This answer though goes into a lot of detail and suggests a newer approach: https://stackoverflow.com/a/41177163/3244479 –  Jul 04 '17 at 23:53