search

Jericho / ZoomNet

.NET client library for the Zoom.us REST API v2
MIT License
69 stars 46 forks source link

Added custom attributes support to User update. #251

Closed cdmdotnet closed 1 year ago

cdmdotnet commented 1 year ago
Jericho commented 1 year ago

Thanks. This seems like a worthwhile improvement but please make sure you raise an issue to describe the problem you want to solve, the bug you are fixing, the improvement you are recommending, etc. That's the best place for us to discuss. Also, the release notes I generate for each release are based on issues and therefore, without an issue, developers who download future releases of this library will not know about these changes. Also you won't get the recognition that you deserve! In this case, it would be great to have an issue titled something like "Unable to update custom attributes when updating a User" and then when you submit your PR you can say "this PR resolves #xyz" where xyz is the issue number.

Also, for future reference, I would much prefer that each improvement/bug fix get their own issue+PR so they can be debated and accepted/rejected independently. In this case, a separate issue titled something like "Allow OAuth connection without an access token". I definitely want to test this scenario and engage in a conversation with you about this, because I didn't even know this was possible! Did you find out about this by trial and error? Did Zoom update their documentation and I missed this change?

Jericho commented 1 year ago

After some investigation, it turns out that I did know about null access token! I even have some code in the OAuthConnectionInfo constructor to deal with such an empty token. However, as you have discovered, the parameter validation in the OAuthConnectionInfo constructor overrules this logic and forces developers to provide a token.

cdmdotnet commented 1 year ago

Relates to issues #252

cdmdotnet commented 1 year ago

Relates to #253

cdmdotnet commented 1 year ago

Regarding how I found this works, I did a quick step through/step over to by pass the null reference check as normal OAuth flows allow for saving just the refreshtoken... accesstokens are supposed to be transient, so it just made sense that it would work.