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

Add account uid to system.notification.account #537

Merged
merged 1 commit into from Jun 15, 2018

Conversation

rxx
Copy link

@rxx rxx commented Jun 14, 2018

No description provided.

Copy link

@geoffw8 geoffw8 left a comment

Choose a reason for hiding this comment

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

Can we make this account_uid in line with the document verification events?

Right now we have Peatio as 'uid' and Barong as 'account_uid'.

@rxx rxx force-pushed the bugfix/event_account_uid branch from 43ad8ef to 4ea6440 Compare June 14, 2018 13:03
@mod
Copy link
Contributor

mod commented Jun 14, 2018

@maksim-litvinov shouldn't we use uid instead of account_uid ?
also uid means user-id

@rxx rxx force-pushed the bugfix/event_account_uid branch from 4ea6440 to a7bed6c Compare June 15, 2018 07:41
@@ -8,12 +8,14 @@ def confirmation_instructions(record, token, opts = {})
end

def reset_password_instructions(record, token, opts = {})
EventAPI.notify('system.notification.account', reset_password_token: token)
EventAPI.notify('system.notification.account',
Copy link

Choose a reason for hiding this comment

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

I think the naming keys here are the wrong way round, it should actually be system.account.notification right?

Also - how comes this isn't actually system.account.forgotten_password?

@rxx rxx added the PR: WIP label Jun 15, 2018
@rxx rxx force-pushed the bugfix/event_account_uid branch from a7bed6c to 445539d Compare June 15, 2018 08:45
@rxx rxx removed the PR: WIP label Jun 15, 2018
@mod mod merged commit 6466fb2 into openware:1-8-stable Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants