-
Notifications
You must be signed in to change notification settings - Fork 42
Bug fixes #56
Bug fixes #56
Conversation
caitlinrussell
commented
May 23, 2017
- Fixed serialization of Duration type
- Removed duplicate implementation of DatatypeFactory
- Fixed Sendmail endpoint
I fixed a couple problems with this implementation: - Added serializer and deserializer for Duration type - Used existing DatatypeFactory instead of reimplementing from xerces
| @@ -0,0 +1,25 @@ | |||
| # Making Custom Calls to Graph | |||
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.
Should a link to this page be added to README or docs/overview.md?
| Then you can instantiate a new request to use to call Graph: | ||
|
|
||
| ```Java | ||
| CustomRequest request = new CustomRequest("https://graph.microsoft.com/v1.0/custom", graphServiceClient, requestOptions); |
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.
Are there any limitations that we should mention here? Can any URL be passed in or does it have to be a known graph deployment (main/china cloud/etc)? Can you pass in query parameters via the URL or do they need to be passed in via requestOptions?
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.
Yes, you can pass in any URL to this service, and you can include query parameters in either the URL or requestOptions.
| List<AttendeeBase> attendees = new ArrayList<>(); | ||
| AttendeeBase attendeeBase = new AttendeeBase(); | ||
| EmailAddress email = new EmailAddress(); | ||
| email.address = "katiej@mod810997.onmicrosoft.com"; |
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.
Should we have a constants file to handle mock data? Would it make it easier to update all of these in the future?
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.
That is a good idea. Added to .Net backlog
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.
I am currently using a constants file and I should use this for Planner and OneNote tests. In this instance, I should probably be getting this address programmatically so it works on any tenant instead of hard coding an email.
MIchaelMainer
left a comment
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.
Looking good, 3 comments.
docs/custom-queries.md
Outdated
| @@ -0,0 +1,25 @@ | |||
| # Making Custom Calls to Graph | |||
|
|
|||
| The Graph SDK attempts to enable all available scenarios through Microsoft Graph. There are times, however, through errors or custom Graph functionality, that makes calling the desired endpoint is not possible through the provided requests and builders. | |||
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.
Remove the 'is' in 'that makes calling the desired endpoint is not possible.'
| @@ -0,0 +1,8 @@ | |||
| # Known Issues | |||
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.
Great idea to add the known issues page.
I think you should remove that first sentence. The subordinate clause contains information that is not readily understood by customers. For example, workload teams may not be widely understood.
Instead, I suggest that we point out that the client library: 1) is a snapshot of the Microsoft Graph and may not be in sync with the service at any one point so issues may arise from that, 2) the client library is generated; the generator may not capture all potential scenarios, 3) service behavior changes may not be reflected in the client library as the development of the client library and the Microsoft Graph are loosely coupled
| List<AttendeeBase> attendees = new ArrayList<>(); | ||
| AttendeeBase attendeeBase = new AttendeeBase(); | ||
| EmailAddress email = new EmailAddress(); | ||
| email.address = "katiej@mod810997.onmicrosoft.com"; |
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.
That is a good idea. Added to .Net backlog