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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

update artifacts of the todo-list example to the latest style generated by the LoopBack CLI #6598

Merged
merged 1 commit into from Nov 10, 2020

Conversation

emonddr
Copy link
Contributor

@emonddr emonddr commented Oct 19, 2020

Update the artifacts of this example
to the latest style generated by the
LoopBack CLI.

Update the documentation to add the
step of adding TodoListImage controller.

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

) {}

@post('/todo-lists/{id}/image', {
@post('/todo-list-images', {
Copy link
Contributor Author

@emonddr emonddr Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /todo-list/id/todo-list-image urls go into their own controller; see todo-list-todo-list-image.controller.ts.

@@ -182,6 +182,10 @@ $ npm start
Server is running at http://127.0.0.1:3000
```

{% include note.html content="
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this because there are some default filter objects that produce zero results or cause an http 500. So clearing these default filter objects provided by the API Explorer (swagger ui?) is best.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET /todos has a default filter of

{
  "offset": 0,
  "limit": 100,
  "skip": 0,
  "where": {
    "additionalProp1": {}
  },
  "include": [
    {
      "relation": "string",
      "scope": {
        "offset": 0,
        "limit": 100,
        "skip": 0,
        "where": {
          "additionalProp1": {}
        },
        "include": [
          {
            "additionalProp1": {}
          }
        ]
      }
    }
  ]
}

if user presses [Execute] button,
an exception is thrown

{
  "error": {
    "statusCode": 500,
    "message": "Internal Server Error"
  }
}

if we clear this filter and try again, the proper number of todos is returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET /todos/id

has filter

{
  "offset": 0,
  "limit": 100,
  "skip": 0,
  "include": [
    {
      "relation": "string",
      "scope": {
        "offset": 0,
        "limit": 100,
        "skip": 0,
        "where": {
          "additionalProp1": {}
        },
        "include": [
          {
            "additionalProp1": {}
          }
        ]
      }
    }
  ]
}

produces 500 error as well. Must be cleared.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET /todos/count

has a where object

{
  "additionalProp1": {}
}

that returns 0. Clearing it returns proper amount.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET /todo-lists/{id}/todo-list-image
and
/todo-lists/{id}/todo

have same filter which causes error 500. Clearing them works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps I should change the note.html to

{% include note.html content=" When using the API Explorer, be sure to clear out any default filter object or where object as this may affect your results. " %}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have more details about what causes the 500 error? Any stacktrace?

Copy link
Contributor Author

@emonddr emonddr Oct 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have more details about what causes the 500 error? Any stacktrace?

GET /todos (with default filter above) returns:

Error: Invalid "filter.include" entries: {"relation":"string","scope":{"offset":0,"limit":100,"skip":0,"where":{"additionalProp1":{}},"include":[{"additionalProp1":{}}]}}

This is the full stack trace:

Server is running at http://[::1]:3000
Request GET /todos?filter=%7B%0A%20%20%22offset%22%3A%200%2C%0A%20%20%22limit%22%3A%20100%2C%0A%20%20%22skip%22%3A%200%2C%0A%20%20%22where%22%3A%20%7B%0A%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%7D%2C%0A%20%20%22include%22%3A%20%5B%0A%20%20%20%20%7B%0A%20%20%20%20%20%20%22relation%22%3A%20%22string%22%2C%0A%20%20%20%20%20%20%22scope%22%3A%20%7B%0A%20%20%20%20%20%20%20%20%22offset%22%3A%200%2C%0A%20%20%20%20%20%20%20%20%22limit%22%3A%20100%2C%0A%20%20%20%20%20%20%20%20%22skip%22%3A%200%2C%0A%20%20%20%20%20%20%20%20%22where%22%3A%20%7B%0A%20%20%20%20%20%20%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%20%20%20%20%20%20%7D%2C%0A%20%20%20%20%20%20%20%20%22include%22%3A%20%5B%0A%20%20%20%20%20%20%20%20%20%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%20%20%20%20%5D%0A%20%20%20%20%20%20%7D%0A%20%20%20%20%7D%0A%20%20%5D%0A%7D failed with status code 500. Error: Invalid "filter.include" entries: {"relation":"string","scope":{"offset":0,"limit":100,"skip":0,"where":{"additionalProp1":{}},"include":[{"additionalProp1":{}}]}}
    at Object.includeRelatedModels (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/repository/dist/relations/relation.helpers.js:70:21)
    at TodoRepository.includeRelatedModels (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/repository/dist/repositories/legacy-juggler-bridge.js:389:28)
    at TodoRepository.find (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/repository/dist/repositories/legacy-juggler-bridge.js:286:21)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/rest/dist/providers/invoke-method.provider.js:40:32
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/express/dist/middleware-interceptor.js:82:25
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/express/dist/middleware-interceptor.js:82:25
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/rest/dist/providers/send.provider.js:39:32
    at async MiddlewareSequence.handle (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/rest/dist/sequence.js:201:9)
    at async HttpHandler._handleRequest (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback-next/packages/rest/dist/http-handler.js:69:9)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a separate issue #6641 to track this potential problem in the API Explorer. It isn't related to the re-generated artifacts of this todolist example per se.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same error happens with existing todo-list example

Server is running at http://127.0.0.1:3000
Unhandled error in GET /todos?filter=%7B%0A%20%20%22offset%22%3A%200%2C%0A%20%20%22limit%22%3A%20100%2C%0A%20%20%22skip%22%3A%200%2C%0A%20%20%22where%22%3A%20%7B%0A%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%7D%2C%0A%20%20%22fields%22%3A%20%7B%0A%20%20%20%20%22id%22%3A%20true%2C%0A%20%20%20%20%22title%22%3A%20true%2C%0A%20%20%20%20%22desc%22%3A%20true%2C%0A%20%20%20%20%22isComplete%22%3A%20true%2C%0A%20%20%20%20%22todoListId%22%3A%20true%0A%20%20%7D%2C%0A%20%20%22include%22%3A%20%5B%0A%20%20%20%20%7B%0A%20%20%20%20%20%20%22relation%22%3A%20%22string%22%2C%0A%20%20%20%20%20%20%22scope%22%3A%20%7B%0A%20%20%20%20%20%20%20%20%22offset%22%3A%200%2C%0A%20%20%20%20%20%20%20%20%22limit%22%3A%20100%2C%0A%20%20%20%20%20%20%20%20%22skip%22%3A%200%2C%0A%20%20%20%20%20%20%20%20%22where%22%3A%20%7B%0A%20%20%20%20%20%20%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%20%20%20%20%20%20%7D%2C%0A%20%20%20%20%20%20%20%20%22fields%22%3A%20%7B%0A%20%20%20%20%20%20%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%20%20%20%20%20%20%7D%2C%0A%20%20%20%20%20%20%20%20%22include%22%3A%20%5B%0A%20%20%20%20%20%20%20%20%20%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%22additionalProp1%22%3A%20%7B%7D%0A%20%20%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%20%20%20%20%5D%0A%20%20%20%20%20%20%7D%0A%20%20%20%20%7D%0A%20%20%5D%0A%7D: 500 Error: Invalid "filter.include" entries: {"relation":"string","scope":{"offset":0,"limit":100,"skip":0,"where":{"additionalProp1":{}},"fields":{"additionalProp1":{}},"include":[{"additionalProp1":{}}]}}
    at Object.includeRelatedModels (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/repository/dist/relations/relation.helpers.js:70:21)
    at TodoRepository.includeRelatedModels (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/repository/dist/repositories/legacy-juggler-bridge.js:389:28)
    at TodoRepository.find (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/repository/dist/repositories/legacy-juggler-bridge.js:286:21)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/rest/dist/providers/invoke-method.provider.js:40:32
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/express/dist/middleware-interceptor.js:82:25
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/express/dist/middleware-interceptor.js:82:25
    at async /Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/rest/dist/providers/send.provider.js:39:32
    at async MiddlewareSequence.handle (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/rest/dist/sequence.js:201:9)
    at async HttpHandler._handleRequest (/Users/dremond/Documents/PRs_LoopBack/authTwo/loopback4-example-todo-list/node_modules/@loopback/rest/dist/http-handler.js:69:9)

@raymondfeng ^^

@@ -69,6 +69,28 @@ lb4 repository
Repository TodoListImageRepository was created in src/repositories/
```

### Create the controller
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step was missing in the documentation. The controller appeared in the finished sample, but the docs didn't contain the steps. Adding the step in.

@@ -103,6 +125,45 @@ $ lb4 relation
Relation BelongsTo was created in src/
```

As you can see, this created the relational controllers
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the same idea that is used in the todo/id/todlist controller. Move the only method to the todo controller, and delete the relation controller that only has one method. We'll do the same thing here for the /todo-list-image/id/todo-list method.

@emonddr emonddr force-pushed the dremond_update_todolist_example branch 2 times, most recently from bfb2ead to c302288 Compare October 19, 2020 14:54
@emonddr emonddr requested review from raymondfeng, dhmlau and jannyHou and removed request for hacksparrow, nabdelgadir and agnes512 October 19, 2020 15:30
@emonddr emonddr force-pushed the dremond_update_todolist_example branch from c302288 to bc693c3 Compare October 19, 2020 16:22
@emonddr
Copy link
Contributor Author

emonddr commented Oct 19, 2020

Looks like I need to add more test cases...

@emonddr emonddr force-pushed the dremond_update_todolist_example branch 2 times, most recently from 25cb2e3 to 9507f33 Compare October 23, 2020 14:24
@emonddr
Copy link
Contributor Author

emonddr commented Nov 2, 2020

All tests are passing. Just need an approval :)

Update the artifacts of this example
to the latest style generated by the
LoopBack CLI.

Update the documentation to add the
step of adding TodoListImage controller.

Signed-off-by: Dominique Emond <dremond@ca.ibm.com>
@emonddr emonddr force-pushed the dremond_update_todolist_example branch from bbc58e6 to d46849e Compare November 10, 2020 15:35
@emonddr emonddr merged commit 1d875e8 into master Nov 10, 2020
2 checks passed
@emonddr emonddr deleted the dremond_update_todolist_example branch November 10, 2020 19:22
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 this pull request may close these issues.

None yet

3 participants