GoDaddy - new components #18446
GoDaddy - new components #18446
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds a GoDaddy integration: client with HTTP helpers and propDefinitions, domain-related constants, four actions (check availability, list, renew, suggest), a polling source base plus a “New Domain Added” source with incremental fetching, and a package version bump with a new dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Action (List / Check / Renew / Suggest)
participant App as GoDaddy App Client
participant API as GoDaddy API
User->>Action: Invoke with props
Action->>App: call corresponding method (listDomains / checkDomainAvailability / renewDomain / suggestDomains) with params and `$`
App->>API: HTTP request to /v1/... (GET/POST)
API-->>App: 200 OK (payload)
App-->>Action: return payload
Action-->>User: Return result + export `$summary`
sequenceDiagram
autonumber
participant Timer as Source Timer
participant Source as New Domain Added Source
participant DB as Source DB
participant App as GoDaddy App Client
participant API as GoDaddy API
Timer->>Source: Trigger run()
Source->>DB: _getLastTs()
Source->>App: listDomains({ modifiedDate: lastTs, includes, statuses, statusGroups })
App->>API: GET /v1/domains?modifiedDate=...
API-->>App: 200 OK (domains page)
App-->>Source: domains
loop For each domain
Source->>Source: generateMeta(domain)
Source->>Source: $emit(domain, meta)
end
Source->>DB: _setLastTs(newestModifiedDate)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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: 9
🧹 Nitpick comments (3)
components/godaddy/actions/suggest-domains/suggest-domains.mjs (1)
68-82: Avoid sending undefined query params.Filter out undefined/null before calling the API.
Apply:
- const domains = await this.godaddy.suggestDomains({ - $, - params: { - query: this.query, - country: this.country, - city: this.city, - sources: this.sources, - tlds: this.tlds, - lengthMax: this.lengthMax, - lengthMin: this.lengthMin, - limit: this.limit, - waitMs: this.waitMs, - }, - }); + const rawParams = { + query: this.query, + country: this.country, + city: this.city, + sources: this.sources, + tlds: this.tlds, + lengthMax: this.lengthMax, + lengthMin: this.lengthMin, + limit: this.limit, + waitMs: this.waitMs, + }; + const params = Object.fromEntries( + Object.entries(rawParams).filter(([, v]) => v !== undefined && v !== null), + ); + const domains = await this.godaddy.suggestDomains({ $, params });components/godaddy/actions/check-domain-availability/check-domain-availability.mjs (1)
33-41: Filter undefined params for a clean request.Prevents
undefinedfrom being serialized in the query string.Apply:
- const response = await this.godaddy.checkDomainAvailability({ - $, - params: { - domain: this.domain, - checkType: this.checkType, - forTransfer: this.forTransfer, - }, - }); + const params = Object.fromEntries( + Object.entries({ + domain: this.domain, + checkType: this.checkType, + forTransfer: this.forTransfer, + }).filter(([, v]) => v !== undefined && v !== null), + ); + const response = await this.godaddy.checkDomainAvailability({ $, params });components/godaddy/common/constants.mjs (1)
244-253: Normalize DOMAIN_SUGGESTION_SOURCES to a single canonical set.Mixing upper/lowercase duplicates can confuse users; keep one set.
Apply:
const DOMAIN_SUGGESTION_SOURCES = [ - "CC_TLD", - "EXTENSION", - "KEYWORD_SPIN", - "PREMIUM", - "cctld", - "extension", - "keywordspin", - "premium", + "CC_TLD", + "EXTENSION", + "KEYWORD_SPIN", + "PREMIUM", ];
📜 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 (9)
components/godaddy/actions/check-domain-availability/check-domain-availability.mjs(1 hunks)components/godaddy/actions/list-domains/list-domains.mjs(1 hunks)components/godaddy/actions/renew-domain/renew-domain.mjs(1 hunks)components/godaddy/actions/suggest-domains/suggest-domains.mjs(1 hunks)components/godaddy/common/constants.mjs(1 hunks)components/godaddy/godaddy.app.mjs(1 hunks)components/godaddy/package.json(2 hunks)components/godaddy/sources/common/base.mjs(1 hunks)components/godaddy/sources/new-domain-added/new-domain-added.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#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/godaddy/package.json
🧬 Code graph analysis (7)
components/godaddy/actions/list-domains/list-domains.mjs (2)
components/godaddy/actions/suggest-domains/suggest-domains.mjs (1)
domains(69-82)components/godaddy/godaddy.app.mjs (1)
domains(13-15)
components/godaddy/sources/common/base.mjs (1)
components/godaddy/sources/new-domain-added/new-domain-added.mjs (1)
lastTs(42-42)
components/godaddy/actions/renew-domain/renew-domain.mjs (1)
components/godaddy/actions/check-domain-availability/check-domain-availability.mjs (1)
response(34-41)
components/godaddy/actions/check-domain-availability/check-domain-availability.mjs (1)
components/godaddy/actions/renew-domain/renew-domain.mjs (1)
response(27-33)
components/godaddy/godaddy.app.mjs (3)
components/godaddy/actions/list-domains/list-domains.mjs (1)
domains(49-59)components/godaddy/actions/suggest-domains/suggest-domains.mjs (1)
domains(69-82)components/godaddy/sources/new-domain-added/new-domain-added.mjs (2)
domains(43-43)results(45-54)
components/godaddy/actions/suggest-domains/suggest-domains.mjs (2)
components/godaddy/actions/list-domains/list-domains.mjs (1)
domains(49-59)components/godaddy/godaddy.app.mjs (1)
domains(13-15)
components/godaddy/sources/new-domain-added/new-domain-added.mjs (2)
components/godaddy/actions/list-domains/list-domains.mjs (1)
domains(49-59)components/godaddy/godaddy.app.mjs (2)
domains(13-15)results(119-121)
⏰ 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 (5)
components/godaddy/actions/renew-domain/renew-domain.mjs (1)
26-36: LGTM.Implementation aligns with app method; optional
periodwill be omitted from JSON.components/godaddy/package.json (1)
15-17: LGTM on dependency; confirm platform version compatibility.Looks good. Please confirm
@pipedream/platform@^3.1.0is required forDEFAULT_POLLING_SOURCE_TIMER_INTERVALand aligns with other GoDaddy components.Run to check for usage and ensure only one platform version is referenced:
components/godaddy/actions/suggest-domains/suggest-domains.mjs (2)
37-42: Verify propDefinition "tlds" exists in app.If
propDefinition: [godaddy, "tlds"]isn’t defined, component build will fail.Run:
12-17: Keepqueryoptional — docs mark it optional.GoDaddy’s /v1/domains/suggest documents
queryas optional, so leaveoptional: true.Likely an incorrect or invalid review comment.
components/godaddy/godaddy.app.mjs (1)
77-94: Consistency: useparams(notdata) for GET methodslistDomains, listTlds, checkDomainAvailability and suggestDomains are GET endpoints — callers/docs/examples must pass query args via
params. The client mapsdata → paramsas a safeguard, but align callers toparamsto avoid ambiguity. Location: components/godaddy/godaddy.app.mjs (methods ~lines 77–94).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/godaddy/godaddy.app.mjs (1)
12-17: Limit dynamic options page size to reduce API load.Add a small per-page cap (e.g., 50) to avoid fetching large batches in the UI picker.
Apply:
async options({ prevContext }) { const domains = await this.listDomains({ params: { marker: prevContext?.marker, + limit: 50, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/godaddy/actions/list-domains/list-domains.mjs(1 hunks)components/godaddy/common/constants.mjs(1 hunks)components/godaddy/godaddy.app.mjs(1 hunks)components/godaddy/sources/common/base.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/godaddy/common/constants.mjs
- components/godaddy/actions/list-domains/list-domains.mjs
- components/godaddy/sources/common/base.mjs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: LucBerge
PR: PipedreamHQ/pipedream#14080
File: components/nocodb/nocodb.app.mjs:133-133
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When implementing pagination with an offset, incrementing `args.params.offset` within the loop ensures correct tracking of the offset, particularly when a maximum count limit (`max`) is used.
Applied to files:
components/godaddy/godaddy.app.mjs
🧬 Code graph analysis (1)
components/godaddy/godaddy.app.mjs (3)
components/godaddy/actions/list-domains/list-domains.mjs (1)
domains(49-59)components/godaddy/actions/suggest-domains/suggest-domains.mjs (1)
domains(69-82)components/godaddy/sources/new-domain-added/new-domain-added.mjs (2)
domains(43-43)results(45-54)
⏰ 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: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/godaddy/godaddy.app.mjs (5)
12-17: Dynamic options query param fix looks good.Marker is correctly passed under params now.
62-64: Defaulting to production URL is correct. Consider optional OTE toggle.Optionally allow switching to OTE via a prop or env flag for testing.
65-77: Harden HTTP helper: set method default, headers, timeout, and guard GET params.Prevents subtle bugs and improves resilience.
Apply:
- _makeRequest({ - $ = this, - path, - ...opts - }) { - return axios($, { - url: `${this._baseUrl()}${path}`, - headers: { - "Authorization": `sso-key ${this.$auth.api_key}:${this.$auth.api_secret}`, - }, - ...opts, - }); - }, + _makeRequest({ + $ = this, + path, + method = "GET", + headers = {}, + ...opts + }) { + const config = { + method, + url: `${this._baseUrl()}${path}`, + headers: { + "Authorization": `sso-key ${this.$auth.api_key}:${this.$auth.api_secret}`, + "Accept": "application/json", + ...(method?.toUpperCase() !== "GET" ? { "Content-Type": "application/json" } : {}), + ...headers, + }, + timeout: 10000, + ...opts, + }; + // If callers mistakenly pass GET query under `data`, map it to `params` + if (config.method.toUpperCase() === "GET" && config.data && !config.params) { + config.params = config.data; + delete config.data; + } + return axios($, config); + },
96-103: URL‑encode domain in renew endpoint path.Avoids path issues with edge cases and IDNs.
Apply:
return this._makeRequest({ method: "POST", - path: `/domains/${domain}/renew`, + path: `/domains/${encodeURIComponent(domain)}/renew`, ...opts, });
111-135: Fix lostthisbinding and respect caller-provided limit in paginate.Current call to fn(...) loses context and will throw; also overwrites limit.
Apply:
- async *paginate({ - fn, params = {}, max, - }) { - params = { - ...params, - limit: 100, - }; + async *paginate({ + fn, params = {}, max, $ = this, + }) { + params = { + limit: params.limit ?? 100, + ...params, + }; let total, count = 0; do { - const results = await fn({ - params, - }); + const results = await fn.call(this, { + $, + params, + }); if (!results?.length) { return; } for (const item of results) { yield item; if (max && ++count >= max) { return; } } total = results?.length; params.marker = results[results.length - 1].domain; } while (total === params.limit); },
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/godaddy/actions/suggest-domains/suggest-domains.mjs (3)
42-53: Add basic bounds on lengthMin/lengthMax.Provide minimal validation to prevent invalid requests.
lengthMax: { type: "integer", label: "Length Max", description: "Maximum length of second-level domain", optional: true, + min: 1, }, lengthMin: { type: "integer", label: "Length Min", description: "Minimum length of second-level domain", optional: true, + min: 1, },
54-65: Constrain limit and waitMs to sane ranges.Prevents accidental huge payloads or excessive waits.
limit: { type: "integer", label: "Limit", description: "Maximum number of suggestions to return", optional: true, + min: 1, + max: 1000, }, waitMs: { type: "integer", label: "Wait Ms", description: "Maximum amount of time, in milliseconds, to wait for responses. If elapses, return the results compiled up to that point", optional: true, + min: 0, + max: 30000, },
83-84: Guard summary in case of non-array response.Minor safety to avoid
.lengthon unexpected payloads.- $.export("$summary", `Found ${domains.length} domain(s)`); - return domains; + const list = Array.isArray(domains) ? domains : []; + $.export("$summary", `Found ${list.length} domain(s)`); + return list;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/godaddy/actions/suggest-domains/suggest-domains.mjs(1 hunks)components/godaddy/godaddy.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-09-25T16:13:11.505Z
Learnt from: LucBerge
PR: PipedreamHQ/pipedream#14080
File: components/nocodb/nocodb.app.mjs:133-133
Timestamp: 2024-09-25T16:13:11.505Z
Learning: When implementing pagination with an offset, incrementing `args.params.offset` within the loop ensures correct tracking of the offset, particularly when a maximum count limit (`max`) is used.
Applied to files:
components/godaddy/godaddy.app.mjs
🧬 Code graph analysis (2)
components/godaddy/actions/suggest-domains/suggest-domains.mjs (2)
components/godaddy/godaddy.app.mjs (1)
domains(13-17)components/godaddy/actions/list-domains/list-domains.mjs (1)
domains(49-59)
components/godaddy/godaddy.app.mjs (3)
components/godaddy/actions/suggest-domains/suggest-domains.mjs (1)
domains(68-81)components/godaddy/actions/list-domains/list-domains.mjs (1)
domains(49-59)components/godaddy/sources/new-domain-added/new-domain-added.mjs (2)
domains(43-43)results(45-54)
⏰ 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: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (5)
components/godaddy/godaddy.app.mjs (5)
12-27: LGTM: options now pass marker under params.Pagination for the “Domain” prop will work as expected.
96-104: URL‑encode domain in renew path.Avoids path issues for edge cases.
return this._makeRequest({ method: "POST", - path: `/domains/${domain}/renew`, + path: `/domains/${encodeURIComponent(domain)}/renew`, ...opts, });
62-64: Provide a production fallback for base URL (avoid undefined/v1).If
this.$auth.api_urlisn’t set, requests will targetundefined/v1. Default to prod and trim trailing slashes._baseUrl() { - return `${this.$auth.api_url}/v1`; + const base = (this.$auth.api_url || "https://api.godaddy.com").replace(/\/+$/, ""); + return `${base}/v1`; },
65-77: Normalize requests and fix array query serialization; add headers/timeout.Adds method defaults, JSON headers, 10s timeout, GET data→params mapping, and a paramsSerializer that repeats array keys (required for GoDaddy).
- _makeRequest({ - $ = this, - path, - ...opts - }) { - return axios($, { - url: `${this._baseUrl()}${path}`, - headers: { - "Authorization": `sso-key ${this.$auth.api_key}:${this.$auth.api_secret}`, - }, - ...opts, - }); - }, + _makeRequest({ + $ = this, + path, + method = "GET", + headers = {}, + ...opts + }) { + const config = { + method, + url: `${this._baseUrl()}${path}`, + headers: { + "Authorization": `sso-key ${this.$auth.api_key}:${this.$auth.api_secret}`, + "Accept": "application/json", + ...(method?.toUpperCase() !== "GET" ? { "Content-Type": "application/json" } : {}), + ...headers, + }, + timeout: 10000, + // Ensure arrays are serialized as repeated keys: ?tlds=com&tlds=net + paramsSerializer: (params) => { + const usp = new URLSearchParams(); + for (const [k, v] of Object.entries(params || {})) { + if (v === undefined || v === null) continue; + if (Array.isArray(v)) { + for (const item of v) usp.append(k, item); + } else { + usp.append(k, v); + } + } + return usp.toString(); + }, + ...opts, + }; + // Support callers that mistakenly pass GET query under `data` + if (config.method.toUpperCase() === "GET" && config.data && !config.params) { + config.params = config.data; + delete config.data; + } + return axios($, config); + },
111-134: Blocker: lostthisbinding in paginate + hard‑overwrites limit.Unbound method will crash; also ignore caller’s limit. Bind
this, pass$, and only default limit when absent.- async *paginate({ - fn, params = {}, max, - }) { - params = { - ...params, - limit: 100, - }; + async *paginate({ + fn, params = {}, max, $ = this, + }) { + params = { + ...params, + limit: params.limit ?? 100, + }; let total, count = 0; do { - const results = await fn({ - params, - }); + const results = await fn.call(this, { + $, + params, + }); if (!results?.length) { return; } for (const item of results) { yield item; if (max && ++count >= max) { return; } } total = results?.length; - params.marker = results[results.length - 1].domain; + params.marker = results[results.length - 1]?.domain; } while (total === params.limit); },
|
/approve |
Resolves #16068
Summary by CodeRabbit