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

Init pwd #88

Merged
merged 8 commits into from Jul 1, 2019
Merged

Init pwd #88

merged 8 commits into from Jul 1, 2019

Conversation

bsoniam
Copy link
Contributor

@bsoniam bsoniam commented Jun 21, 2019

added INIT_PASSWORD event when a support user calls the API execute-actions-email with the action sms-password-set

@coveralls
Copy link

coveralls commented Jun 21, 2019

Pull Request Test Coverage Report for Build 842

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 93.436%

Totals Coverage Status
Change from base Build 828: 0.008%
Covered Lines: 2249
Relevant Lines: 2407

💛 - Coveralls

@bsoniam bsoniam requested review from harture and fperot74 June 21, 2019 09:42
@@ -490,9 +490,14 @@ func (c *component) ExecuteActionsEmail(ctx context.Context, realmName string, u
var accessToken = ctx.Value(cs.CtContextAccessToken).(string)

var actions = []string{}
var init_password_action = "sms-password-set"
Copy link
Contributor

Choose a reason for hiding this comment

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

You might have a go-lint recommandation asking not to use underscore in variable names. initPasswordAction would be more Go-compliant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hahaha, only now I noticed how I named this variable (Friday tiredness) :))).
Thanks, good point.

@@ -474,8 +474,8 @@ func (c *component) ResetPassword(ctx context.Context, realmName string, userID
}

//store the API call into the DB
// the error should be treated
_ = c.reportEvent(ctx, "INIT_PASSWORD", database.CtEventRealmName, realmName, database.CtEventUserID, userID)
//_ = c.reportEvent(ctx, "INIT_PASSWORD", database.CtEventRealmName, realmName, database.CtEventUserID, userID)
Copy link
Contributor

Choose a reason for hiding this comment

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

If not used, you'd better completely remove this block

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 would prefer to leave it as I don't know if the back office will start using it or not soon

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

Choose a reason for hiding this comment

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

If we decide to leave the functionality, I would suggest to also leave the reportEvent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -1281,8 +1281,8 @@ func TestExecuteActionsEmail(t *testing.T) {
var accessToken = "TOKEN=="
var realmName = "master"
var userID = "1245-7854-8963"
var reqActions = []api.RequiredAction{"action1", "action2"}
var actions = []string{"action1", "action2"}
var reqActions = []api.RequiredAction{"sms-password-set", "action1", "action2"}
Copy link
Contributor

Choose a reason for hiding this comment

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

If you move init_password_action declaration in a const (...) block at the beginning of component.go, you could reuse it here and prevent potential mis-spelling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -474,8 +474,8 @@ func (c *component) ResetPassword(ctx context.Context, realmName string, userID
}

//store the API call into the DB
// the error should be treated
_ = c.reportEvent(ctx, "INIT_PASSWORD", database.CtEventRealmName, realmName, database.CtEventUserID, userID)
//_ = c.reportEvent(ctx, "INIT_PASSWORD", database.CtEventRealmName, realmName, database.CtEventUserID, userID)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we decide to leave the functionality, I would suggest to also leave the reportEvent.

@harture harture merged commit 5e453f2 into master Jul 1, 2019
@harture harture deleted the init-pwd branch October 28, 2019 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants