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

Missing parameter in IMeeting.GetAsync method #352

Closed
sitkevich opened this issue Jul 20, 2024 · 6 comments
Closed

Missing parameter in IMeeting.GetAsync method #352

sitkevich opened this issue Jul 20, 2024 · 6 comments
Assignees
Labels
Enhancement New feature or request
Milestone

Comments

@sitkevich
Copy link

Please add parameter show_previous_occurrences to the API get metting method

https://developers.zoom.us/docs/meeting-sdk/apis/#operation/meeting

@Jericho Jericho self-assigned this Jul 20, 2024
@Jericho Jericho added the Breaking Change This change causes backward compatibility issue(s) label Jul 20, 2024
@Jericho Jericho added this to the 0.79.0 milestone Jul 20, 2024
@Jericho
Copy link
Owner

Jericho commented Jul 20, 2024

Looks like a new parameter. Thanks for bringing it to my attention, I wasn't aware of it. They added the same parameter to "get webinar" as well.

@Jericho
Copy link
Owner

Jericho commented Jul 21, 2024

I have been experimenting with this new parameter when fetching a recuring meeting and indeed I can see that past occurrences are omitted from the result when this parameter is false and they are included when true. As you pointed out, the ZoomNet library currently does not allow you to configure this parameter to include past occurrences and therefore I understand why you requested this new feature.

I could certainly add a new boolean parameter to the Meetings.GetAsync method (and the Webinars.GetAsync as well) but here's an alternate idea: what if I don't add a new parameter to the GetAsync methods and instead I hardcode show_previous_occurrences=true. This would result in the past occurrences to always be included when fetching a recurring meeting, and the developer could choose to ignore these past occurrences if desired.

One reason why I am considering this alternate solution if that GetAsync is used to retrieve any type of meeting, not just recurring ones. But this new parameter would be useless, and in fact ignored, when fetching any other type of meeting (such as 'scheduled' for example). Couldn't this lead to confusion?

If we end up deciding to add this new parameter, I would add a note in the XML comments along these lines:

/// <remarks>
/// Please note that this boolean parameter is applicable only when fetching a recurring meeting.
/// It will be ignored if you are fetching any other type of meeting such as scheduled, personal or instant for example.
/// </remarks>

Let me know what you think.

@sitkevich
Copy link
Author

Personally, I only need the true parameter, and solved it with reflection. But for a library, it's better to have an optional parameter. You can add a default method parameter
GetAsync( ..., show_previous_occurrences=true, ...)
If someone wants, they can change it.

Thanks for quick response.

Jericho added a commit that referenced this issue Jul 22, 2024
…tAsync to indicate whether to include previous occurrences
@Jericho
Copy link
Owner

Jericho commented Jul 22, 2024

I ended up adding an overload of the GetAsync method to avoid the confusion. Developers can use

  • Task<Meeting> GetAsync(long meetingId, string occurrenceId = null, CancellationToken cancellationToken = default) which will retrieve the meeting without past occurences. This matches the current behavior
  • Task<Meeting> GetAsync(long meetingId, string occurrenceId = null, bool includePreviousOccurrences = false, CancellationToken cancellationToken = default) which allows developers to specify whether they want past occurrences to be included or not.

@Jericho Jericho added Enhancement New feature or request and removed Breaking Change This change causes backward compatibility issue(s) labels Jul 23, 2024
@Jericho
Copy link
Owner

Jericho commented Jul 23, 2024

I am preparing to release 0.79.0 which will include this enhancement.

@Jericho Jericho closed this as completed Jul 23, 2024
Jericho added a commit that referenced this issue Jul 23, 2024
@Jericho
Copy link
Owner

Jericho commented Jul 23, 2024

🎉 This issue has been resolved in version 0.79.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
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants