Merge v0.3.0 #342
Merge v0.3.0 #342
Conversation
Merge pull request #295 from PinguApps/dev
added task issue type
Fixed typo in readme
*one test still fails, but i need a checkpoint to work form
Implemented Get team membership
Implemented update membership
…ver implementation
Implemented update team membership status
…ences Implemented get team preferences
Implemented Update preferences
…hip-from-server Removed create team membership from server
…-publishing-to-nuget Bumped to v 0.3.0
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary by Korbit AI
Code Execution Comments
- Add
UpdateTeamMembershipStatustoITeamsClientand include session parameter for authentication consistency. - Correct
JsonPropertyNameforMembershipIdand ensureTeamIdvalidation aligns with requirements.
Code Health Comments
- Use specific exception types and log exceptions for clearer error handling and debugging.
- Fix typos and align
Emailvalidation with intended behavior for clarity. - Add validation to limit
Rolesto 100 entries and resolveTeamIdvalidation discrepancies.
Files scanned
| File Path | Reviewed |
|---|---|
| src/PinguApps.Appwrite.Client/Clients/AccountClient.cs | ✅ |
| src/PinguApps.Appwrite.Client/Clients/AppwriteClient.cs | ✅ |
| src/PinguApps.Appwrite.Client/Clients/IAppwriteClient.cs | ✅ |
| src/PinguApps.Appwrite.Client/Clients/ITeamsClient.cs | ✅ |
| src/PinguApps.Appwrite.Client/Clients/SessionAwareClientBase.cs | ✅ |
| src/PinguApps.Appwrite.Client/Clients/TeamsClient.cs | ✅ |
| src/PinguApps.Appwrite.Client/Internals/ITeamsApi.cs | ✅ |
| src/PinguApps.Appwrite.Client/ServiceCollectionExtensions.cs | ✅ |
| src/PinguApps.Appwrite.Server/Clients/AccountClient.cs | ✅ |
| src/PinguApps.Appwrite.Server/Clients/AppwriteClient.cs | ✅ |
| src/PinguApps.Appwrite.Server/Clients/IAppwriteClient.cs | ✅ |
| src/PinguApps.Appwrite.Server/Clients/ITeamsClient.cs | ✅ |
| src/PinguApps.Appwrite.Server/Clients/TeamsClient.cs | ✅ |
| src/PinguApps.Appwrite.Server/Clients/UsersClient.cs | ✅ |
| src/PinguApps.Appwrite.Server/Internals/ITeamsApi.cs | ✅ |
| src/PinguApps.Appwrite.Server/ServiceCollectionExtensions.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Constants.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Enums/ValidationContext.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/BaseRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/CreateTeamMembershipRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/CreateTeamRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/DeleteTeamMembershipRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/DeleteTeamRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/GetTeamMembershipRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/GetTeamPreferencesRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/GetTeamRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/ListTeamMembershipsRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/ListTeamsRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/TeamIdBaseRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/TeamMembershipIdBaseRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/UpdateMembershipRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/UpdateNameRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/UpdatePreferencesRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/UpdateTeamMembershipStatusRequest.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/CreateTeamMembershipRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/CreateTeamRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/DeleteTeamMembershipRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/DeleteTeamRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/GetTeamMembershipRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/GetTeamPreferencesRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/GetTeamRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/ListTeamMembershipsRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/ListTeamsRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/TeamIdBaseRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/TeamMembershipIdBaseRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/UpdateMembershipRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/UpdateNameRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/UpdatePreferencesRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Teams/Validators/UpdateTeamMembershipStatusRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Requests/Users/Validators/UserIdBaseRequestValidator.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Responses/Membership.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Responses/MembershipsList.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Responses/Team.cs | ✅ |
| src/PinguApps.Appwrite.Shared/Responses/TeamsList.cs | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Naming ✅ Database Operations ✅ Documentation ✅ Logging ✅ Error Handling ✅ Systems and Environment ✅ Objects and Data Structures ✅ Tests ❌ Readability and Maintainability ✅ Asynchronous Processing ✅ Design Patterns ✅ Third-Party Libraries ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
| /// <summary> | ||
| /// <para>Invite a new member to join your team. Provide an ID for existing users, or invite unregistered users using an email or phone number. If initiated from a Client SDK, Appwrite will send an email or sms with a link to join the team to the invited user, and an account will be created for them if one doesn't exist. If initiated from a Server SDK, the new member will be added automatically to the team.</para> | ||
| /// <para>You only need to provide one of a user ID, email, or phone number. Appwrite will prioritize accepting the user ID > email > phone number if you provide more than one of these parameters.</para> | ||
| /// <para>Use the url parameter to redirect the user from the invitation email to your app. After the user is redirected, use <see cref="UpdateTeamMembershipStatus(UpdateTeamMembershipStatusRequest)"/> to allow the user to accept the invitation to the team.</para> |
There was a problem hiding this comment.
Missing UpdateTeamMembershipStatus method in ITeamsClient interface.
The CreateTeamMembership method's documentation references a method called UpdateTeamMembershipStatus, but this method is not defined in the ITeamsClient interface. To ensure full functionality and consistency with the documentation, please add the UpdateTeamMembershipStatus method to the interface.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| catch (Exception e) | ||
| { | ||
| return e.GetExceptionResponse<TeamsList>(); | ||
| } |
There was a problem hiding this comment.
Log exceptions before returning responses.
Consider logging the exception before returning the exception response. This will help with debugging and monitoring of errors. You can add a log statement like _logger.LogError(e, "Error occurred while listing teams"); before returning the exception response.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| [Patch("/teams/{teamId}/memberships/{membershipId}/status")] | ||
| Task<IApiResponse<Membership>> UpdateTeamMembershipStatus(string teamId, string membershipId, UpdateTeamMembershipStatusRequest request); |
There was a problem hiding this comment.
Missing session parameter in UpdateTeamMembershipStatus method.
The UpdateTeamMembershipStatus method is missing the session parameter, which is required for authentication. This could lead to functionality issues when trying to update team membership status. Please add the session parameter to ensure consistent authentication across all team-related operations.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| RuleFor(x => x.Email) | ||
| .NotEmpty() | ||
| .WithMessage("Email must either be null or a non empty string.") | ||
| .EmailAddress() | ||
| .When(x => x.Email is not null) | ||
| .WithMessage("Invalid email format."); |
There was a problem hiding this comment.
Inconsistent email validation rules.
The validation for the Email property seems inconsistent. The comment suggests that the email can be null, but the NotEmpty() rule contradicts this. Consider removing the NotEmpty() rule and adjusting the validation to allow null values while still validating non-null values as email addresses. This change will ensure that the validation aligns with the intended behavior described in the comment.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| [JsonPropertyName("teamId")] | ||
| [SdkExclude] | ||
| public string MembershipId { get; set; } = string.Empty; |
There was a problem hiding this comment.
Incorrect JsonPropertyName attribute for MembershipId.
There seems to be a mistake in the JsonPropertyName attribute for the MembershipId property. It's currently set to 'teamId', which is incorrect and could lead to confusion and incorrect serialization/deserialization. Please correct it to 'membershipId'.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| .Matches("^[a-zA-Z0-9][a-zA-Z0-9._-]{0,35}$") | ||
| .WithMessage("TeamId can only contain a-z, A-Z, 0-9, period, hyphen, and underscore, and can't start with a special char. Max length is 36 chars."); |
There was a problem hiding this comment.
Discrepancy between TeamId validation regex and error message.
There's a discrepancy between the TeamId validation regex and the error message. The regex allows for up to 36 characters ({0,35}$ means 0 to 35 characters after the first character), but the error message states the max length is 35 characters. Consider updating either the regex to {0,34}$ or the error message to state 'Max length is 36 chars' for consistency.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| /// <summary> | ||
| /// Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about <see href="https://appwrite.io/docs/permissions">roles and permissions</see>. Maximum of 100 roles are allowed, each 32 characters long. | ||
| /// </summary> | ||
| [JsonPropertyName("roles")] | ||
| public List<string> Roles { get; set; } = []; |
There was a problem hiding this comment.
Add validation for the maximum number of roles in CreateTeamMembershipRequest.
The Roles property in CreateTeamMembershipRequest is currently implemented as a List<string> without any size restrictions. However, the comment specifies that a maximum of 100 roles are allowed. Consider adding a validation check to ensure that the number of roles doesn't exceed 100, as per the specified limit.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| /// <summary> | ||
| /// The reuqest for removing a user from a team | ||
| /// </summary> |
There was a problem hiding this comment.
Typo in class summary comment.
There's a typo in the class summary comment. The word 'reuqest' should be 'request'. Correct documentation is important for maintaining code clarity and preventing potential misuse. Please update the comment to read: '/// The request for removing a user from a team'.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| /// </summary> | ||
| [JsonPropertyName("teamId")] | ||
| [SdkExclude] | ||
| public string TeamId { get; set; } = string.Empty; |
There was a problem hiding this comment.
Add validation for the TeamId property in ListTeamMembershipsRequest.
The TeamId property in the ListTeamMembershipsRequest class lacks validation to ensure it meets the specified requirements (valid characters, length, etc.). Consider adding validation logic, either through a custom validator in the ListTeamMembershipsRequestValidator class or by using data annotations on the property itself. This will help prevent invalid team IDs from being processed, improving the robustness of the application.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| public string GetCurrentSessionOrThrow() | ||
| { | ||
| return GetCurrentSession() ?? throw new Exception(ISessionAware.SessionExceptionMessage); | ||
| } |
There was a problem hiding this comment.
Use a more specific exception type in GetCurrentSessionOrThrow method.
In the GetCurrentSessionOrThrow method, consider using a more specific exception type instead of the generic Exception. For example, you could use InvalidOperationException or create a custom exception type like SessionNotFoundException. This would provide more clarity about the nature of the error and allow for more precise error handling. Here's an example:
public string GetCurrentSessionOrThrow()
{
return GetCurrentSession() ?? throw new InvalidOperationException(ISessionAware.SessionExceptionMessage);
}Or with a custom exception:
public class SessionNotFoundException : Exception
{
public SessionNotFoundException(string message) : base(message) {}
}
public string GetCurrentSessionOrThrow()
{
return GetCurrentSession() ?? throw new SessionNotFoundException(ISessionAware.SessionExceptionMessage);
}Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
Bumped to v 0.3.0
Description by Korbit AI
What change is being made?
Integrate a new Teams service into the Appwrite client and server SDKs, update the version to 0.3.0, and enhance documentation and validation for team-related requests.
Why are these changes being made?
These changes introduce a comprehensive Teams service, allowing users to manage team memberships and roles, which enhances the functionality of the SDKs. The update to version 0.3.0 reflects these significant additions. The changes also improve code maintainability and usability by adding detailed documentation and validation for team-related operations.