-
Couldn't load subscription status.
- Fork 5.5k
13401 components dart #18813
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
base: master
Are you sure you want to change the base?
13401 components dart #18813
Conversation
…date, and delete functionalities. Add new properties for document management and update package version to 0.2.0. Enhance existing actions for task management with version updates.
WalkthroughAdds doc CRUD actions and corresponding Dart client methods; refactors task actions to use flat create/update/list APIs with expanded props and parseObject utility; increases DEFAULT_LIMIT, removes TASK_PRIORITIES export, bumps package and several source versions, and makes minor formatting changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant DartApp as Dart App Client
participant DartAPI as Dart API
User->>Action: invoke create-doc / update-doc / delete-doc
Action->>Action: read props (dart, title/folder/text/docId)
alt Create Doc
Action->>DartApp: createDoc({ $, title, folder, text })
DartApp->>DartAPI: POST /public/docs
else Update Doc
Action->>DartApp: updateDoc({ $, data: { item: { id, title, folder, text } } })
DartApp->>DartAPI: PUT /public/docs/{id}
else Delete Doc
Action->>DartApp: deleteDoc({ $, docId })
DartApp->>DartAPI: DELETE /public/docs/{docId}
end
DartAPI-->>DartApp: API response
DartApp-->>Action: response
Action->>Action: $.export("$summary", ...)
Action-->>User: return response
sequenceDiagram
participant User
participant Action
participant DartApp as Dart App Client
participant DartAPI as Dart API
User->>Action: invoke create-task / update-task / find-or-create-task
Action->>Action: read expanded props (title, dartboard, parentId, assignees, tags, ...)
alt Find existing
Action->>DartApp: listTasks({ $, dartboard, title, ... })
DartApp->>DartAPI: GET /public/tasks?...
DartAPI-->>DartApp: results[]
DartApp-->>Action: results
Action->>Action: export found summary (results[0].id)
else Create or Update
Action->>DartApp: createTask / updateTask({ $, item: { id?, title, parentId, dartboard, type, status, ... } })
DartApp->>DartAPI: POST /public/tasks or PUT /public/tasks/{id}
DartAPI-->>DartApp: { item }
DartApp-->>Action: response
Action->>Action: $.export("$summary", ...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 1
🧹 Nitpick comments (2)
components/dart/actions/create-doc/create-doc.mjs (1)
35-49: Add defensive null check for response structure.The code assumes
response.item.idexists at line 47 without validation. If the API returns an unexpected response structure, this will cause a runtime error.Apply this diff to add a defensive check:
- $.export("$summary", `New doc successfully created with ID: ${response.item.id}`); + $.export("$summary", `New doc successfully created with ID: ${response?.item?.id || 'unknown'}`); return response;Alternatively, if the response structure is guaranteed by the API contract, consider verifying the response format:
const response = await this.dart.createDoc({ $, data: { item: { title: this.title, folder: this.folder, text: this.text, }, }, }); + + if (!response?.item?.id) { + throw new Error("Unexpected response format from createDoc API"); + } $.export("$summary", `New doc successfully created with ID: ${response.item.id}`); return response;components/dart/dart.app.mjs (1)
183-189: Consider refactoring for consistency and reduced coupling.The
updateDocmethod accessesopts.data.item.idto construct the path, which tightly couples it to the caller's data structure. In contrast,deleteDoc(lines 190-198) uses a destructureddocIdparameter, which is cleaner and more flexible.Consider refactoring
updateDocto matchdeleteDoc's approach:-updateDoc(opts = {}) { +updateDoc({ + docId, ...opts +}) { return this._makeRequest({ method: "PUT", - path: `/docs/${opts.data.item.id}`, + path: `/docs/${docId}`, ...opts, }); },Then update the caller in
components/dart/actions/update-doc/update-doc.mjs(line 43):const response = await this.dart.updateDoc({ + docId: this.docId, $, data: { item: { - id: this.docId, title: this.title, folder: this.folder, text: this.text, }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
components/dart/actions/create-doc/create-doc.mjs(1 hunks)components/dart/actions/create-task/create-task.mjs(1 hunks)components/dart/actions/delete-doc/delete-doc.mjs(1 hunks)components/dart/actions/find-or-create-task/find-or-create-task.mjs(1 hunks)components/dart/actions/update-doc/update-doc.mjs(1 hunks)components/dart/actions/update-task/update-task.mjs(1 hunks)components/dart/dart.app.mjs(4 hunks)components/dart/package.json(2 hunks)components/dart/sources/new-doc-created/new-doc-created.mjs(1 hunks)components/dart/sources/new-doc-updated/new-doc-updated.mjs(1 hunks)components/dart/sources/new-task-created/new-task-created.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/dart/actions/update-doc/update-doc.mjs (2)
components/dart/actions/create-doc/create-doc.mjs (1)
response(36-45)components/dart/actions/delete-doc/delete-doc.mjs (1)
response(24-27)
components/dart/actions/create-doc/create-doc.mjs (2)
components/dart/actions/delete-doc/delete-doc.mjs (1)
response(24-27)components/dart/actions/update-doc/update-doc.mjs (1)
response(43-53)
components/dart/actions/delete-doc/delete-doc.mjs (2)
components/dart/actions/create-doc/create-doc.mjs (1)
response(36-45)components/dart/actions/update-doc/update-doc.mjs (1)
response(43-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (16)
components/dart/package.json (1)
3-3: LGTM! Version bumps are appropriate.The package version bump from 0.1.0 to 0.2.0 aligns with the addition of new doc-related actions (create-doc, update-doc, delete-doc) and the dependency update is appropriate.
Also applies to: 16-16
components/dart/actions/find-or-create-task/find-or-create-task.mjs (1)
7-7: LGTM! Version bump for consistency.The version increment aligns with the broader package updates in this PR.
components/dart/actions/update-task/update-task.mjs (1)
7-7: LGTM! Version bump for consistency.The version increment aligns with the broader package updates in this PR.
components/dart/sources/new-doc-updated/new-doc-updated.mjs (2)
10-10: LGTM! Version bump aligns with new doc features.The version increment is appropriate for this source update.
20-27: Remove this review comment—the complete doc object is already emitted as the event payload.The
processEvent()method incommon/base.mjs(line 29) callsthis.$emit(item, meta), emitting the full doc object as the event payload. Consumers already have access to all doc fields includingupdatedByClientDuid(visible intest-event.mjs). ThegenerateMeta()method correctly creates only deduplication metadata (ID and timestamp), which is its intended purpose—not to duplicate the full payload.Likely an incorrect or invalid review comment.
components/dart/sources/new-task-created/new-task-created.mjs (2)
9-9: LGTM! Version bump aligns with package updates.The version increment is appropriate for this source update.
20-26: Creator ID is available in the emitted task object; verify if metadata inclusion is needed.The task object returned by
listTaskscontains the creator ID ascreatedByDuid. Since the base class (base.mjs) already emits the full task object viathis.$emit(item, meta), the complete creator information is accessible in the event payload. However, if explicit inclusion in metadata is a requirement per PR objectives, consider addingcreatedByDuidto thegenerateMeta()method for easier access.components/dart/sources/new-doc-created/new-doc-created.mjs (2)
9-9: LGTM! Version bump aligns with new doc features.The version increment is appropriate for this source update.
20-26: Review comment cannot be definitively verified without access to the PR objectives and full Dart API documentation.Based on examination of the codebase:
- The test event payload (
components/dart/sources/new-doc-created/test-event.mjs) containsduid,createdAt,drafterDuid, andeditorDuidsfields, but no explicitcreator,creatorId,createdBy, orauthorIdfield.- The
generateMetamethod currently uses available fields:doc.duidanddoc.createdAt.- Potentially related fields are
drafterDuid(nullable) oreditorDuids(array of user IDs).The review comment references "PR objectives" stating a required "creator id" prop should be included, but without access to the PR description or Dart API documentation, this claim cannot be confirmed. The test event structure suggests no dedicated creator field exists in the current API response.
Verify the PR objectives and Dart API response to confirm whether:
- A creator field exists in the full API documentation
drafterDuidoreditorDuids[0]should be treated as the creator- The requirement to include creator information is actually mandated
components/dart/actions/create-task/create-task.mjs (1)
7-7: LGTM! Version bump for consistency.The version increment aligns with the broader package updates in this PR.
components/dart/actions/create-doc/create-doc.mjs (1)
1-34: LGTM! Clean action structure.The action properly implements the create-doc functionality per PR objectives:
- Configurable doc name (title prop)
- Target folder specification (folder prop)
- Additional text content for doc body
The use of propDefinitions ensures consistency with the dart.app.mjs definitions.
components/dart/dart.app.mjs (2)
66-84: LGTM! Prop definition follows established patterns.The
docIdpropDefinition correctly maps API response fields (id→value,title→label) and follows the same pattern as other prop definitions in the file.
129-129: Verify the base URL and listDocs endpoint changes against Dart's current API.The endpoint changes cannot be conclusively verified from available documentation. While the base URL (
https://app.dartai.com/api/v0/public) appears consistent across the codebase, Dart's API structure uses OpenAPI and MCP specifications rather than traditional REST paths. Confirm that thelistDocspath change from/docsto/docs/listand thecreateDocpath (/docs) match Dart's current API specification before merging, as these are breaking changes to the integration.components/dart/actions/delete-doc/delete-doc.mjs (1)
14-31: LGTM! Implementation is clean and follows established patterns.The action correctly uses the
deleteDocmethod from the Dart app, properly passes parameters, and provides a clear success message.components/dart/actions/update-doc/update-doc.mjs (2)
14-41: LGTM! Props correctly configured for partial updates.All update properties (
title,folder,text) are optional, which aligns with the description stating "Any properties that are not specified will not be changed." Thetitleoverride tooptional: trueis appropriate since the base definition doesn't mark it optional, and this action supports partial updates.
42-57: I found the Dart AI service integration. Now let me check how the API client handles undefined values:Let me check how the API client handles undefined values when making requests:
Based on my verification, the original review comment about undefined/null handling can be resolved:
When JavaScript serializes objects to JSON, properties with undefined values are automatically omitted from the output. This means the current implementation in
update-doc.mjs(lines 48-50) correctly handles optional fields. Whentitle,folder, ortextare undefined, they are completely omitted from the JSON, which aligns with the API's documented behavior: "Any properties that are not specified will not be changed."The implementation is correct as-is. No changes are needed.
…task actions and document sources
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/configcat/configcat.app.mjs(1 hunks)components/dart/actions/create-task/create-task.mjs(1 hunks)components/dart/actions/find-or-create-task/find-or-create-task.mjs(1 hunks)components/dart/actions/update-task/update-task.mjs(1 hunks)components/dart/dart.app.mjs(3 hunks)components/dart/sources/new-doc-created/new-doc-created.mjs(1 hunks)components/dart/sources/new-doc-updated/new-doc-updated.mjs(1 hunks)components/dart/sources/new-task-created/new-task-created.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/configcat/configcat.app.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- components/dart/sources/new-doc-created/new-doc-created.mjs
- components/dart/actions/create-task/create-task.mjs
- components/dart/sources/new-doc-updated/new-doc-updated.mjs
- components/dart/sources/new-task-created/new-task-created.mjs
- components/dart/actions/update-task/update-task.mjs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (7)
components/dart/dart.app.mjs (6)
109-125: LGTM! New doc propDefinitions are well-defined.The new propDefinitions for
title,folder, andtextare clear, follow standard patterns, and have appropriate optional/required flags for doc operations.
176-182: LGTM! The createDoc method follows standard patterns.The
createDocmethod is implemented correctly with appropriate HTTP method and path.
190-198: LGTM! The deleteDoc method is well-implemented.The
deleteDocmethod uses clean destructuring and follows RESTful patterns correctly.
183-189: Remove the refactoring suggestion—the current implementation is correct.The nested
opts.data.item.idstructure is intentional and matches the actual calling convention. The action file (components/dart/actions/update-doc/update-doc.mjslines 43-46) callsupdateDocwith the nested structure:{ $, data: { item: { ... } } }. Refactoring to destructureddocIdwould break this existing call site. ThedeleteDocmethod has a different API contract and is not a valid comparison.Likely an incorrect or invalid review comment.
172-175: Path is consistent with established API design; no issues found.The
createTransactionendpoint at/public/transactions/createfollows the correct pattern. Analysis of the codebase shows this is intentional API design:
- Read operations (
listDocs,listTasks):/docs,/tasks— no/publicprefix- Write operations (
createTransaction,createDoc,updateDoc,deleteDoc):/public/*— all use/publicprefixThis is consistent, not an inconsistency. The
/publicprefix distinguishes mutation endpoints from read endpoints. Additionally,createTransactionhas no existing usages in the codebase, so there are no breaking changes to address.
129-129: The initial search returned results for the official Dart programming language APIs. However, the review context indicates this is a proprietary third-party service (based on the domain namesapp.itsdart.comandapp.dartai.com, and endpoints like "tasks", "dartboards", "users"). Let me search for information about this specific service:Confirm API migration timeline and update endpoint accordingly.
The domain change from itsdart.com to dartai.com reflects Dart's rebranding. API documentation is accessible at the new domain, and recent changelog entries confirm ongoing API improvements.
Before merging:
- Verify clients and integrations have been notified of the domain change
- Check if app.itsdart.com redirects to app.dartai.com or if both need support
- Confirm this aligns with the official Dart migration plan
components/dart/actions/find-or-create-task/find-or-create-task.mjs (1)
7-7: No action required—the path refactoring maintains functional compatibility.The commit refactored the base URL structure, moving
/publicfrom the base URL to individual method paths. The final API endpoint URL forcreateTransactionremains identical, so the existing action will continue to work without modification. The version bump appropriately documents this internal restructuring.
- Renamed and updated prop definitions for better clarity, including changing `dartboardId` to `dartboard`. - Introduced new optional properties: `tags`, `priority`, `size`, `status`, `type`, and `customProperties` to support enhanced task attributes. - Updated task creation and update methods to reflect new property structure and improved API paths. - Added utility function `parseObject` for better handling of input data formats. - Incremented version numbers for task-related actions to reflect changes.
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: 2
♻️ Duplicate comments (1)
components/dart/dart.app.mjs (1)
70-70: Correct the field name in docId propDefinition fromidtoduid.As flagged in a previous review, the
docIdpropDefinition should destructureduid: valueinstead ofid: valueto match the actual API response format (consistent withtaskIdon line 51).Apply this diff:
return results?.map(({ - id: value, title: label, + duid: value, title: label, }) => ({
🧹 Nitpick comments (1)
components/dart/dart.app.mjs (1)
124-124: Consider adding fallback for consistency.Unlike other similar prop definitions (lines 84, 94, 104, 114), the
typeoptions don't include a fallback|| []. While this might work iftypesis always defined, adding the fallback would make the code more defensive and consistent.Apply this diff:
- return types.map((type) => type); + return types?.map((type) => type) || [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/dart/actions/create-task/create-task.mjs(2 hunks)components/dart/actions/find-or-create-task/find-or-create-task.mjs(2 hunks)components/dart/actions/update-task/update-task.mjs(2 hunks)components/dart/common/constants.mjs(1 hunks)components/dart/common/utils.mjs(1 hunks)components/dart/dart.app.mjs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/dart/actions/find-or-create-task/find-or-create-task.mjs (2)
components/dart/actions/create-task/create-task.mjs (1)
response(117-137)components/dart/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/dart/actions/update-task/update-task.mjs (3)
components/dart/actions/create-task/create-task.mjs (1)
response(117-137)components/dart/actions/find-or-create-task/find-or-create-task.mjs (1)
response(129-149)components/dart/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/dart/actions/create-task/create-task.mjs (3)
components/dart/actions/find-or-create-task/find-or-create-task.mjs (1)
response(129-149)components/dart/actions/update-task/update-task.mjs (1)
response(124-145)components/dart/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (6)
components/dart/common/constants.mjs (1)
1-5: LGTM!The increase in
DEFAULT_LIMITfrom 50 to 100 and the removal ofTASK_PRIORITIESalign with the broader refactoring of task-related logic across the Dart actions.components/dart/common/utils.mjs (1)
1-24: LGTM!The
parseObjectutility provides robust handling for parsing JSON strings in various input formats (arrays, strings, objects) with graceful fallback on parse errors. This is used consistently across task actions to handle complex fields like assignees, tags, and custom properties.components/dart/actions/update-task/update-task.mjs (1)
1-148: LGTM!The refactoring from a transaction-based payload to a direct item-based payload is implemented correctly. The use of
parseObjectfor complex fields (assignees, tags, customProperties) is consistent with the pattern used in other task actions.components/dart/actions/find-or-create-task/find-or-create-task.mjs (1)
1-154: Otherwise, the refactoring looks good.Apart from the property reference issue, the migration to an item-based payload and the use of
parseObjectfor complex fields are consistent with other task actions.components/dart/actions/create-task/create-task.mjs (1)
1-141: Otherwise, LGTM!The refactoring to an item-based payload and the use of
parseObjectfor complex fields are implemented correctly and consistently with other task actions.components/dart/dart.app.mjs (1)
1-283: The broader API refactoring looks solid.The addition of new methods (
createTask,updateTask,getConfig,getDartboard,createDoc,updateDoc,deleteDoc) and expanded prop definitions provide comprehensive support for the new Dart AI API. The base URL change and new public endpoints align well with the updated action modules.
components/dart/actions/find-or-create-task/find-or-create-task.mjs
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…k.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/dart/actions/create-task/create-task.mjs(2 hunks)components/dart/actions/find-or-create-task/find-or-create-task.mjs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/dart/actions/find-or-create-task/find-or-create-task.mjs (3)
components/dart/actions/create-task/create-task.mjs (1)
response(117-137)components/dart/actions/update-task/update-task.mjs (1)
response(124-145)components/dart/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/dart/actions/create-task/create-task.mjs (3)
components/dart/actions/find-or-create-task/find-or-create-task.mjs (1)
response(129-149)components/dart/actions/update-task/update-task.mjs (1)
response(124-145)components/dart/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/dart/actions/find-or-create-task/find-or-create-task.mjs (2)
1-1: LGTM!The import of
parseObjectis correctly used throughout the file for handling complex fields likeassignees,tags, andcustomProperties.
24-29: Property reference issue has been resolved.The past review comment about the property reference mismatch has been addressed. The prop is now correctly named
titleand is properly referenced asthis.titleat line 119.components/dart/actions/create-task/create-task.mjs (3)
1-8: LGTM!The import, description, and version updates are consistent with the refactored API structure across task actions.
138-138: Typo fix confirmed, but verify ID field consistency.The typo "wiht" has been correctly fixed to "with". However, this line uses
response.idwhilefind-or-create-task.mjs(line 151) usesresponse.duid || response.id. Both files call the samecreateTaskmethod and should handle the response consistently.Consider aligning the ID extraction logic:
- $.export("$summary", `Successfully created task with ID: ${response.id}`); + $.export("$summary", `Successfully created task with ID: ${response.duid || response.id}`);Or verify that
response.idis the correct field and updatefind-or-create-task.mjsaccordingly.
117-137: LGTM!The task creation payload is well-structured with proper use of
parseObjectfor complex fields (assignees,tags,customProperties). The structure is consistent with other task actions in the PR.
Resolves #13401
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores