Skip to content

Emit resetPasswordRequest event with options #2992

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.

Sign up for GitHub

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

Jump to bottom
Merged
bajtos merged 1 commit into from
Jan 5, 2017
Merged

Emit resetPasswordRequest event with options #2992

bajtos merged 1 commit into from
Jan 5, 2017

Conversation

@rocknrolla777
Copy link
Contributor

@rocknrolla777 rocknrolla777 commented Nov 30, 2016 •
edited
Loading

Description

Currently the 'resetPasswordRequest' event only sends an email address from all options object. I propose to send all options object, because e.g. I have needed all options more than one time.

@bajtos @raymondfeng @superkhau does it make sense?

Related issues

  • None

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the styleguide

@slnode
Copy link

slnode commented Nov 30, 2016

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@slnode
Copy link

slnode commented Nov 30, 2016

Can one of the admins verify this patch?

3 similar comments
@slnode
Copy link

slnode commented Nov 30, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Nov 30, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Nov 30, 2016

Can one of the admins verify this patch?

@rocknrolla777 rocknrolla777 force-pushed the feature/resetPasswordRequest branch from 06ca7a8 to caa707f Compare November 30, 2016 12:05
@rocknrolla777 rocknrolla777 changed the title feat (models/user) emit resetPasswordRequest event with options feat(user) emit reset password event with options Nov 30, 2016
@superkhau
Copy link
Contributor

superkhau commented Nov 30, 2016

Can you add some tests to verify your changes and prevent regressions in the future?

@superkhau superkhau self-assigned this Nov 30, 2016
@rocknrolla777 rocknrolla777 force-pushed the feature/resetPasswordRequest branch from 2cb3577 to a4c8cc7 Compare December 1, 2016 10:35
@rocknrolla777
Copy link
Contributor Author

rocknrolla777 commented Dec 1, 2016

@superkhau I've done it)

@rocknrolla777
Copy link
Contributor Author

rocknrolla777 commented Dec 1, 2016 •
edited
Loading

@superkhau maybe it will be good to remove email in "resetPasswordRequest" like independent argument and emitting this event with email inside options?
Does it make sense? If yes - I will do it

@beeman
Copy link
Contributor

beeman commented Dec 6, 2016

Looks like a duplicate of #1628

@superkhau
Copy link
Contributor

superkhau commented Dec 6, 2016 •
edited
Loading

@beeman Thanks for finding the dupe. I think we should land this one as it includes/updates tests accordingly.

@rocknrolla777 LGTM

@superkhau superkhau removed the triaging label Dec 6, 2016
@superkhau
Copy link
Contributor

superkhau commented Dec 6, 2016

@slnode test please

@rocknrolla777 rocknrolla777 force-pushed the feature/resetPasswordRequest branch from a4c8cc7 to 02c3531 Compare December 6, 2016 09:59
@rocknrolla777
Copy link
Contributor Author

rocknrolla777 commented Dec 6, 2016 •
edited
Loading

@superkhau I have rebased master branch. Can you run tests again?

@beeman beeman mentioned this pull request Dec 6, 2016
Merged
2 tasks
@superkhau
Copy link
Contributor

superkhau commented Dec 6, 2016

@slnode test please

This was referenced Dec 8, 2016
Closed
Closed
@bajtos bajtos changed the title feat(user) emit reset password event with options Emit reset password event with options Dec 8, 2016
@bajtos bajtos changed the title Emit reset password event with options Emit resetPasswordRequest event with options Dec 8, 2016
@bajtos
Copy link
Member

bajtos commented Dec 8, 2016

The tests are all green. The last step is to squash all commits into a single one and add more meat to the commit message (see http://loopback.io/doc/en/contrib/git-commit-messages.html).

@superkhau
Copy link
Contributor

superkhau commented Dec 8, 2016

@rocknrolla777 Can you do as @bajtos suggested at #2992 (comment) so we can land this?

@rocknrolla777 rocknrolla777 force-pushed the feature/resetPasswordRequest branch from 02c3531 to 59d494e Compare December 9, 2016 15:22
@rocknrolla777
Copy link
Contributor Author

rocknrolla777 commented Dec 9, 2016 •
edited
Loading

@bajtos @superkhau Done.

@rocknrolla777 rocknrolla777 force-pushed the feature/resetPasswordRequest branch from 59d494e to 91a6f9e Compare December 9, 2016 15:53
@rocknrolla777 rocknrolla777 force-pushed the feature/resetPasswordRequest branch from 91a6f9e to fa8bca8 Compare December 9, 2016 16:14
@superkhau
Copy link
Contributor

superkhau commented Dec 10, 2016

@slnode test please

@bajtos bajtos merged commit 298635d into strongloop:master Jan 5, 2017
@bajtos
Copy link
Member

bajtos commented Jan 5, 2017

Landed, sorry for the long delay.

@bajtos bajtos mentioned this pull request Jan 5, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@bajtos bajtos bajtos approved these changes

@superkhau superkhau superkhau approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@superkhau superkhau

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rocknrolla777 @slnode @superkhau @beeman @bajtos @crandmck