Calling Service Method Getting Property Does Not Exists

I have created a service and am trying call the service from a method in a page. I am not sure what I have defined wrong and would appreciate any insight. The statement:

this.userService.login(this.loginForm.username, this.loginForm.password)

Is where the error is at. It indicates that there is no property type on userService for login… not sure why the method is not accessible.

Here is the Typescript for the Page:

import {Page, NavController, Platform} from 'ionic-angular';
import {FormBuilder, Validators} from '@angular/common';
import {HomePage} from '../../pages/home/home';
import {UserService} from '../../providers/user-service';

/*
  Generated class for the LoginPage page.

  See http://ionicframework.com/docs/v2/components/#navigation for more info on
  Ionic pages and navigation.
*/
@Page({
  templateUrl: 'build/pages/login/login.html',
})
export class LoginPage {
  loginForm: any;
  jsonData: any;
  un:string;
  pw:string;

  constructor(public nav: NavController, public formBuilder:FormBuilder, public userService:UserService) {
    //this.userService = userService;
    this.loginForm = formBuilder.group({
      username: ['',Validators.required],
      password: ['',Validators.required]
    })
  }

  login(event): void {

    this.userService.login(this.loginForm.username, this.loginForm.password)
      .then(data => {
        this.jsonData = data;
      });

    // Prevent the default action
    event.preventDefault();
    // Call teh backend service to log the user in


    this.nav.setRoot(HomePage);

  }
}

Here is the Service

import {Injectable} from '@angular/core';
import {Http} from '@angular/http';
import {Storage, SqlStorage} from 'ionic-angular';
import 'rxjs/add/operator/map';

/*
  Generated class for the UserService provider.

  See https://angular.io/docs/ts/latest/guide/dependency-injection.html
  for more info on providers and Angular 2 DI.
*/
@Injectable()
export class UserService {
  data: any = null;

  constructor(public http: Http, public storage:Storage) {

    // Define the storage
    this.storage = new Storage(SqlStorage);

    // Create the table if it does not exist
    this.storage.query("CREATE TABLE IF NOT EXISTS User (userId PRIMARY KEY, roleId INTEGER, themeId INTEGER, firstName TEXT, lastName TEXT, emailAddress TEXT, password TEXT, active STRING, approver INTEGER, salesOffice TEXT, reportsToId INTEGER, lastLoginDate STRING, createDate STRING, lastUpdate STRING)")
      .then((data) => {console.log('USER TABLE CREATED  ->' + JSON.stringify(data.res)); },
      (error) => {console.log('Error ->' + JSON.stringify(error.err));
    });

  }

  save(user) {
    // Create the insert statement
    let sql = 'INSERT INTO User (userId, roleId, themeId, firstName, lastName, emailAddress, password, active, approver, salesOffice, reportsToId, lastLoginDate, createDate, lastUpdate) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?)';

    // return the promise
    return this.storage.query(sql,[user.userId,user.roleId,user.themeId, user.firstName, user.lastName, user.emailAddress, user.password, user.active, user.approver, user.salesOffice, user.reportsToId, user.lastLoginDate, user.createDate, user.lastUpdate]);
  }

  find() {

    // Should only be one user to get
    let sql = 'SELECT * FROM User';
    return this.storage.query(sql);

  }

  login(username:string, password:string) {

    return new Promise(resolve => {
      // We're using Angular Http provider to request the data,
      // then on the response it'll map the JSON data to a parsed JS object.
      // Next we process the data and resolve the promise with the new data.
      this.http.get('http://localhost:8080/mobile/api/login?username=' + username + '&password=' + password)
        .map(res => res.json())
        .subscribe(data => {
          // we've got back the raw data, now generate the core schedule data
          // and save the data for later reference
          this.data = data;
          resolve(this.data);
        });
    });

  }

  load() {
    if (this.data) {
      // already loaded data
      return Promise.resolve(this.data);
    }

    // don't have the data yet
    return new Promise(resolve => {
      // We're using Angular Http provider to request the data,
      // then on the response it'll map the JSON data to a parsed JS object.
      // Next we process the data and resolve the promise with the new data.
      this.http.get('path/to/data.json')
        .map(res => res.json())
        .subscribe(data => {
          // we've got back the raw data, now generate the core schedule data
          // and save the data for later reference
          this.data = data;
          resolve(this.data);
        });
    });
  }
}

I haven’t tested it but looking at the code it seems that you’re trying to pass undefined variables of type Control to a method that expects parameters of type string, i.e. the method signature doesn’t match and therefore the error - try to update your code this way:

this.userService.login(
    this.loginForm.controls.username.value,
    this.loginForm.controls.password.value
);

EDIT: Updated the comment according to the remarks of @rapropos.

I’m sort of surprised that the error isn’t on this.loginForm.username and this.loginForm.password. I thought those had to hang off of controls, like this.loginForm.controls.username.

As a side note, your service methods use the explicit promise constructor antipattern. I see this particular boilerplate a lot on these forums, so I assume that it comes from some sort of generator. If anybody knows exactly which driftyco project it comes from, I’ll open an issue about it. Bottom line: it probably makes the most sense to just return the underlying Observable. If you really really want a Promise instead, use Observable.toPromise instead of creating a Promise from scratch.

1 Like

Yes, you’re right, they should be referenced through the controls property. I updated my comment above accordingly. It’s not surprising that it doesn’t give an error though, because they’re just undefined properties for the this.loginForm object which is declared as any.

I’m not sure about this. I think that I saw it in an article or tutorial somewhere, but I’m not able to find it right now. However it’s worth to check the generator templates as well.

@dhook This is a good advice and I’ll recommend you to consider it.

1 Like

Ah, I didn’t think of that. I am religious about typing things and try to avoid any as much as humanly possible.

Yes, very good advice… new to ionic2 as you can tell and yes, code was generated then adapted …

Before I posted this, I tried just passing in strings to see if that resolved it and it did not …

Just passing in strings does not resolve the issue either so I i will look at changing the method to pass back an observable… I have seen instances in which a Promise was used, hence the code…

Thank you guys for your insight!

The load method that is recognized from the login.ts:

load() {
    if (this.data) {
      // already loaded data
      return Promise.resolve(this.data);
    }

    // don't have the data yet
    return new Promise(resolve => {
      // We're using Angular Http provider to request the data,
      // then on the response it'll map the JSON data to a parsed JS object.
      // Next we process the data and resolve the promise with the new data.
      this.http.get('path/to/data.json')
        .map(res => res.json())
        .subscribe(data => {
          // we've got back the raw data, now generate the core schedule data
          // and save the data for later reference
          this.data = data;
          resolve(this.data);
        });
    });
  }

If you have control over the server API, I would strongly suggest redesigning it. If you don’t, I would suggest punching whoever wrote it in the face. Usernames and cleartext passwords are going to go in the server logs and appear in browser histories. That is bad. They should not be in URLs. They should be parameters passed to a POST request, not components of a GET one. GET requests do not have state effects. Authorization is a state effect.

Yes, totally agree. Right now, just trying to build something to learn and see how things work… So yes, back end can and will be changed… i dont want to punch myself…Right now curious why the Load method is visible and not the other methods…

I would recommend you to double-check this and test with hardcoded strings, looking at the code it should work this way. Changing the result of the method won’t fix the problem, i.e. it might be better from a coding perspective but it’s not related to the problem itself. Could you check if UserService.login() method is properly generated in the app.bundle.js in www/build/js?

Could you post the exact error message and also are there any other error messages?

On a side note: I noticed that you update the same class property in both login() and load() methods (look at this.data) which might cause errors, e.g. won’t work if you call first login() and then load() because the load() method will return directly the data stored by the login() method instead of the data that is expected to be returned by the load() method.

I think it’s the conference app.

Yes, maybe you’re right - conference-data.ts, but I think that I saw it also somewhere on the web.

What is the team’s position on issues related to code style in the sample apps? It’s not a real bug, per se, as I expect the existing code does what it’s designed to do, and I’m sure especially at this point in time there are obviously lots of more important things to worry about, but I do think these official sample projects are treated as a Holy Book by newcomers to the ecosystem, and perpetuating antipatterns would seem to be doing them a disservice.

I’ve only taken a cursory glance at that service, but I think it would be more idiomatic to eliminate use of promises completely, and have data be a ReplaySubject that is primed by an initial call to load() (which could be advertised publicly to show how to replace the backend with an external data source that can be refreshed). This would eliminate both the explicit promise construction and the ghost promise antipatterns, eliminate the need for calling load() in any of the get* functions - they could all just be maps off the data Subject, and they would both block before the initial load call and receive updates whenever load is explicitly called later. The client code would be in charge of deciding when to get upstream data - that’s more in line with the needs of real-world client-server apps, IMHO.

I don’t want to be stepping on people’s toes, though.

1 Like

I did use the conference app as a generator. Also, I found the issue, I had a duplicate class. Sorry for the runaround, however, do appreciate the input and time.

I guess that @brandyshea or @tim could answer this, IMHO all improvements should be welcome (however an improvement is a subjective notion, see below).

While I agree that this is probably much better from professional point of view, I believe that it will make the code too complex for newcomers, especially considering that many of them have only a basic programming knowledge. IMHO the main goal of the app is to show the new users how to create a basic app and to start building up from there - both in terms of app complexity and Ionic/Angular knowledge. That’s why I think that it’s very important to keep the right balance between a good architecture and an understandable code.

Considering that this is an open-source project I believe that every contribution counts. Some PRs will be accepted, others might be rejected, but in the end there will be progress towards a better app. Even the PRs that don’t align with the project’s goals could find their place in a new fork (e.g. one that contains a codebase targeted at professional/expert programmers).

Ok, great, I’m glad you figured it out.

Thank you again for your time and insight!

@iignatov & @rapropos,

Yes, all contributions are welcome and appreciated. We’ve neglected the conference app a bit to focus on the framework, so there are probably a lot of places that could be improved.

If you’d like to create an issue for any changes on the repository we can discuss them prior to implementation. Then, if you decide you want to create a PR, we will already be on the same page with the changes. :slight_smile:

2 Likes