Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String type in the generated model class from lb4 discover not correct #3806

Closed
2 of 3 tasks
dhmlau opened this issue Sep 25, 2019 · 3 comments · Fixed by #4180
Closed
2 of 3 tasks

String type in the generated model class from lb4 discover not correct #3806

dhmlau opened this issue Sep 25, 2019 · 3 comments · Fixed by #4180
Assignees
Labels

Comments

@dhmlau
Copy link
Member

dhmlau commented Sep 25, 2019

Related to #2953

It seems like some of the items mentioned in #2953 wasn't fixed completely.

Runs the lb4 discover command and the generated code isn't exactly what we're looking for.

Steps to reproduce

  1. Create a very simple table, e.g.
  Table "public.customer"
  Column  | Type | Collation | Nullable | Default 
----------+------+-----------+----------+---------
 custid   | text |           | not null | 
 custname | text |           |          | 

This is my command on postgres:

CREATE TABLE customer (custid text PRIMARY KEY, custname text);
  1. Scaffold an app and create a datasource that connects to a database

  2. Run:

npm run build
npm run migrate

Current Behavior

@property({
    type: String,
    required: true,
    id: 1,
    postgresql: {"columnName":"custid","dataType":"text","dataLength":null,"dataPrecision":null,"dataScale":null,"nullable":"NO"},
  })
  custid: String;

Expected Behavior

@property({
    type: string, <----- problem 1: lower case "string"
    required: true,
    id: 1,
    postgresql: {"columnName":"custid","dataType":"text","dataLength":null,"dataPrecision":null,"dataScale":null,"nullable":"NO"}, <--- problem 2: ideally it is formatted properly
  })
  custid: String;  <---- problem 3: lower case "string" 

See details in #2953 (comment)

Acceptance Criteria

There are more details in #3711 from @bajtos.

@bajtos
Copy link
Member

bajtos commented Sep 27, 2019

See also #3711 that describes another aspect of this problem, one related to controllers & repositories.

@emonddr
Copy link
Contributor

emonddr commented Oct 10, 2019

Double-check for other types as well.

@dhmlau dhmlau removed the p2 label Oct 15, 2019
@dhmlau dhmlau added this to the Nov 2019 milestone milestone Nov 1, 2019
@jannyHou jannyHou assigned jannyHou and unassigned jannyHou Nov 14, 2019
@jannyHou
Copy link
Contributor

My finding so far:

The model discover generator calls discoverSingleModel to get model definitions, and the schema returned from this function has primitive data type as camel case like String, instead of all lower case string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants