-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Habitify - new components #19416
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
Habitify - new components #19416
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughReorganizes Habitify integration: moves and expands the app module, adds a shared polling base, introduces three polling sources (habit-logged, habit-status-updated, new-habit-created refactored), adds test-event samples, and updates package metadata and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as Polling Source
participant DB as Local DB
participant API as Habitify API
participant Evt as Event Emitter
Source->>DB: read persisted state (lastTs / previousStatuses)
DB-->>Source: return state or default
Source->>API: fetch data (habits / logs / statuses)
API-->>Source: return data array
Note over Source: compare with persisted state / filter results
loop per changed/new item
Source->>Evt: emit event (payload + metadata)
Evt-->>Source: ack (implicit)
end
Source->>DB: persist updated state (new lastTs / statuses)
DB-->>Source: persisted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/habitify/sources/new-habit-created/new-habit-created.mjs (1)
12-25: Fix lostthisbinding inrun(this.emitEventused as unbound callback)
habits.reverse().forEach(this.emitEvent);callsemitEventas a plain function, sothisinsideemitEventisundefinedin ESM strict mode andthis.$emitwill throw at runtime. Preserve the component context by wrapping in an arrow (or binding):async run() { const { data: habits } = await this.habitify.getHabits(); - habits.reverse().forEach(this.emitEvent); + habits.reverse().forEach((habit) => this.emitEvent(habit)); },
📜 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 (10)
components/habitify/app/habitify.app.mjs(0 hunks)components/habitify/habitify.app.mjs(1 hunks)components/habitify/package.json(2 hunks)components/habitify/sources/common/base-polling.mjs(1 hunks)components/habitify/sources/habit-logged/habit-logged.mjs(1 hunks)components/habitify/sources/habit-logged/test-event.mjs(1 hunks)components/habitify/sources/habit-status-updated/habit-status-updated.mjs(1 hunks)components/habitify/sources/habit-status-updated/test-event.mjs(1 hunks)components/habitify/sources/new-habit-created/new-habit-created.mjs(2 hunks)components/habitify/sources/new-habit-created/test-event.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- components/habitify/app/habitify.app.mjs
🧰 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/habitify/package.json
📚 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/habitify/habitify.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: 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/habitify/habitify.app.mjs
📚 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/habitify/sources/habit-status-updated/habit-status-updated.mjscomponents/habitify/sources/habit-logged/habit-logged.mjscomponents/habitify/sources/new-habit-created/new-habit-created.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/habitify/package.json (1)
3-16: Version bump / main entry look good; verify platform 3.x across componentsThe new
version,main, and@pipedream/platformdependency are consistent with the relocatedhabitify.app.mjsand new source modules. Just make sure all Habitify sources and actions have been exercised against@pipedream/platform@^3.1.1and that the new main path is recognized by your build / publish pipeline before releasing. Based on learnings, thanks for avoiding adding built‑in Node modules as dependencies.components/habitify/sources/new-habit-created/test-event.mjs (1)
1-29: Sample habit payload matches Habitify/habitsschemaThe test event mirrors the documented Habit properties (ids, dates, goal, recurrence, etc.), so it’s a good representative sample for the “New Habit Created” source. (docs.habitify.me)
components/habitify/sources/habit-logged/test-event.mjs (1)
1-7: Sample log payload matches Habitify/logs/:habit_idschemaThe test event correctly includes
id,value,created_date,unit_type, andhabit_id, matching the documented log object shape. (docs.habitify.me)components/habitify/sources/habit-logged/habit-logged.mjs (4)
1-11: LGTM!Imports and metadata are well-structured. The
dedupe: "unique"strategy paired withidfromgenerateMetaensures proper deduplication.
12-20: LGTM!Props correctly extend common.props and add the habitIds selection.
21-36: LGTM!Methods are well-implemented. The default to current time on first run prevents retroactive event emission, which is the expected behavior for polling sources. Based on learnings, this pattern ensures only new events are emitted.
54-56: Verify API logs endpoint sort order guarantees.The code assumes
data[data.length - 1]contains the most recent log, but the Habitify API documentation does not specify the sort order for the logs endpoint. If logs are returned unsorted or in descending order,maxTscould be set incorrectly.Consider computing the actual maximum timestamp across all fetched logs to ensure correct behavior regardless of API sort order.
components/habitify/sources/habit-status-updated/habit-status-updated.mjs
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: 2
♻️ Duplicate comments (3)
components/habitify/sources/habit-status-updated/habit-status-updated.mjs (2)
64-85: The issues flagged in the previous review comment are still present.The past review comment already identified three critical issues in this run method that remain unaddressed:
- API response handling:
data.statusmay be accessing the wrong property (boolean flag instead of the nested habit status string).- Filter logic: Empty array
[]is treated as "emit nothing" rather than "no filter" because!this.statusesevaluates tofalsefor[].- Mutation vs. immutability: Line 80 mutates the API response object directly (
data.habit_id = habitId) rather than creating a new event object.Please refer to the previous review comment for the complete suggested fix.
34-46: Fix API response handling in deploy hook to match actual Habitify response structure.The deploy hook has the same response-structure issue as the run method (flagged in the past comment). If the Habitify API returns a nested structure where
data.statusis a boolean success flag rather than the habit status string, thenpreviousStatuseswill be initialized with incorrect values (booleans instead of status strings like"completed","failed", etc.).This would cause every subsequent run to detect a "change" because you'd be comparing a boolean to a string, triggering false-positive emissions.
Based on the past review comment and Habitify docs, once the app's
_makeRequestreturnsresponse.data, apply a fix similar to the run method.components/habitify/sources/habit-logged/habit-logged.mjs (1)
59-62: Chronological sort before emission looks good and addresses earlier feedback.Sorting
logsbycreated_datebefore emitting ensures consumers see events in time order, which aligns with the earlier review suggestion. The use ofDate.parsefor normalization is appropriate given the likely string timestamps.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/habitify/sources/habit-logged/habit-logged.mjs(1 hunks)components/habitify/sources/habit-status-updated/habit-status-updated.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/habitify/sources/habit-logged/habit-logged.mjscomponents/habitify/sources/habit-status-updated/habit-status-updated.mjs
📚 Learning: 2025-01-29T22:59:38.825Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
Applied to files:
components/habitify/sources/habit-logged/habit-logged.mjs
🔇 Additional comments (1)
components/habitify/sources/habit-logged/habit-logged.mjs (1)
4-20: Source metadata andhabitIdsprop wiring look consistent with common base.The key/name/description/dedupe metadata and the
habitIdspropDefinition extension offcommon.props.habitifyall look aligned with standard Pipedream polling-source patterns. No issues from my side here.
jcortes
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.
Hi @michelle0927 lgtm! Ready for QA!
For Integration QA:
|
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
Resolves #9096
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.