-
Notifications
You must be signed in to change notification settings - Fork 0
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/implement the client ap is #20
Conversation
- Removed the version comment from index.ts.
- Extracted FrappeResponse, TypedResponse, and ApiParams interfaces from index.ts to types.ts for better organization and modularity. - Updated index.ts to import these types from the new file.
…meters - Rearranged the order of properties in the GetDocListArgs interface for clarity. - Moved the orFilters property to the end of the parameters in the FrappeDB class to improve readability. - Ensured consistent handling of query parameters in the FrappeDB class.
- Added FrappeClient class to facilitate communication with the Frappe API, including methods for fetching, inserting, updating, and deleting documents. - Introduced type definitions for request parameters in a new types.ts file, enhancing type safety and clarity. - Each method includes error handling to manage API response errors effectively.
- Introduced FrappeFileDownload class to handle file download operations from a Frappe instance. - Implemented downloadFile method to fetch files using Axios, with error handling for download failures. - Added comprehensive documentation for the new class and its methods.
WalkthroughThis pull request reorganizes the codebase by centralizing API response and parameter type definitions. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrappeClient
participant Axios
User->>FrappeClient: Call API method (e.g., getList)
FrappeClient->>Axios: Send HTTP request
Axios-->>FrappeClient: Return response or error
FrappeClient-->>User: Deliver data or error details
sequenceDiagram
participant User
participant FrappeFileDownload
participant Axios
User->>FrappeFileDownload: Invoke downloadFile(fileURL)
FrappeFileDownload->>Axios: GET request to /api/method/download_file?fileURL=...
Axios-->>FrappeFileDownload: Return file data or error
FrappeFileDownload-->>User: Send downloaded file data or error
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Here's the code health analysis summary for commits Analysis Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/file/index.ts (1)
234-248
: Enhance error handling in the downloadFile methodWhile the implementation is functional, the error handling could be improved to match the level of detail provided in other methods like
uploadFile
.async downloadFile(fileURL: string): Promise<void> { - const response = await this.axios.get(`/api/method/download_file`, { - params: { - file_url: fileURL, - }, - }) - return response.data + return this.axios + .get(`/api/method/download_file`, { + params: { + file_url: fileURL, + }, + }) + .then((response) => response.data) + .catch((error) => { + throw { + ...error.response?.data, + httpStatus: error.response?.status, + httpStatusText: error.response?.statusText, + message: error.response?.data?.message ?? 'There was an error while downloading the file.', + exception: error.response?.data?.exception ?? '', + } as Error + }) }src/db/types.ts (1)
147-167
: Synchronize pagination and filtering property namesYou've introduced several new or repositioned parameters (
groupBy
,limit_start
,limit
,parent
,debug
, andorFilters
) to expand interface flexibility. However, there's an inconsistency with other parts of the code (e.g.limit
here vs.limit_page_length
elsewhere). Consider aligning these naming conventions across all files to maintain consistency and reduce confusion.src/client/types.ts (1)
1-59
: Add clarity to pagination optionsThe newly added interfaces (
GetListArgs
,GetCountArgs
,GetDocArgs
, andGetValueArgs
) increase the clarity and type safety of Frappe API calls. However, note the difference betweenlimit
insrc/db/types.ts
andlimit_page_length
here, which may lead to confusion for developers. Consider using consistent naming conventions across the codebase.src/client/index.ts (1)
1-436
: Refine request handling and naming consistency
- Verify that sending data via
params
in POST and PUT requests (e.g.setValue
,insert
,bulk_update
) is the intended approach, since standard practice is to include the payload in the request body (data
).- The
@ts-expect-error
annotation for a privateappURL
indicates it's intentionally unused. If it's not needed, consider removing it or referencing it if required.- Ensure pagination parameters match the definitions in
src/db/types.ts
(e.g.limit
,limit_start
vs.limit_page_length
). Consistency will help avoid confusion in future maintenance.Overall, the class layout is well-structured for multiple document operations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/call/index.ts
(1 hunks)src/call/types.ts
(1 hunks)src/client/index.ts
(1 hunks)src/client/types.ts
(1 hunks)src/db/index.ts
(1 hunks)src/db/types.ts
(1 hunks)src/file/index.ts
(1 hunks)src/index.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Mergify Merge Protections
🔇 Additional comments (9)
src/index.ts (1)
36-36
: Export addition looks good!The addition of
export * from './client'
appropriately exposes the new client module functionality from the main entry point of the SDK, matching the existing export pattern for other modules.src/call/types.ts (3)
1-4
: Well-defined base response typeThe
FrappeResponse
interface is correctly defined with clear JSDoc comments explaining its purpose.
6-12
: Well-implemented typed response interfaceThe
TypedResponse<T>
interface properly extends the baseFrappeResponse
interface, providing type safety for the message property through generics.
14-19
: Good API parameters type definitionThe
ApiParams
type is well-defined using the Record utility type to map string keys to various value types, with clear documentation explaining its purpose and type parameters.src/db/index.ts (2)
176-177
: Reordered destructuring looks goodThe restructured parameter destructuring now includes the new
groupBy
parameter and reorders the properties in a logical way.
182-186
: Consistent parameter mappingThe reordering of parameter assignments in the
params
object aligns with the destructured parameters order, and correctly handles the newgroup_by
parameter.src/file/index.ts (2)
219-231
: Well-documented new class for file downloadsThe
FrappeFileDownload
class is well-documented with clear JSDoc comments that explain its purpose and include usage examples.
232-232
: Concise constructor implementationThe constructor is correctly implemented to accept and store an Axios instance for use in the download method.
src/call/index.ts (1)
28-28
: Consolidating the type definitionsBy importing
ApiParams
andTypedResponse
from the newly created./types
module, you're enhancing modularity and maintainability. This is a solid improvement.
- Introduced a new .mergify.yml file to define rules for automatic merging and squashing of pull requests based on CI success and review approvals. - Configured conditions to prevent merging with specific labels and set up commit message templates for squashed merges.
- Added a new `handleRequest` function to streamline Axios requests with customizable error handling and response transformation. - Introduced `RequestHandlerOptions` interface to define options for the request handler, improving type safety and clarity. - Integrated detailed error handling for Axios errors, providing structured error responses aligned with Frappe's error format.
- Replaced direct Axios error handling with a centralized `handleRequest` function in FrappeAuth, FrappeCall, FrappeClient, FrappeDB, and FrappeFileUpload classes. - Updated error types from `Error` to `FrappeError` for consistency in error handling. - Enhanced documentation to reflect changes in error throwing and request handling methods. BREAKING CHANGE: - Errors thrown from API requests now return `FrappeError` instead of `Error`. - This may affect existing try/catch implementations that expect standard `Error` properties like `name` or `stack`. - Ensure your error handling logic correctly handles `FrappeError` properties (`httpStatus`, `httpStatusText`, `exception`, etc.). - The new `handleRequest` function replaces direct Axios calls in affected classes. - If you were relying on direct Axios responses or customizing Axios calls per method, you may need to update your logic.
- Changed string quotes in the downloadFile method of the FrappeFileDownload class from backticks to single quotes for consistency with the project's coding style.
… and FrappeDB classes - Changed method signatures for get, post, put, delete, and other methods in FrappeCall, FrappeClient, and FrappeDB classes from async to synchronous, aligning with the new request handling approach. - Updated method documentation to reflect the changes in return types and behavior.
…tion - Replaced the async implementation of downloadFile in FrappeFileDownload class with a call to handleRequest for improved error handling and response transformation. - Updated method parameters to align with the new request handling approach, enhancing consistency across file operations.
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Refactor
Chores