Skip to content

Conversation

@MacgyverH
Copy link

  • Introduces models, interfaces, and resources for Zoom Phone call queues, including CallQueue and CallQueueMember entities.
  • Adds ICallQueues and CallQueues resource classes with methods to retrieve call queue details, list call queues, and get queue members.
  • Updates ZoomClient and IZoomClient to expose the new CallQueues resource.
  • Registers new models in the JSON serializer context.

* Introduces models, interfaces, and resources for Zoom Phone call queues, including CallQueue and CallQueueMember entities.
* Adds ICallQueues and CallQueues resource classes with methods to retrieve call queue details, list call queues, and get queue members.
* Updates ZoomClient and IZoomClient to expose the new CallQueues resource.
* Registers new models in the JSON serializer context.
@MacgyverH MacgyverH marked this pull request as draft January 8, 2026 18:04
Moved CallQueuePhoneNumber and CallQueueSite to separate files and added XML documentation to CallQueueMember, CallQueuePhoneNumber, and CallQueueSite classes for improved clarity and maintainability.
@MacgyverH MacgyverH marked this pull request as ready for review January 8, 2026 18:24
Copy link
Owner

@Jericho Jericho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. A few very minor changes and also, let's debate/discuss whether these methods really deserve their own resource of whether they would fit in the existing 'Phone' resource.

}

/// <inheritdoc/>
public Task<PaginatedResponseWithToken<CallQueue>> GetAllAsync(string department = null, string cost_center = null, string site_id = null, int recordsPerPage = 30, string pagingToken = null, CancellationToken cancellationToken = default)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid underscore and other "special" characters when naming parameters, Can you rename to costCenter, siteId, etc.

In the mean time, I'll investigate if there's a way to configure StyleCop to let you know that such characters violate our naming convention.

/// <returns>
/// A paginated response of <see cref="CallQueue"/>.
/// </returns>
public Task<PaginatedResponseWithToken<CallQueue>> GetAllAsync(string department = null, string cost_center = null, string site_id = null, int recordsPerPage = 30, string pagingToken = null, CancellationToken cancellationToken = default);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above. Please avoid underscores.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah my apologies! I thought I had caught these as I was going through afterwards. I'll get these corrected!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Genuine question: is it preferable to create a new resource for call queues or would it be better to simply add these three methods under the existing Phone resource?

Meaning something along the lines of Phone.GetAllCallQueues, Phone.GetCallQueue, etc.

If we create a new resource for every grouping of methods, we'll end up with a super large number of resources. and how would we decide which methods deserve their own resource and which should be under the Phone resource?

My gut feeling tells me to keep all the phone related methods under the Phone resource but my mind is not made up, I'm open to a debate.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I thought about this a bit. You're right. These API endpoints are part of the Zoom Phone APIs, so I guess it would make more logical sense to move these into the Phone resource. I'll amend my code to follow that standard for now.

I think it would be good to discuss further however, I think it would be beneficial for maintainability, and ease of use for those new to ZoomNet to follow the existing structure of way the Zoom API is built out as seen here:

image https://developers.zoom.us/docs/api/#workplace

Your top-level resources following that would be:
Meetings, TeamChat, Phone, Mail, Calendar, Scheduler, and so on. I think it make sense to break these out further as the API library is expanded.

For instance, For the phone resource the official API has these "categories" within the phone api:
image

My thought is that you could then have Phone.Accounts, Phone.Alerts, Phone.CallLogs, Phone.CallQueues and so on, for example to get all of the CallQueues would be Phone.CallQueues.GetAllAsync(...)

I think adherence to that would make use of the library much easier to follow for anyone looking at the Zoom API documentation.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(...) so I guess it would make more logical sense to move these into the Phone resource. I'll amend my code to follow that standard for now.

Perfect, thanks. I'll wait for you to implement this change before merging.

My thought is that you could then have Phone.Accounts, Phone.Alerts, Phone.CallLogs, Phone.CallQueues and so on, for example to get all of the CallQueues would be Phone.CallQueues.GetAllAsync(...)

This is an even better suggestion: group methods into "sub-resources" to match the Zoom API documentation. It would be a major breaking change, which would cause some pain to developers to transition but it would absolutely make thinks more discoverable.

I will raise a separate issue to track this new feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants