-
Notifications
You must be signed in to change notification settings - Fork 129
Polls #179
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
base: develop
Are you sure you want to change the base?
Polls #179
Conversation
…tration add meeting registrant apis
Add panelist api
prschmid
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.
Thank you for this PR and our work to expand this library! I've added in a couple of comments for minor fixes. Looking forward to merging this in.
Thanks!
| responses.add( | ||
| responses.POST, "http://foo.com/webinar/ID/panelists", | ||
| ) | ||
| response = self.component.add_panelists(panelists=[{"name": "Mary", "email": "[email protected]"}]) |
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.
Can you make this a more human readable email? Maybe[email protected]?
| ) | ||
| response = self.component.add_panelists(panelists=[{"name": "Mary", "email": "[email protected]"}]) | ||
| self.assertEqual( | ||
| response.request.body, '{panelists: [{"name": "Mary", "email": "[email protected]"}]}' |
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.
See above
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() No newline at end of file |
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.
Did you run black to check for formatting? Looks like some files are missing a newline at the end.
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.
| ) | ||
| response = self.component.delete_panelists(panelists=[{"name": "Mary", "email": "[email protected]"}]) | ||
| self.assertEqual( | ||
| response.request.body, '{panelists: [{"name": "Mary", "email": "[email protected]"}]}' |
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.
See previous comment regarding email
zoomus/__init__.py
Outdated
|
|
||
| __all__ = ["API_VERSION_1", "API_VERSION_2", "ZoomClient"] | ||
| __version__ = "1.1.3" | ||
| __version__ = "1.1.4" |
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.
Please revert this change and leave it as 1.1.3 -- I'll update it to the appropriate version when I cut the next release (which likely will be 1.1.6).
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.
Sure Did it
zoomus/components/__init__.py
Outdated
| from __future__ import absolute_import | ||
|
|
||
| from . import meeting, metric, past_meeting, phone, recording, report, user, webinar | ||
| from . import meeting, metric, past_meeting, phone, recording, report, user, webinar, poll |
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.
Can you add the poll in the correct alphabetical order? I.e. after phone. Thanks!
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.
Sure! Did the same
zoomus/components/poll.py
Outdated
| def list(self, **kwargs): | ||
| util.require_keys(kwargs, "id") | ||
| return self.get_request( | ||
| f"/{self.type}/{kwargs.get('id')}/polls" |
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.
Looks like in some cases you are using f"...." and in others "...".format(). Since the rest of the library uses the "...".format() syntax, can you please switch it to that for consistency's sake?
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.
Sure, I have changed the f"...."and in this "...".format()
|
@vishnuku Thank you for this PR! We merged in some code that it appears you based your work off of. Can I trouble you to resolve the conflicts so that we can get this merged? Thank you for your time and effort! |
Hi @mfldavidson , I will complete these on this weekend. Thanks for reminding |
@vishnuku no problem, we really appreciate your work on this! |
Add poll functionality for meeting and webinars