Skip to content
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

Only one recording is associated with a given meeting therefore CloudRecordings.GetRecordingsAsync should be singular #161

Closed
flydart opened this issue Dec 14, 2021 · 6 comments
Assignees
Labels
Breaking Change This change causes backward compatibility issue(s)
Milestone

Comments

@flydart
Copy link

flydart commented Dec 14, 2021

When I use CloudRecordings.GetRecordingsAsync, I'm getting back only one recording instead of the list of recordings as the name suggests.

For the only recording returning, there is ShareUrl, but there's no passcode to access the recording.

How is everything getting a list of all recordings for a given recurring meeting?

@Jericho Jericho self-assigned this Dec 14, 2021
@Jericho Jericho added the Bug This change resolves a defect label Dec 14, 2021
@Jericho
Copy link
Owner

Jericho commented Dec 14, 2021

You're right, GetRecordingsAsync currently returns a single recording. It should return a paginated response if Zoom supports pagination for this API call or an array of Recordings if pagination is not supported. I'll fix that.

As for the passcode: either I completely missed this when I wrote the CloudRecordings resource or this information was added to the API response relatively recently and ZoomNet simply needs to be adjusted accordingly. Either way, I'll look into adding it to the Recording model class.

By the way, a little while ago I applied to become a Zoom ISV partner in order to get access to Zoom's entire API and therefore be in a better position to continue improving ZoomNet but haven't heard back. If you know anybody at Zoom who can influence their decision to accept my application, please put in a good word for me. Thanks!

@Jericho
Copy link
Owner

Jericho commented Dec 14, 2021

You're right, GetRecordingsAsync currently returns a single recording. It should return a paginated response if Zoom supports pagination for this API call or an array of Recordings if pagination is not supported. I'll fix that.

I researched this situation a little bit more and what I originally said is not correct. There is only a single recording for a given meeting and therefore it makes perfect sense for GetRecordingsAsync to return a single Recording object. There can be multiple files associated with the recording and the ZoomNet recording class has a RecordingFiles property to give you access to each of these files.

Having said that, I realize that the name of the method leads to confusion. The fact that GetRecordings is plural can certainly lead a reasonable person to conclude that multiple recordings can be associated with a given meeting, which is not the case. So, I propose renaming the method to remove the confusion: GetRecordingAsync.

Also I discovered that passcode is not the only missing property (for instance, we are also missing an array of audio files) so I will raise a separate issue.

@Jericho Jericho changed the title CloudRecordings.GetRecordingsAsync is only returning the latest recording instead of a list of all recordings and no passcode is available Only one recording is associated with a given meeting therefore CloudRecordings.GetRecordingsAsync should be singular Dec 14, 2021
@Jericho Jericho added this to the 0.37.0 milestone Dec 14, 2021
@Jericho Jericho added Breaking Change This change causes backward compatibility issue(s) and removed Bug This change resolves a defect labels Dec 14, 2021
@Jericho
Copy link
Owner

Jericho commented Dec 15, 2021

As if this wasn't confusing enough, the Zoom API documentation uses "recordings" and "recording files" interchangeably.

@Jericho
Copy link
Owner

Jericho commented Dec 17, 2021

In an effort to dispel confusion, I will be renaming the following methods on the CloudRecordings resource:

  • To make it clear that we are retrieving information about a meeting recording (which includes recording files and audio files)

    • GetRecordingsAsync renamed to GetRecordingInformationAsync
  • To make it clear that the following methods affects recording files

    • DeleteRecordingsAsync renamed to DeleteRecordingFilesAsync
    • DeleteRecordingAsync renamed to DeleteRecordingFileAsync
    • RecoverRecordingsAsync renamed to RecoverRecordingFilesAsync
    • RecoverRecordingAsync renamed to RecoverRecordingFileAsync

Jericho added a commit that referenced this issue Dec 17, 2021
@Jericho
Copy link
Owner

Jericho commented Dec 19, 2021

@flydart I published a pre-release version on my MyGet feed. Let me know if you can try it out and if you find the method names less confusion. Also, you should be able to access the missing "password" property.

Jericho added a commit that referenced this issue Dec 19, 2021
Jericho added a commit that referenced this issue Jan 15, 2022
@Jericho
Copy link
Owner

Jericho commented Jan 15, 2022

🎉 This issue has been resolved in version 0.37.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change This change causes backward compatibility issue(s)
Projects
None yet
Development

No branches or pull requests

2 participants