-
Notifications
You must be signed in to change notification settings - Fork 93
Improve tasks and batches types, refactor related tests #1952
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
Conversation
WalkthroughThe changes update code samples, documentation, and tests to reflect a new API usage pattern for task and batch management in the Meilisearch JavaScript client, moving from direct client method calls to namespaced access via Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Tasks
participant Batches
User->>Client: client.tasks.getTasks(params)
Client->>Tasks: getTasks(params)
Tasks-->>Client: TasksResults
Client-->>User: TasksResults
User->>Client: client.batches.getBatches(params)
Client->>Batches: getBatches(params)
Batches-->>Client: BatchesResults
Client-->>User: BatchesResults
Assessment against linked issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1952 +/- ##
==========================================
- Coverage 99.03% 99.02% -0.02%
==========================================
Files 18 18
Lines 1449 1429 -20
Branches 305 303 -2
==========================================
- Hits 1435 1415 -20
Misses 14 14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@coderabbitai don't review |
Tip For best results, initiate chat on the files or code changes. @flevi29 Understood! I will not review this PR as requested. Feel free to tag me again if you need any assistance in the future. 🐰📝 |
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 (2)
src/types/shared.ts (1)
23-23
: Consider adding documentation for SafeOmit.While
SafeOmit
is a direct alias forOmit<T, K>
, it would be helpful to document why this alias exists (e.g., for consistency, future extensibility, or semantic clarity in the task/batch context).+/** Safe omission utility type for better semantic clarity in task/batch contexts. */ export type SafeOmit<T, K extends keyof T> = Omit<T, K>;
src/types/task_and_batch.ts (1)
227-227
: Consider documenting the commented field.The
batcherStoppedBecause
field is commented out. If this is planned for future implementation, consider adding a TODO comment explaining the timeline or requirements.Would you like me to help document this field or create an issue to track its implementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.code-samples.meilisearch.yaml
(3 hunks)README.md
(2 hunks)src/batch.ts
(1 hunks)src/task.ts
(2 hunks)src/types/shared.ts
(1 hunks)src/types/task_and_batch.ts
(6 hunks)src/types/types.ts
(1 hunks)tests/tasks-and-batches.test.ts
(1 hunks)tests/utils/meilisearch-test-utils.ts
(4 hunks)tests/utils/tasks-and-batches.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/batch.ts (1)
src/types/task_and_batch.ts (2)
TasksOrBatchesQuery
(63-79)BatchesResults
(235-235)
src/task.ts (1)
src/types/task_and_batch.ts (4)
TasksOrBatchesQuery
(63-79)TasksResults
(177-177)DeleteOrCancelTasksQuery
(87-90)EnqueuedTaskPromise
(156-162)
tests/tasks-and-batches.test.ts (3)
tests/utils/meilisearch-test-utils.ts (3)
getClient
(364-364)assert
(226-229)objectEntries
(352-352)src/types/task_and_batch.ts (1)
TasksOrBatchesQuery
(63-79)tests/utils/tasks-and-batches.ts (3)
possibleTaskTypes
(19-33)possibleTaskStatuses
(11-17)assert
(161-164)
tests/utils/meilisearch-test-utils.ts (3)
src/types/types.ts (1)
MeiliSearchErrorResponse
(754-761)src/types/task_and_batch.ts (3)
Task
(141-150)TaskStatus
(33-35)TaskType
(42-56)src/http-requests.ts (1)
T
(199-264)
tests/utils/tasks-and-batches.ts (3)
tests/utils/meilisearch-test-utils.ts (2)
objectKeys
(353-353)assert
(226-229)src/types/task_and_batch.ts (5)
TaskStatus
(33-35)TaskType
(42-56)EnqueuedTask
(97-103)Batch
(219-228)TasksResults
(177-177)src/types/shared.ts (1)
SafeOmit
(23-23)
🔇 Additional comments (32)
src/types/shared.ts (2)
20-21
: LGTM! Simple and effective utility type.The
PascalToCamelCase
utility correctly leverages TypeScript's built-inUncapitalize<S>
to convert PascalCase strings to camelCase. The Stack Overflow reference provides helpful context for the implementation.
25-27
: Well-designed utility types for flexible query parameters.The
OptionStarOr
andOptionStarOrList
types provide excellent support for APIs that accept wildcard values, specific values, or null. The tuple structure["*"]
for the list variant is particularly clever as it maintains type safety while allowing wildcard semantics.src/types/types.ts (1)
79-80
: LGTM! Good addition for configurable task waiting.The
defaultWaitOptions
property allows users to customize default polling behavior for task waiting operations. This is a thoughtful enhancement that improves the developer experience by avoiding repetitive option passing.src/batch.ts (2)
21-23
: LGTM! Clean simplification.Removing the intermediate variable and directly returning the awaited result makes the code more concise without affecting functionality. The method signature and return type remain unchanged.
26-28
: Good improvements for consistency and clarity.The parameter rename from
batchesQuery
toparams
provides better consistency with similar methods, and the direct return pattern improves readability. The method maintains the same functionality while being more concise.src/task.ts (5)
77-81
: LGTM! Consistent simplification pattern.The direct return of the awaited HTTP request result matches the simplification pattern used in
BatchClient
and improves code readability without changing functionality.
84-86
: LGTM! Clean and consistent.The simplification follows the same pattern as other methods in this refactor, maintaining functionality while improving code clarity.
89-90
: Excellent documentation improvement.The expanded documentation clearly explains that
waitForTask
uses polling withgetTask
, which helps developers understand the underlying mechanism and potential performance implications.
166-166
: LGTM! Cleaner method call formatting.The refactored method call with a single object argument improves readability and maintains the same functionality.
173-173
: LGTM! Consistent formatting improvement.The refactoring matches the pattern used in
cancelTasks
and improves code consistency across the class..code-samples.meilisearch.yaml (2)
102-131
: LGTM! Consistent API refactoring to namespaced task methods.The migration from direct client task methods (e.g.,
client.getTasks()
) to namespaced access (e.g.,client.tasks.getTasks()
) is consistently applied across all task-related code samples. This improves API organization and discoverability.
809-812
: LGTM! Consistent API refactoring to namespaced batch methods.The batch methods have been consistently updated to use the new
client.batches
namespace, maintaining the same pattern as the task methods refactoring.README.md (3)
534-553
: LGTM! Comprehensive documentation update for namespaced task API.The README correctly documents the new task API structure with the
client.tasks
namespace. The examples show both client-level task operations and the method signatures, providing clear guidance for developers.
558-565
: LGTM! Index-scoped task methods properly documented.The documentation correctly shows that index-scoped task methods also use the new namespaced pattern (
client.index('myIndex').tasks.*
), maintaining consistency with the client-level task methods.
584-591
: LGTM! Batch API documentation updated consistently.The batch methods documentation follows the same namespaced pattern as task methods, using
client.batches.*
which maintains API consistency.tests/tasks-and-batches.test.ts (5)
17-31
: Excellent type safety with sophisticated test value mapping.The
TestValues
andSimplifiedTestValues
types provide strong type safety for the parameterized tests, ensuring that test values align with theTasksOrBatchesQuery
interface. The type mapping approach is well-designed.
32-98
: Comprehensive test coverage with proper edge case handling.The test values record covers all query parameters from
TasksOrBatchesQuery
including:
- Standard values and wildcard (
"*"
) values for array parameters- Date strings and wildcard values for timestamp filters
- All possible task types and statuses from the type definitions
This ensures thorough testing of the API's filtering capabilities.
123-150
: Excellent integration testing of task lifecycle.The test properly validates the complete task lifecycle from enqueueing through completion, and includes verification of timeout and interval parameters for
waitForTask
. The spy-based verification of timeout parameters is a good testing practice.
187-213
: Sophisticated parameterized testing approach.The use of
describe.for
andtest.for
with theobjectEntries
utility creates comprehensive test coverage for all query parameters across bothgetTasks
andgetBatches
methods. This ensures that both APIs behave consistently with the same parameter types.
215-252
: Thorough validation of task cancellation and deletion operations.The tests properly validate that
cancelTasks
anddeleteTasks
return the correct task types ("taskCancelation"
and"taskDeletion"
respectively) and verify the structure of the task details includingmatchedTasks
,canceledTasks
/deletedTasks
, andoriginalFilter
properties.tests/utils/meilisearch-test-utils.ts (5)
102-143
: Well-designed assertion utilities for promise testing.The addition of the
RESOLVED
symbol andresolves
assertion method provides a good counterpart to the existingrejects
method. The implementation correctly catches and fails on any rejection, providing clear error messaging.
144-150
: Proper validation of MeiliSearch error response structure.The
isErrorResponse
assertion correctly validates the exact structure ofMeiliSearchErrorResponse
with all four required string properties (message
,code
,type
,link
). This aligns with the type definition in the relevant code snippets.
151-219
: Comprehensive and thorough task validation.The
isTask
assertion provides excellent validation coverage:
- Validates object property count (11-12 properties)
- Proper type checking for all task properties
- Validates enums against the actual type definitions using
objectKeys
- Handles nullable properties correctly
- Validates nested error objects when present
This is a robust validation that ensures tasks conform to the expected structure and types.
220-224
: Concise and effective successful task validation.The
isTaskSuccessful
method efficiently combines general task validation with specific success criteria (null error, "succeeded" status). This provides a convenient assertion for tests that expect successful task completion.
342-349
: Type-safe utility functions for object operations.The
objectKeys
andobjectEntries
functions provide type-safe alternatives to the standardObject.keys()
andObject.entries()
methods. TheobjectKeys
function is particularly useful for extracting typed keys from enum-like objects as seen in the task validation code.tests/utils/tasks-and-batches.ts (4)
11-33
: Excellent type-safe approach for defining constants!Using
objectKeys
with type parameters is a clever way to maintain type safety while creating arrays of possible values. This ensures the arrays stay in sync with the type definitions.
35-49
: Well-structured validation for EnqueuedTask!The assertion function properly validates all required fields with appropriate type checks and null handling for
indexUid
.
113-131
: Robust validation for optional fields!The validation logic for optional fields (
progressTrace
,writeChannelCongestion
,internalDatabaseSizes
) is well-implemented, properly handling bothundefined
and ensuring non-null object types when present.
58-61
:⚠️ Potential issueFix incorrect null handling for progress field.
The assertion doesn't match the error message or the type definition. According to the
Batch
type,progress
can benull
, but the current assertion will fail for null values.Apply this fix:
- extAssert( - typeof progress === "object", - "expected progress to be of type object or null", - ); + extAssert( + progress === null || typeof progress === "object", + "expected progress to be of type object or null", + );Likely an incorrect or invalid review comment.
src/types/task_and_batch.ts (3)
28-56
: Clean type transformation with excellent documentation!The use of
PascalToCamelCase
utility provides a maintainable way to align with JavaScript naming conventions while keeping consistency with the Rust backend. The documentation links and Rust type references are particularly helpful.
63-90
: Improved type safety for query parameters!The use of
Partial
with option wrapper types (OptionStarOr
,OptionStarOrList
) provides more precise typing for optional and nullable query parameters. The switch toSafeOmit
enhances type safety.
164-178
: Consistent and explicit pagination structure!The new
Results<T>
generic type provides a clear and consistent pagination structure for both tasks and batches, making the API more intuitive than the previous cursor-based approach.Also applies to: 235-235
Pull Request
Retry of #1943 due to merge conflicts.
What does this PR do?
Summary by CodeRabbit
Documentation
tasks
andbatches
namespaces on the client.New Features
defaultWaitOptions
property to the configuration type for customizable task waiting behavior.Refactor
Tests