OAuth SDK #118
OAuth SDK #118
Conversation
| } | ||
| } | ||
|
|
||
| private void ValidateNonEmptyString(Dictionary<string, object> payload, string field) |
There was a problem hiding this comment.
ValidateNonEmptyString and ValidateNonNull can be clubbed
There was a problem hiding this comment.
IMO no. For some cases, we are checking that a field should exist. In some cases, field can be optional, but if passed, value cant be empty string.
| { | ||
| if (statusCode >= 400 && statusCode < 500) | ||
| { | ||
| errorCode = ErrorCodes.BAD_REQUEST_ERROR.ToString(); |
There was a problem hiding this comment.
a more detailed error would be better here
There was a problem hiding this comment.
This is actually just a mapping of error code and not defining error message, that will be passed further as it is. The above change is due to the reason that the SDK currently is tightly coupled with what we have in API monolith. Since we are calling Auth service here, we dont receive such error codes in response. Hence we are just setting errorCode based on statusCode we receive from APIs via auth service.
src/OAuthTokenClient.cs
Outdated
|
|
||
| public string GetAuthUrl(Dictionary<string, object> data) | ||
| { | ||
| ValidateAuthUrlRequest(data); |
There was a problem hiding this comment.
ValidateAuthUrlRequest etc functions seems to be overhead. inside those functions there is nothing but 1 line of code to validate. we can replace that here itself
Note :- Please follow the below points while attaching test cases document link below:
- If label
Testedis added then test cases document URL is mandatory.- Link added should be a valid URL and accessible throughout the org.
- If the branch name contains hotfix / revert by default the BVT workflow check will pass.
This PR consists of changes to help Platform partners onboard to OAuth flow via SDK. Using this SDK, they can: