-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/conversation contact #33
Conversation
… in Verification creation method
await _http.Send<ListContactsResponse>(uri, HttpMethod.Get, cancellationToken); | ||
request.PageToken = response.NextPageToken; | ||
foreach (var contact in response.Contacts) yield return contact; | ||
} while (request.PageToken is not null); |
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.
note to self
} while (request.PageToken is not null); | |
} while (!string.IsNullOrEmpty(request.PageToken)); |
private string _email; | ||
private string _externalId; | ||
private string _id; | ||
private string _language; |
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.
From specs it is a dedicated schema and a (long) list of predefined values.
Ideally we should provide enums as helper ?
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.
Yeah, makes sense.
Side comment: did you enjoy working with enums of 20+ 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 😞
But from SDK user point of view and because not related to any ISO definition, we can just assuming providing list of supported values will be helpful
/// <returns></returns> | ||
internal string GetPropertiesMask() | ||
{ | ||
return string.Join(',', _setFields.Select(StringUtils.ToSnakeCase)); |
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.
Ideally this logic could be dynamic and dependent of OAS description based onto style
and explode
specs.
This, to avoid hard coded logic here and logic/helper shared across domains
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.
Okay, why to avoid makes sense but how, you mean add helpers for that?
I had an idea for that but in a other task where i see the pattern
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.
Java SDK is based onto this: https://github.com/sinch/sinch-sdk-java/blob/feat/voice/core/src/main/com/sinch/sdk/core/http/URLParameterUtils.java
And thanks to generator tool using OAS file description, generated sources contains directives about parameters format to be used (e.g. https://github.com/sinch/sinch-sdk-java/blob/feat/voice/openapi-contracts/src/main/com/sinch/sdk/domains/numbers/adapters/api/v1/ActiveNumberApi.java#L278)
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.
Ah, okay, I'll refactor in other task
Add support for contacts api endpoints