-
Notifications
You must be signed in to change notification settings - Fork 0
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
update castle sdk remove deprecated fields #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that Risk and Filter aren't as similar as they were before, I wonder if it makes sense to split the code into a risk.go
file and a filter.go
file, where the relevant types are defined next to the corresponding API methods?
client.go
Outdated
r := &castleAPIRequest{ | ||
params := Params{ | ||
Email: req.User.Email, | ||
Username: req.User.Name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the name necessarily the username? I wonder if we're reusing the name
field in our generic request type in a way that wasn't intended by the underlying API, or are they actually the same values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No so i actually was just thinking about this now, I remember us using either the account number or the email as username in a service somewhere but i can't think where.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really sure whether to include this now, looking at the code from the castle-processor the username is seems to be either the email or account number depending on what the user used to sign in. The username value is contained in the request properties so I've just set that as the username, might be that the username and the email are duplicates in some cases but I don't think that will be an issue.
Co-authored-by: James Tait <[email protected]>
Co-authored-by: James Tait <[email protected]>
Updating the request sent to castle to include the created_at timestamp, context here, and fix general api drift.