Canvas - new components #19607
Canvas - new components #19607
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughAdds List Assignments and Update Assignment actions plus supporting Canvas app methods and constants; introduces cascading propDefinitions for accountId → userId → courseId → assignmentId, API helpers, and a package version bump. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as Action Module
participant App as Canvas App (propDefs + methods)
participant API as Canvas REST API
rect rgb(235,245,255)
note right of User: List Assignments flow
User->>Action: invoke list-assignments (accountId,userId,courseId)
Action->>App: this.canvas.listAssignments(userId, courseId)
App->>API: GET /api/v1/courses/{courseId}/assignments?user_id={userId}
API-->>App: assignments[]
App-->>Action: assignments[]
Action-->>User: return assignments (exports summary)
end
rect rgb(245,235,255)
note right of User: Update Assignment flow
User->>Action: invoke update-assignment (courseId,assignmentId,fields...)
Action->>Action: validate at least one field present
Action->>App: this.canvas.updateAssignment(courseId, assignmentId, payload)
App->>API: PUT /api/v1/courses/{courseId}/assignments/{assignmentId} (payload)
API-->>App: updatedAssignment
App-->>Action: updatedAssignment
Action-->>User: return API response (exports summary)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/canvas/actions/list-assignments/list-assignments.mjscomponents/canvas/actions/update-assignment/update-assignment.mjscomponents/canvas/canvas.app.mjscomponents/canvas/package.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/canvas/canvas.app.mjs
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/canvas/canvas.app.mjs
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: The Salesloft API list endpoints (listPeople, listCadences, listUsers, listAccounts) return arrays directly in the response body, not wrapped in a metadata object with a nested data property. The _makeRequest method correctly returns response.data which contains the arrays that can be mapped over directly in propDefinitions.
Applied to files:
components/canvas/canvas.app.mjs
🧬 Code graph analysis (2)
components/canvas/actions/list-assignments/list-assignments.mjs (1)
components/canvas/canvas.app.mjs (1)
assignments(60-63)
components/canvas/canvas.app.mjs (1)
components/canvas/actions/list-assignments/list-assignments.mjs (1)
assignments(42-46)
⏰ 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/canvas/package.json (1)
3-3: LGTM!The version bump from
0.6.0to0.7.0appropriately reflects the addition of new actions and API methods introduced in this PR.components/canvas/canvas.app.mjs (4)
1-2: LGTM!The axios import from
@pipedream/platformis correct for making authenticated HTTP requests.
74-87: LGTM!The
_baseUrland_makeRequesthelper methods correctly construct Canvas API requests with proper authentication using the OAuth access token.
118-126: LGTM!The
updateAssignmentmethod correctly uses the course-scoped endpoint (/courses/{courseId}/assignments/{assignmentId}), which differs from the user-scoped listing endpoint. This path difference is intentional per the Canvas API design.
6-72: The code is correct. The@pipedream/platformaxios returnsresponse.dataon success by default, so the list methods (listAccounts,listUsers,listCourses,listAssignments) return arrays directly that can be mapped over in propDefinitions. No changes needed.components/canvas/actions/update-assignment/update-assignment.mjs (1)
126-165: LGTM!The validation logic correctly ensures at least one update field is provided before making the API call. The payload construction properly maps props to Canvas API field names, and Pipedream's axios automatically excludes undefined values, ensuring only specified fields are sent in the update request.
components/canvas/actions/list-assignments/list-assignments.mjs (1)
1-52: LGTM!The list assignments action correctly implements the dependency chain (accountId → userId → courseId) and provides proper pluralization in the summary message. The implementation aligns well with the Canvas API method defined in
canvas.app.mjs.Note: Pagination concerns for the underlying
listAssignmentsmethod have been flagged in thecanvas.app.mjsreview.
lcaresia
left a comment
lcaresia
left a comment
There was a problem hiding this comment.
Hey, looks good, just have some improves to do. Moving to QA
| options: [ | ||
| "online_quiz", | ||
| "none", | ||
| "on_paper", | ||
| "discussion_topic", | ||
| "external_tool", | ||
| "online_upload", | ||
| "online_text_entry", | ||
| "online_url", | ||
| "media_recording", | ||
| "student_annotation", | ||
| ], |
There was a problem hiding this comment.
Move this options to a constants.mjs file.
| "points", | ||
| "not_graded", | ||
| ], | ||
| optional: true, |
There was a problem hiding this comment.
Move this options to a constants.mjs file.
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @components/canvas/common/constants.mjs:
- Around line 1-26: The SUBMISSION_TYPES constant is missing the
"basic_lti_launch" value used by Canvas; update the SUBMISSION_TYPES array in
components/canvas/common/constants.mjs to include "basic_lti_launch" (e.g., add
it alongside the other strings in the SUBMISSION_TYPES declaration) so any
submission processing that checks SUBMISSION_TYPES will recognize that type;
leave GRADING_TYPES and the default export unchanged.
♻️ Duplicate comments (1)
components/canvas/actions/update-assignment/update-assignment.mjs (1)
64-70: Previous concern about multiple submission types remains unresolved.The constants have been properly extracted (addressing prior feedback), but the Canvas API's
submission_typesfield accepts an array to support multiple submission types per assignment (e.g., both "online_upload" and "online_text_entry"). The current implementation still only allows selecting a single submission type.🔎 Recommended refactor to support multiple submission types
Change the prop to accept an array:
submissionType: { - type: "string", + type: "string[]", label: "Submission Type", - description: "The type of submission for the assignment", + description: "The types of submission allowed for the assignment. You can select multiple types.", options: constants.SUBMISSION_TYPES, optional: true, },Then simplify the payload construction in the run method:
- submission_types: this.submissionType - ? [ - this.submissionType, - ] - : undefined, + submission_types: this.submissionType,Based on Canvas API documentation,
submission_typesis designed to accept arrays like["online_text_entry", "online_url"].
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/canvas/actions/update-assignment/update-assignment.mjscomponents/canvas/common/constants.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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/canvas/actions/update-assignment/update-assignment.mjs (4)
1-15: LGTM!The imports correctly reference the new constants file (addressing previous review feedback), and the action metadata is properly structured with appropriate annotations for an update operation.
16-51: LGTM! Cascading prop dependencies are properly implemented.The cascading propDefinitions (accountId → userId → courseId → assignmentId) correctly enable progressive selection in the UI.
Minor note:
assignmentIdreceives bothcourseIdanduserId(lines 47-48), thoughuserIdmay be redundant since assignments are typically scoped to courses. This is not an issue and may be required by the Canvas app's internal implementation.
110-122: Excellent validation logic!Requiring at least one updatable field prevents no-op API calls and provides clear feedback to users. This is a good practice for update operations.
124-148: API call structure and field mapping look correct.The payload construction properly maps JavaScript naming conventions to Canvas API's snake_case format, and the assignment response is appropriately returned with a success message.
Note: Lines 132-136 correctly wrap the submission type in an array as required by the Canvas API, but this implementation is limited by the single-selection
submissionTypeprop (see earlier comment on lines 64-70).
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
Resolves #18719
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.