Add "signup" / create user in authentication-jwt module #5602
Conversation
HrithikMittal
requested review from
emonddr,
hacksparrow and
jannyHou
as code owners
madaky
left a comment
madaky
left a comment
There was a problem hiding this comment.
@HrithikMittal : can you perform npm test on local to check for the issue. There is a step by step tutorial on how to contribute a code change :
https://loopback.io/doc/en/lb4/submitting_a_pr.html
| @requestBody({ | ||
| content: { | ||
| 'application/json': { | ||
| schema: getModelSchemaRef(NewUserRequest, { |
There was a problem hiding this comment.
No definition for NewUserRequest
There was a problem hiding this comment.
| newUserRequest: NewUserRequest, | ||
| ): Promise<User> { | ||
| const password = await hash(newUserRequest.password, await genSalt()); | ||
| const savedUser = await this.userRepository.create( |
There was a problem hiding this comment.
Will throw a property does not exist. as no dependency injection.
| }) | ||
| newUserRequest: NewUserRequest, | ||
| ): Promise<User> { | ||
| const password = await hash(newUserRequest.password, await genSalt()); |
There was a problem hiding this comment.
No definition for hash and gen salt
There was a problem hiding this comment.
They are imported from bcryptjs:
https://github.com/strongloop/loopback-next/blob/master/examples/todo-jwt/src/controllers/user.controller.ts#L19
extensions/authentication-jwt/src/__tests__/fixtures/controllers/user.controller.ts
Show resolved
Hide resolved
jannyHou
left a comment
jannyHou
left a comment
There was a problem hiding this comment.
@HrithikMittal Thank you for taking care of the improvement 👍
And thanks for @madaky and @dougal83 's review! Please fix the imports, otherwise LGTM.
| @requestBody({ | ||
| content: { | ||
| 'application/json': { | ||
| schema: getModelSchemaRef(NewUserRequest, { |
There was a problem hiding this comment.
| }) | ||
| newUserRequest: NewUserRequest, | ||
| ): Promise<User> { | ||
| const password = await hash(newUserRequest.password, await genSalt()); |
There was a problem hiding this comment.
They are imported from bcryptjs:
https://github.com/strongloop/loopback-next/blob/master/examples/todo-jwt/src/controllers/user.controller.ts#L19
|
Hi, Everyone Sorry to create an another pull request for this but I do the changes there and also test it in my local laptop. I think it would work Please look at this #5763 |
5 participants
This pull request is to provide the signup method in the authentication-jwt So it would be easier for developers.
https://github.com/strongloop/loopback-next/blob/master/extensions/authentication-jwt/src/__tests__/fixtures/controllers/user.controller.ts
Fixes #5312
Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈