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

Feature/sdk 94 add delete directory api endpoint to net #33

Merged

Conversation

logangingerich
Copy link
Contributor

Adds a Delete Directory API Endpoint and corresponding test.

@logangingerich logangingerich requested a review from a team as a code owner April 21, 2021 22:52
@linear
Copy link

linear bot commented Apr 21, 2021

@rohanjadvani
Copy link
Contributor

Could we add a test for this?

/// An optional token to cancel the request.
/// </param>
/// <returns>A Task wrapping a deleted WorkOS Directory record.</returns>
public async Task<Directory> DeleteDirectory(string id, CancellationToken cancellationToken = default)
Copy link
Contributor

Choose a reason for hiding this comment

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

This return type isn't correct, as we don't return a Directory from the endpoint.

We just return a HTTP 202 with no body, so the return type for this method should just be Task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxdeviant When using public async Task DeleteDirectory, I seem to get this error:

Since 'DirectorySyncService.DeleteDirectory(string, CancellationToken)' is an async method that returns 'Task', a return keyword must not be followed by an object expression. Did you intend to return 'Task<T>'?

Are there other adjustments needed to make this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

@logangingerich You can't return anything from a method that returns Task instead of Task<T>, so you'll need to discard the result of the API request:

await this.Client.MakeAPIRequestAsync<Directory>(request, cancellationToken)

instead of

return await this.Client.MakeAPIRequestAsync<Directory>(request, cancellationToken)

@logangingerich
Copy link
Contributor Author

Could we add a test for this?

My .NET knowledge is pretty limited and unfortunately there are no existing tests for deletion in this SDK to reference (there's no test for the delete connection endpoint). But I'm working on it 😅

@logangingerich logangingerich merged commit ec84fc2 into main May 4, 2021
@logangingerich logangingerich deleted the feature/sdk-94-add-delete-directory-api-endpoint-to-net branch May 4, 2021 20:15
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.

None yet

3 participants