-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Edusign - new components #19460
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
Edusign - new components #19460
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughAdds an Edusign integration: API client with propDefinitions, resource methods, four actions (create course/group/student, add student to course), a polling base, three polling sources (documents, groups, students) with test fixtures, and a package version/dependency bump. Changes
sequenceDiagram
autonumber
participant Source as Polling Source
participant App as edusign.app
participant DB as DB (state)
participant Runtime as Platform ($)
Note over Source,DB: processEvents start
Source->>DB: _getLastCreated()
DB-->>Source: lastCreated
Source->>App: listResources(updatedAfter=lastCreated)
App-->>Source: results[]
Source->>Source: filter results by DATE_CREATED > lastCreated
alt found new items
Source->>DB: _setLastCreated(maxCreated)
DB-->>Source: ack
loop each new item
Source->>Runtime: $.emit(event with generateMeta(item))
Runtime-->>Source: ack
end
else no new items
Source-->>Source: exit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
components/edusign/actions/add-student-to-course/add-student-to-course.mjs(1 hunks)components/edusign/actions/create-course/create-course.mjs(1 hunks)components/edusign/actions/create-group/create-group.mjs(1 hunks)components/edusign/actions/create-student/create-student.mjs(1 hunks)components/edusign/edusign.app.mjs(1 hunks)components/edusign/package.json(2 hunks)components/edusign/sources/common/base-polling.mjs(1 hunks)components/edusign/sources/new-document-created/new-document-created.mjs(1 hunks)components/edusign/sources/new-document-created/test-event.mjs(1 hunks)components/edusign/sources/new-group-created/new-group-created.mjs(1 hunks)components/edusign/sources/new-group-created/test-event.mjs(1 hunks)components/edusign/sources/new-student-created/new-student-created.mjs(1 hunks)components/edusign/sources/new-student-created/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/edusign/package.json
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/edusign/sources/new-document-created/new-document-created.mjscomponents/edusign/sources/new-student-created/new-student-created.mjscomponents/edusign/sources/new-group-created/new-group-created.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: 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/edusign/edusign.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/edusign/edusign.app.mjs
🧬 Code graph analysis (4)
components/edusign/sources/new-document-created/new-document-created.mjs (2)
components/edusign/sources/new-group-created/new-group-created.mjs (3)
lastCreated(15-15)maxCreated(16-16)meta(50-50)components/edusign/sources/new-student-created/new-student-created.mjs (4)
lastCreated(15-15)lastCreated(18-22)maxCreated(16-16)meta(45-45)
components/edusign/sources/common/base-polling.mjs (3)
components/edusign/sources/new-document-created/new-document-created.mjs (1)
lastCreated(15-15)components/edusign/sources/new-group-created/new-group-created.mjs (1)
lastCreated(15-15)components/edusign/sources/new-student-created/new-student-created.mjs (2)
lastCreated(15-15)lastCreated(18-22)
components/edusign/actions/create-course/create-course.mjs (3)
components/edusign/actions/add-student-to-course/add-student-to-course.mjs (1)
response(30-36)components/edusign/actions/create-group/create-group.mjs (1)
response(37-46)components/edusign/actions/create-student/create-student.mjs (1)
response(67-81)
components/edusign/sources/new-student-created/new-student-created.mjs (4)
components/zep/actions/get-threads/get-threads.mjs (1)
max(39-39)components/edusign/sources/new-document-created/new-document-created.mjs (3)
lastCreated(15-15)maxCreated(16-16)meta(45-45)components/edusign/sources/new-group-created/new-group-created.mjs (3)
lastCreated(15-15)maxCreated(16-16)meta(50-50)components/edusign/edusign.app.mjs (1)
students(48-52)
🪛 Gitleaks (8.30.0)
components/edusign/sources/new-document-created/test-event.mjs
[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 75-75: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/edusign/package.json (1)
3-17: Version bump and @pipedream/platform dependency look correctThe new version and dependency align with the new app client and actions that import from
@pipedream/platform, and no built-in Node modules are added here, which matches prior guidance. Based on learnings, this manifest change looks good.components/edusign/sources/new-group-created/test-event.mjs (1)
1-18: Group test event payload is consistent and usefulThe sample event includes the expected Edusign group fields (e.g.,
ID,NAME,STUDENTS) plus a lowercaseid, which should work well for sample emits and meta generation. No issues from a component perspective.components/edusign/sources/new-student-created/test-event.mjs (1)
1-29: Student test event payload looks appropriateThe sample student object contains the expected identifiers and name/email fields, uses
example.comaddresses, and appears safe and aligned with the rest of the Edusign integration. No changes needed.components/edusign/edusign.app.mjs (1)
95-176: HTTP client and resource helpers are clean and cohesiveCentralizing the base URL and auth header in
_makeRequestand layeringlist*/create*/addStudentToCoursehelpers on top gives a clear, reusable surface for the actions and sources. The method signatures are consistent and easy to work with.components/edusign/actions/add-student-to-course/add-student-to-course.mjs (1)
14-36: Double-check the request body shape foraddStudentToCourseHere you send:
this.edusign.addStudentToCourse({ $, courseId: this.courseId, data: { studentId: this.studentId, }, });Elsewhere in the integration (e.g., create-course / create-student), Edusign payload keys are UPPERCASE (
NAME,FIRSTNAME, etc.), so this camelCasestudentIdstands out.Please verify against the Edusign
PUT /v1/course/attendance/{courseId}docs that:
- The field name should indeed be
studentId(and not e.g.STUDENT_IDor an array field), and- The nesting under
datamatches what the app client and API expect.Adjust the key or structure if the docs specify something different.
components/edusign/actions/create-student/create-student.mjs (1)
14-84: Action is correctly implemented—password field is not required and auto-generation is expected behaviorThe Edusign API (
POST /v1/student) does not require a password in the request. The platform automatically generates credentials for new students and marks them withNEW_PASSWORD_NEEDED: 1in the response, requiring users to reset their password on first login. The current implementation is correct in omitting the password field.Regarding optional fields:
dateOfBirthandcountryare indeed optional in the Edusign API, but their absence from this action does not impact functionality. The action currently exposes the most practical fields (phone, photo, file number, tags, groups) and is fully operational.components/edusign/sources/common/base-polling.mjs (1)
1-36: LGTM!The base polling infrastructure is well-designed with proper state management, abstract method pattern, and lifecycle hooks. The deploy hook with a max limit of 10 ensures initial sample events are emitted without overwhelming the system.
components/edusign/sources/new-document-created/new-document-created.mjs
Show resolved
Hide resolved
components/edusign/sources/new-group-created/new-group-created.mjs
Outdated
Show resolved
Hide resolved
components/edusign/sources/new-group-created/new-group-created.mjs
Outdated
Show resolved
Hide resolved
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: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/edusign/sources/new-group-created/new-group-created.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/edusign/sources/new-group-created/new-group-created.mjs
🧬 Code graph analysis (1)
components/edusign/sources/new-group-created/new-group-created.mjs (5)
components/zep/actions/get-threads/get-threads.mjs (1)
max(39-39)components/edusign/sources/new-document-created/new-document-created.mjs (3)
lastCreated(15-15)maxCreated(16-16)meta(45-45)components/edusign/sources/new-student-created/new-student-created.mjs (4)
lastCreated(15-15)lastCreated(18-22)maxCreated(16-16)meta(45-45)components/edusign/edusign.app.mjs (1)
groups(30-34)components/userflow/actions/create-update-user/create-update-user.mjs (1)
group(224-227)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/edusign/sources/new-group-created/new-group-created.mjs (3)
4-13: Source definition and composition look consistent with other Edusign sourcesUsing
...common+...common.methods, a uniquekey,dedupe: "unique", and wiring insampleEmitmatches established patterns in this repo. Nothing blocking here.
39-47: Confirm intended checkpoint behavior when there are more thanmaxnew groupsYou call
this._setLastCreated(maxCreated)before applying themaxlimit, then potentially truncategroups:this._setLastCreated(maxCreated); if (max && groups.length > max) { groups.length = max; }This means the cursor advances to the most recently created group seen across all pages, even if you only emit the first
maxgroups. Any “older” new groups beyondmax(but still > previouslastCreated) will never be emitted on future runs. This mirrors the prior design where state is updated for both emitted and non‑emitted items to avoid retroactive emissions. Based on learnings, this may be intentional.Please confirm this is the desired behavior for Edusign groups. If instead you want to drain backlog over multiple runs, you’d need to either:
- Advance
lastCreatedonly to the last emitted group, or- Stop paginating once
groups.length >= max, so unprocessed items remain eligible in the next run.
54-60: Meta generation is clear and compatible withdedupe: "unique"Using
group.IDasid, a descriptive summary, andDate.parse(group.DATE_CREATED)fortsis straightforward and should work well with unique deduping.
GTFalcao
left a 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.
I made very minor suggestions for descriptions - but otherwise looks good to go!
Co-authored-by: Guilherme Falcão <[email protected]>
Co-authored-by: Guilherme Falcão <[email protected]>
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/edusign/actions/create-course/create-course.mjs (1)
14-70: Prop required/optional contract diverges from issue #13472 (description vs start/end, plus extra required fields).
Issue #13472 says description required and start/end optional, but heredescriptionis optional andstart,end,professorId,needStudentSignatureare effectively required. Please align to the issue or update the issue/spec to match the intended contract.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/edusign/actions/create-course/create-course.mjs(1 hunks)
⏰ 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: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
Resolves #13472
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.