-
Notifications
You must be signed in to change notification settings - Fork 133
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
Polls #179
base: develop
Are you sure you want to change the base?
Polls #179
Conversation
…tration add meeting registrant apis
Add panelist api
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() |
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
@@ -7,4 +7,4 @@ | |||
|
|||
|
|||
__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
@@ -2,4 +2,4 @@ | |||
|
|||
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