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

LTS [Marketplace]: Appwrite patch #10047

Merged
merged 5 commits into from
Jun 14, 2024
Merged

LTS [Marketplace]: Appwrite patch #10047

merged 5 commits into from
Jun 14, 2024

Conversation

akshaysasidrn
Copy link
Collaborator

@akshaysasidrn akshaysasidrn commented Jun 11, 2024

🛠️ Fixes

blank0537 and others added 3 commits June 11, 2024 20:32
* Appwrite plugin issue fixes and added features

* update test connection, removed duplicate operations and updated types
* update appwrite plugin version

* fix appwrite query
@akshaysasidrn akshaysasidrn changed the base branch from develop to lts-2.50 June 11, 2024 15:09
@akshaysasidrn
Copy link
Collaborator Author

/review

@akshaysasidrn
Copy link
Collaborator Author

akshaysasidrn commented Jun 11, 2024

Code Review Agent Run #37201c

  • AI Based Review: ✔️ Successful

Code Review Overview

  • Summary: The changes primarily focus on updating dependencies and enhancing the Appwrite plugin. Key updates include improved error handling, the introduction of new query operations, and performance optimizations by avoiding sequential execution of asynchronous operations.
  • Files: 8
  • Issue found: Total - 4, High importance - 4      See detailed feedback ->
  • Code change type: Refactoring, Performance Improvement, Dependency Updates
  • Unit tests added: False
  • Estimated effort to review (1-5, lower is better): 2

High-level Feedback

Ensure proper handling of asynchronous operations to avoid performance bottlenecks and unhandled promise rejections. Implement error handling for JSON parsing to prevent unhandled exceptions. Regularly update dependencies to maintain compatibility and performance.

Detailed Feedback

📄 plugins/packages/appwrite/lib/operations.ts
Issues: Total- 3, High importance- 3
Line 139-139 🔴 High importance - 1   
Line 148-148 🔴 High importance - 1   
Line 161-161 🔴 High importance - 1   
📄 plugins/packages/appwrite/lib/index.ts
Issues: Total- 1, High importance- 1
Line 13-13 🔴 High importance - 1   

AI Code Review powered by Bito Logo

collectionId: string,
documentId: string
): Promise<object> {
await db.deleteDocument(databaseId, collectionId, documentId);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bito Code Review Agent Run #37201c - 06/11/2024, 07:45 pm

🔴 High importance
Issue: The 'await' keyword is used inside a return statement. This can lead to unhandled promise rejections if the promise is rejected.
Fix: Store the result of the 'await' call in a variable and then return the variable.
Code suggestion

 @@ -147,7 +147,8 @@
  ): Promise<object> {
 -  await db.deleteDocument(databaseId, collectionId, documentId);
 +  const deleteResult = await db.deleteDocument(databaseId, collectionId, documentId);
 +  return { deleted: true, result: deleteResult };
  }

Copy link

@akshaysasidrn akshaysasidrn merged commit fc967db into lts-2.50 Jun 14, 2024
54 of 57 checks passed
@akshaysasidrn akshaysasidrn deleted the patch/appwrite branch June 14, 2024 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-cypress Cypress E2E action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appwrite plugin is not working (package is deprecated)
3 participants