-
Notifications
You must be signed in to change notification settings - Fork 145
Planner helpers #34
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
Planner helpers #34
Conversation
Planner takes some time to set up and tear down resources such as tasks and buckets, so we'll reuse the same items for each test in the suite.
Although this is how the class is used [see Update PlannerPlanDetails](https://developer.microsoft.com/en-us/graph/docs/api-reference/v1.0/api/plannerplandetails_update), the service expects a JSON object and not a JSON array. Therefore, this breaks de/serialization in the SDK.
Also created a new test plan
| * The class for the Base Planner Assignments. | ||
| */ | ||
| public class BasePlannerAssignments implements IJsonBackedObject { | ||
| public class BasePlannerAssignments extends HashMap<String, PlannerAssignment> implements IJsonBackedObject { |
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.
The only reason I can see for extending HashMap is to support the put method. Rather than expose (and be forced to support) the huge HashMap api why not use composition instead of inheritance here (and only expose the put method)?
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.
You are correct on this.
However, since this is helper code written in C# and ported over to Java, and because we are trying to minimize our costs on custom code like this, I'll leave it as-is.
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.
okey doke
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.
Related to microsoftgraph/MSGraph-SDK-Code-Generator#114.
These helpers do a few things: