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

[REST] Consistent response format for PATCH/PUT #1471

Closed
virkt25 opened this issue Jun 25, 2018 · 3 comments · Fixed by #1770
Closed

[REST] Consistent response format for PATCH/PUT #1471

virkt25 opened this issue Jun 25, 2018 · 3 comments · Fixed by #1770
Assignees
Labels
breaking-change bug REST Issues related to @loopback/rest package and REST transport in general

Comments

@virkt25
Copy link
Contributor

virkt25 commented Jun 25, 2018

Description / Steps to reproduce / Feature proposal

From #1206

A PATCH / PUT request returns true while other REST endpoints return a JSON output. Should these methods also return a JSON response to be consistent?

Current Behavior

  • Returns a boolean for PATCH / PUT

Expected Behavior

  • Return JSON ... something like {success: true} or {result: <value-returned-by-juggler>}
  • Update existing code / templates / tests

cc: @strongloop/sq-lb-apex @raymondfeng @bajtos Thoughts? Is this the intended behaviour or a bug? If it's the intended behaviour, do we want to look into changing it for LB4?

See Reporting Issues for more tips on writing good issues

@bajtos
Copy link
Member

bajtos commented Jun 26, 2018

+1 for returning a JSON like {success: true} or perhaps {result: <value-returned-by-juggler>}.

Please note that we cannot return full model data in most cases. It would require another database command to fetch the updated model instance, which would introduce race condition.

I am going to label this as a bug and assign to Core-GA backlog for consideration.

@bajtos bajtos added bug Core-GA REST Issues related to @loopback/rest package and REST transport in general labels Jun 26, 2018
@jannyHou
Copy link
Contributor

+1 for returning a JSON result. Agree with Miroslav's comment. And it also allows we to customize result details, e.g. Cloudant includes which data fails update and which one passes.

@bajtos
Copy link
Member

bajtos commented Sep 25, 2018

In #1723, I reworked methods returning boolean to return undefined and report "model not found" via 404 Not Found error.

However, the following endpoints are still returning a naked number:

  • PATCH /my-models (repo.updateAll(data, where)) returns a number of updated instances.
  • GET /my-models/count (repo.count(where)) returns a number of instance found.

I think we should rework these two endpoints to wrap the number in an object:

{
  "count": 123
}

For reference, here is the LoopBack 3.x behavior:

  • count returns an object with count property, e.g. {count: 6}.
  • updateAll returns an object with count property too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change bug REST Issues related to @loopback/rest package and REST transport in general
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants