[Components] opensrs - new components #15454
Conversation
jcortes
added
action
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a comprehensive set of modules for the OpenSRS domain management platform. The changes include new actions for domain registration, transfer initiation, and DNS record updates, along with supporting utility functions, constants, and polling sources. The implementation provides a structured approach to interacting with the OpenSRS API, covering domain lifecycle events such as registration, transfer status changes, and DNS zone modifications. Changes
Assessment against linked issues
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (12)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (5)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
components/opensrs/sources/new-transfer-status/new-transfer-status.mjs (2)
20-26: 🛠️ Refactor suggestionConsistent error handling needed for Date.parse().
Similar to new-domain-registration.mjs, add validation for invalid event_date values.
16-19: 🛠️ Refactor suggestionAdd null checks in isResourceRelevant.
Similar to new-domain-registration.mjs, add null safety checks.
🧹 Nitpick comments (16)
components/opensrs/opensrs.app.mjs (2)
70-85: Provide additional error context
When throwing errors, consider including more contextual information (e.g., error codes, request parameters) to facilitate faster debugging.
86-109: Implement retry logic for transient network failures
Your request function gracefully handles errors from the OpenSRS API but does not currently retry on network or rate-limit errors. Adding a retry strategy may improve reliability.components/opensrs/actions/update-dns-records/update-dns-records.mjs (2)
284-337: Add validations for record content
While splitting records into multiple types is helpful, also consider validating subdomains, IP formats, and text record length to avoid malformed DNS entries.
380-396: Offer a post-update verification
After updating DNS records, it can be helpful to query DNS to confirm the changes. Would you like assistance adding a verification step or a new action for DNS lookup?components/opensrs/actions/register-domain/register-domain.mjs (3)
19-28: Store reseller credentials in an encrypted field
Currently,regUsernameandregPasswordare plain strings. Consider using a more secure storage pattern if the platform supports encrypted fields, ensuring credentials aren’t exposed in logs.
233-248: Clarify usage for 'customTechContact'
If the user chooses to override the default reseller’s contact with custom data, ensure the UI or docs highlight the implications of using that override.
351-509: Consider modularizing large JSON object creation
YourgetJsonBody()method is lengthy. For readability and easier maintenance, partition the contact creation blocks into smaller helper functions (e.g.,buildOwnerContact(),buildAdminContact(), etc.).components/opensrs/sources/common/events.mjs (1)
1-13: Add JSDoc documentation for event types.While the event types are self-descriptive, adding JSDoc documentation for each constant would improve maintainability by explaining when each event type is triggered and its significance in the domain management lifecycle.
Example documentation format:
+/** + * Event types for domain management lifecycle. + * @readonly + * @enum {string} + */ export default { + /** Triggered when a domain entity is initially created */ CREATED: "CREATED", + /** Triggered when a domain registration is completed */ REGISTERED: "REGISTERED", // ... add documentation for remaining eventscomponents/opensrs/common/constants.mjs (3)
1-2: Document the rationale for DEFAULT_LIMIT value.Add a comment explaining why 100 was chosen as the default limit. This helps future maintainers understand if this is an API limitation or a performance optimization choice.
29-33: Consider making DEFAULT_NAMESERVER_LIST configurable.Hardcoding nameserver values might not be suitable for all environments or users. Consider:
- Moving these to environment variables or configuration
- Adding documentation about these nameservers
- Making this list customizable through app settings
4-27: Add TypeScript-style JSDoc for better type safety.The enums would benefit from TypeScript-style JSDoc annotations to provide better IDE support and type checking.
Example:
+/** + * @readonly + * @enum {string} + */ const REGISTRY_TYPE = { NEW: "new", TRANSFER: "transfer", SUNRISE: "sunrise", };components/opensrs/sources/new-transfer-status/new-transfer-status.mjs (1)
1-29: Consider extracting common metadata generation logic.Both new-domain-registration.mjs and new-transfer-status.mjs share similar metadata generation patterns. Consider creating a common utility function.
Example approach:
// In common/utils.mjs export const generateEventMeta = (resource, summaryPrefix = "Event") => { const timestamp = Date.parse(resource.event_date); if (isNaN(timestamp)) { throw new Error(`Invalid event_date format: ${resource.event_date}`); } return { id: resource.event_id, summary: `${summaryPrefix}: ${resource.objectData.domain_name}`, ts: timestamp, }; };Then in both source files:
import { generateEventMeta } from "../../common/utils.mjs"; // ... generateMeta(resource) { return generateEventMeta(resource, "New Transfer Status"); },components/opensrs/sources/common/polling.mjs (2)
21-27: Consider implementing thesortFnmethod.The
sortFnmethod is currently empty. If sorting is not needed, consider adding a comment explaining why, or implement a default sorting behavior.sortFn() { - return; + // Default implementation: no sorting required + return 0; }
81-96: Simplify object construction using Object.fromEntries.The current implementation uses reduce for object construction. Consider using Object.fromEntries for better readability.
- const data = resourceItems.reduce((acc, item) => - Object.assign(acc, { - [item["@_key"]]: item.value, - }), {}); + const data = Object.fromEntries( + resourceItems.map(item => [item["@_key"], item.value]) + ); - const objectDataItems = objectData?.dt_assoc?.item.reduce((acc, item) => - Object.assign(acc, { - [item["@_key"]]: item.value, - }), {}); + const objectDataItems = objectData?.dt_assoc?.item + ? Object.fromEntries( + objectData.dt_assoc.item.map(item => [item["@_key"], item.value]) + ) + : {};components/opensrs/common/utils.mjs (1)
36-51: Improve default propsMapper documentation.The default propsMapper function uses
thiscontext which isn't immediately clear. Add JSDoc to explain the expected context and usage.+/** + * @param {Function} propsMapper - Function to map properties + * @param {Object} propsMapper.this - Context object containing property values + * @param {string} propsMapper.this[`${prefix}name`] - Name property from context + * @returns {Object} Mapped properties object + */ propsMapper = function propsMapper(prefix) { const { [`${prefix}name`]: name } = this; return { name, }; },components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs (1)
154-155: Enhance the success message with domain details.The success message could be more informative by including the domain name.
- $.export("$summary", "Successfully initiated domain transfer."); + $.export("$summary", `Successfully initiated transfer for domain: ${this.domain}`);
📜 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 (15)
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs(1 hunks)components/opensrs/actions/register-domain/register-domain.mjs(1 hunks)components/opensrs/actions/update-dns-records/update-dns-records.mjs(1 hunks)components/opensrs/common/constants.mjs(1 hunks)components/opensrs/common/utils.mjs(1 hunks)components/opensrs/opensrs.app.mjs(1 hunks)components/opensrs/package.json(2 hunks)components/opensrs/sources/common/events.mjs(1 hunks)components/opensrs/sources/common/polling.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/test-event.mjs(1 hunks)components/opensrs/sources/new-domain-registration/new-domain-registration.mjs(1 hunks)components/opensrs/sources/new-domain-registration/test-event.mjs(1 hunks)components/opensrs/sources/new-transfer-status/new-transfer-status.mjs(1 hunks)components/opensrs/sources/new-transfer-status/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- components/opensrs/sources/new-transfer-status/test-event.mjs
- components/opensrs/sources/new-dns-zone-change/test-event.mjs
- components/opensrs/sources/new-domain-registration/test-event.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (10)
components/opensrs/opensrs.app.mjs (2)
1-4: Prefer grouping imports for clarity and consistency
Imports look fine and align with your dependencies, but consider grouping or ordering them consistently for better readability.
6-10: Double-check parser configuration for potential side effects
arrayMode: truecan sometimes create nested arrays in unexpected places. Ensure downstream logic correctly handles these arrays, especially if API responses change.components/opensrs/actions/update-dns-records/update-dns-records.mjs (2)
79-123: Dynamic property generation is well-implemented
Great use ofutils.getAdditionalProps()to dynamically generate record props. This approach keeps your code clean and reduces boilerplate.
338-374: Ensure domain is unlocked or ready for DNS updates
Some registrars require unlocking the domain or checking status before DNS changes propagate. Verify if additional workflow steps are needed to prevent silent failures.components/opensrs/actions/register-domain/register-domain.mjs (1)
265-311: Validate registration period
Different TLDs restrict certain year lengths. A quick client-side or server-side check to confirm the chosen period is valid can prevent unexpected registration failures.components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs (3)
1-5: LGTM! Clean and well-organized imports.The imports are properly structured, importing common functionality and constants.
14-27: LGTM! Well-structured methods implementation.The methods are well-implemented:
isResourceRelevantcorrectly checks both object type and event typegenerateMetaprovides all necessary metadata fields
6-13: Consider bumping the version number.The version is set to "0.0.1" which indicates this is the first implementation. However, since this is part of a larger OpenSRS components PR, consider aligning the version with other components being introduced.
Let's check the versions of other OpenSRS components:
components/opensrs/package.json (2)
3-3: LGTM! Version bump follows semantic versioning.The version increase from 0.0.1 to 0.1.0 appropriately reflects the addition of new features.
15-17: Verify fast-xml-parser version for security vulnerabilities.Let's ensure we're using a secure version of the fast-xml-parser package.
✅ Verification successful
[email protected] appears to be secure
Version 4.3.2 shows no signs of security vulnerabilities. While there is a newer version (4.5.1) available, no security-related changes were found between these versions. The package is actively maintained with valid integrity checks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check npm registry for latest version and security advisories echo "Latest version:" npm view fast-xml-parser version echo -e "\nSecurity advisories:" npm audit [email protected] echo -e "\nPackage health score:" npm view [email protected] | grep -A 3 "maintenance"Length of output: 637
Script:
#!/bin/bash # Fetch package data from npm registry echo "Package details from npm registry:" curl -s https://registry.npmjs.org/fast-xml-parser | jq '.versions["4.3.2"].dist.integrity, .versions["4.5.1"].dist.integrity' echo -e "\nRelease information:" curl -s https://api.github.com/repos/NaturalIntelligence/fast-xml-parser/releases | jq -r '.[] | select(.tag_name == "v4.5.1" or .tag_name == "v4.3.2") | {tag_name, body}' echo -e "\nChecking for known vulnerabilities:" curl -s "https://api.snyk.io/v1/vuln/npm:[email protected]" || echo "No public vulnerability data available"Length of output: 868
Script:
#!/bin/bash # Check for security-related commits and issues echo "Checking recent commits for security mentions:" curl -s "https://api.github.com/repos/NaturalIntelligence/fast-xml-parser/commits?since=2023-01-01" | jq -r '.[] | select(.commit.message | test("security|vulnerability|CVE"; "i")) | {sha: .sha, message: .commit.message}' echo -e "\nChecking issues for security mentions:" curl -s "https://api.github.com/repos/NaturalIntelligence/fast-xml-parser/issues?state=all" | jq -r '.[] | select(.title | test("security|vulnerability|CVE"; "i")) | {number: .number, title: .title, state: .state}' echo -e "\nChecking tags for release notes:" curl -s "https://api.github.com/repos/NaturalIntelligence/fast-xml-parser/git/refs/tags" | jq -r '.[] | .ref' | grep -E "v4.3.2|v4.5.1"Length of output: 887
components/opensrs/actions/update-dns-records/update-dns-records.mjs
Outdated
Show resolved
Hide resolved
components/opensrs/sources/new-domain-registration/new-domain-registration.mjs
Show resolved
Hide resolved
components/opensrs/sources/new-domain-registration/new-domain-registration.mjs
Show resolved
Hide resolved
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs
Show resolved
Hide resolved
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
opensrs-new-components
branch
from
e40e7e1 to
0d20591
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (5)
components/opensrs/sources/new-domain-registration/new-domain-registration.mjs (2)
16-19: 🛠️ Refactor suggestionAdd null checks in isResourceRelevant.
The method should handle cases where resource or its properties are undefined.
isResourceRelevant(resource) { + if (!resource?.object || !resource?.event) { + return false; + } return resource.object === constants.OBJECT_TYPE.DOMAIN && resource.event === events.REGISTERED; },
20-26: 🛠️ Refactor suggestionAdd error handling for Date.parse().
Date.parse()can returnNaNfor invalid dates. Consider adding validation to handle invalid event_date values gracefully.generateMeta(resource) { + const timestamp = Date.parse(resource.event_date); + if (isNaN(timestamp)) { + throw new Error(`Invalid event_date format: ${resource.event_date}`); + } return { id: resource.event_id, summary: `New domain: ${resource.objectData.domain_name}`, - ts: Date.parse(resource.event_date), + ts: timestamp, }; },components/opensrs/common/utils.mjs (2)
11-14:⚠️ Potential issueAdd input validation for toPascalCase.
The function should validate its input to handle edge cases.
Apply this diff to add input validation:
function toPascalCase(str) { + if (!str || typeof str !== 'string') return ''; return str.replace(/(\w)(\w*)/g, (_, group1, group2) => group1.toUpperCase() + group2.toLowerCase()); }
164-166:⚠️ Potential issueAdd error handling for buildXmlData.
The function should handle potential XML building errors.
Apply this diff to add error handling:
function buildXmlData(data) { - return builder.build(getXml(data)); + try { + return builder.build(getXml(data)); + } catch (error) { + throw new Error(`Failed to build XML data: ${error.message}`); + } }components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs (1)
137-139:⚠️ Potential issueAdd response validation in initiateDomainTransfer.
The method should validate the response from the API call.
Apply this diff to add response validation:
- initiateDomainTransfer(args = {}) { - return this.app.post(args); + async initiateDomainTransfer(args = {}) { + const response = await this.app.post(args); + const success = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_code")?.value === "200"; + + if (!success) { + const error = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_text")?.value; + throw new Error(`Domain transfer failed: ${error || 'Unknown error'}`); + } + + return response; },
🧹 Nitpick comments (1)
components/opensrs/opensrs.app.mjs (1)
32-40: Consider adding more transfer status options.The
transferStatusesoptions seem limited to just "completed" and "failed". Consider adding intermediate states that could be useful for monitoring the transfer process.options: [ "completed", "failed", + "pending", + "processing", + "cancelled", + "rejected", ],
📜 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 (15)
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs(1 hunks)components/opensrs/actions/register-domain/register-domain.mjs(1 hunks)components/opensrs/actions/update-dns-records/update-dns-records.mjs(1 hunks)components/opensrs/common/constants.mjs(1 hunks)components/opensrs/common/utils.mjs(1 hunks)components/opensrs/opensrs.app.mjs(1 hunks)components/opensrs/package.json(2 hunks)components/opensrs/sources/common/events.mjs(1 hunks)components/opensrs/sources/common/polling.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/test-event.mjs(1 hunks)components/opensrs/sources/new-domain-registration/new-domain-registration.mjs(1 hunks)components/opensrs/sources/new-domain-registration/test-event.mjs(1 hunks)components/opensrs/sources/new-transfer-status/new-transfer-status.mjs(1 hunks)components/opensrs/sources/new-transfer-status/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- components/opensrs/sources/new-domain-registration/test-event.mjs
- components/opensrs/sources/new-dns-zone-change/test-event.mjs
- components/opensrs/package.json
- components/opensrs/sources/new-transfer-status/test-event.mjs
- components/opensrs/sources/common/events.mjs
- components/opensrs/sources/new-transfer-status/new-transfer-status.mjs
- components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs
- components/opensrs/sources/common/polling.mjs
- components/opensrs/common/constants.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/opensrs/sources/new-domain-registration/new-domain-registration.mjs (3)
1-4: LGTM! Well-structured imports.The imports are properly organized, with common utilities separated from test utilities.
6-13: Verify the documentation link accessibility.The component configuration looks good, but let's ensure the documentation link is accessible and points to the correct resource.
✅ Verification successful
Documentation link is accessible and valid
The provided OpenSRS documentation link is working correctly and returns the expected content.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the OpenSRS documentation link curl -I "https://domains.opensrs.guide/docs/registered-domain"Length of output: 1294
28-28: Verify test event implementation.Ensure that the test event provides comprehensive coverage of different domain registration scenarios.
✅ Verification successful
Test events implementation is comprehensive and well-structured
The test events provide good coverage across different domain scenarios:
- Domain registration with period and expiration tracking
- Transfer status changes with order tracking
- Zone check status monitoring
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the test event implementation cat "./components/opensrs/sources/new-domain-registration/test-event.mjs"Length of output: 367
Script:
#!/bin/bash # Search for test files and test events in the opensrs component fd "test" components/opensrs -t f -x cat {}Length of output: 919
components/opensrs/opensrs.app.mjs (1)
1-1: Consider security implications of MD5 hashing.Although OpenSRS may require MD5, it's worth noting that MD5 is not cryptographically secure by modern standards. If the provider supports a more secure hashing algorithm, consider switching.
Also applies to: 54-59
components/opensrs/common/utils.mjs (1)
16-18:⚠️ Potential issueAdd input validation for toUpperCase.
Similar to
toPascalCase, this function should validate its input.Apply this diff to add input validation:
function toUpperCase(str) { + if (!str || typeof str !== 'string') return ''; return str.toUpperCase(); }Likely invalid or redundant comment.
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
opensrs-new-components
branch
from
0d20591 to
26dae10
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/opensrs/actions/update-dns-records/update-dns-records.mjs (1)
376-395:⚠️ Potential issueAdd error handling and response validation.
The API interaction methods need proper error handling and response validation.
Previous review comments already suggested improvements for:
- Response validation in updateDnsRecords method
- Error handling in run method
Please implement the suggested changes from the previous review comments to ensure reliable API interactions.
components/opensrs/actions/register-domain/register-domain.mjs (1)
510-529:⚠️ Potential issueAdd error handling and response validation.
The API interaction methods need proper error handling and response validation.
Previous review comments already suggested improvements for:
- Response validation in registerDomain method
- Error handling in run method
Please implement the suggested changes from the previous review comments to ensure reliable API interactions.
🧹 Nitpick comments (5)
components/opensrs/opensrs.app.mjs (1)
27-35: Security consideration regarding MD5 usage.While this implementation follows OpenSRS API requirements, it's worth noting that MD5 is cryptographically weak by modern standards. Consider adding a comment explaining that this is required by the API despite MD5's known vulnerabilities.
generateSignature(data) { const { api_key: apiKey } = this.$auth; + // Note: MD5 is used here as required by OpenSRS API, despite being cryptographically weak const signature = createHash("md5") .update(data + apiKey) .digest("hex"); return createHash("md5") .update(signature + apiKey) .digest("hex"); },components/opensrs/common/utils.mjs (1)
11-14: Add input validation for toPascalCase.The function should validate input to handle edge cases.
function toPascalCase(str) { + if (!str || typeof str !== 'string') return ''; return str.replace(/(\w)(\w*)/g, (_, group1, group2) => group1.toUpperCase() + group2.toLowerCase()); }components/opensrs/actions/update-dns-records/update-dns-records.mjs (1)
212-283: Consider refactoring record mappers to reduce duplication.The record mapping methods follow a similar pattern. Consider creating a utility function to generate the dt_assoc structure.
+ createDtAssoc(items) { + return { + dt_assoc: { + item: items, + }, + }; + }, aRecordPropsMapper(prefix) { const { [`${prefix}ipAddress`]: ipAddress, [`${prefix}subdomain`]: subdomain, } = this; - return { - dt_assoc: { - item: [ - ...utils.addItem("subdomain", subdomain), - ...utils.addItem("ip_address", ipAddress), - ], - }, - }; + return this.createDtAssoc([ + ...utils.addItem("subdomain", subdomain), + ...utils.addItem("ip_address", ipAddress), + ]); },components/opensrs/actions/register-domain/register-domain.mjs (2)
29-232: Consider optimizing contact information field definitions.The contact information fields (owner, admin, billing, tech) contain significant duplication. Consider creating a factory function to generate contact fields.
function createContactFields(prefix, optional = false) { return { [`${prefix}FirstName`]: { type: "string", label: `${prefix} First Name`, description: `The first name of the ${prefix.toLowerCase()} of the domain.`, }, // ... other fields }; } // Usage in props props: { ...createContactFields("Owner"), ...createContactFields("Admin"), // ... other contacts }
351-509: Consider breaking down getJsonBody method for better maintainability.The method is well-organized but quite long. Consider extracting contact set generation into separate methods.
+ getContactSet(type, data) { + return { + "@_key": type, + "dt_assoc": { + item: [ + ...utils.addItem("first_name", data.firstName), + ...utils.addItem("last_name", data.lastName), + // ... other fields + ], + }, + }; + }, getJsonBody() { // ... existing code ... - { - "@_key": "owner", - "dt_assoc": { - item: [ - ...utils.addItem("first_name", ownerFirstName), - // ... other fields - ], - }, - }, + this.getContactSet("owner", { + firstName: ownerFirstName, + lastName: ownerLastName, + // ... other fields + }),
📜 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 (15)
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs(1 hunks)components/opensrs/actions/register-domain/register-domain.mjs(1 hunks)components/opensrs/actions/update-dns-records/update-dns-records.mjs(1 hunks)components/opensrs/common/constants.mjs(1 hunks)components/opensrs/common/utils.mjs(1 hunks)components/opensrs/opensrs.app.mjs(1 hunks)components/opensrs/package.json(2 hunks)components/opensrs/sources/common/events.mjs(1 hunks)components/opensrs/sources/common/polling.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/test-event.mjs(1 hunks)components/opensrs/sources/new-domain-registration/new-domain-registration.mjs(1 hunks)components/opensrs/sources/new-domain-registration/test-event.mjs(1 hunks)components/opensrs/sources/new-transfer-status/new-transfer-status.mjs(1 hunks)components/opensrs/sources/new-transfer-status/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- components/opensrs/sources/new-transfer-status/test-event.mjs
- components/opensrs/sources/new-domain-registration/test-event.mjs
- components/opensrs/sources/new-dns-zone-change/test-event.mjs
- components/opensrs/package.json
- components/opensrs/sources/common/events.mjs
- components/opensrs/sources/new-transfer-status/new-transfer-status.mjs
- components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs
- components/opensrs/sources/common/polling.mjs
- components/opensrs/sources/new-domain-registration/new-domain-registration.mjs
- components/opensrs/common/constants.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (12)
components/opensrs/opensrs.app.mjs (3)
1-10: LGTM! Parser configuration is appropriate.The XMLParser configuration with
ignoreAttributes: falseandarrayMode: trueis well-suited for handling OpenSRS API responses.
45-60: 🛠️ Refactor suggestionEnhance error handling robustness.
The error handling could be improved to handle more edge cases and provide better error messages.
throwIfError(jsonResponse) { + if (!jsonResponse?.OPS_envelope?.body?.data_block?.dt_assoc) { + throw new Error("Invalid response format from OpenSRS API"); + } + const { item: items } = jsonResponse.OPS_envelope.body.data_block.dt_assoc; + if (!items || !Array.isArray(items)) { + throw new Error("Missing or invalid items in response"); + } const attributes = items.find((item) => item["@_key"] === "attributes"); const { item: errorItems } = attributes?.dt_assoc || {}; const errorItem = errorItems?.find((item) => item["@_key"] === "error"); if (errorItem) { - throw new Error(errorItem.value); + throw new Error(`OpenSRS API Error: ${errorItem.value}`); } const isSuccessItem = items.find((item) => item["@_key"] === "is_success"); const responseTextItem = items.find((item) => item["@_key"] === "response_text"); if (isSuccessItem?.value === 0 && responseTextItem) { - throw new Error(responseTextItem.value); + throw new Error(`Operation failed: ${responseTextItem.value}`); } }Likely invalid or redundant comment.
61-79: 🛠️ Refactor suggestionAdd explicit network error handling.
The
_makeRequestmethod should explicitly handle network errors and timeouts.async _makeRequest({ $ = this, jsonBody, headers, jsonOutput = true, ...args } = {}) { const data = utils.buildXmlData(jsonBody); + try { const response = await axios($, { ...args, url: this.getUrl(), headers: this.getHeaders(data, headers), data, + timeout: 30000, // Add reasonable timeout }); const jsonResponse = parser.parse(response); this.throwIfError(jsonResponse); return jsonOutput ? jsonResponse : response; + } catch (error) { + if (error.code === "ECONNABORTED") { + throw new Error("Request timed out while connecting to OpenSRS API"); + } + if (error.response) { + throw new Error(`HTTP ${error.response.status}: ${error.response.statusText}`); + } + throw new Error(`Network error: ${error.message}`); + } }Likely invalid or redundant comment.
components/opensrs/common/utils.mjs (3)
116-125: 🛠️ Refactor suggestionAdd input validation for addItem.
The function should validate the key parameter and handle falsy values appropriately.
function addItem(key, value) { + if (!key || typeof key !== 'string') { + throw new Error('Key must be a non-empty string'); + } return value ? [ { "@_key": key, value, }, ] : []; }Likely invalid or redundant comment.
164-166: 🛠️ Refactor suggestionAdd error handling for buildXmlData.
The function should handle potential XML building errors.
function buildXmlData(data) { - return builder.build(getXml(data)); + try { + return builder.build(getXml(data)); + } catch (error) { + throw new Error(`Failed to build XML data: ${error.message}`); + } }Likely invalid or redundant comment.
148-162: 🛠️ Refactor suggestionAdd validation for addDnsRecord parameters.
The function should validate its inputs to prevent runtime errors.
function addDnsRecord(key, records) { + if (!key || typeof key !== 'string') { + throw new Error('Key must be a non-empty string'); + } + if (!Array.isArray(records)) { + throw new Error('Records must be an array'); + } return records.length ? [ { "@_key": key, "dt_array": { item: records.map((record, index) => ({ "@_key": index.toString(), + ...(record || {}), })), }, }, ] : []; }Likely invalid or redundant comment.
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs (3)
39-74: 🛠️ Refactor suggestionImprove validation for optional fields.
The fields phone, address1, city, country, state, and postalCode are marked as optional but are required for all domains except .BE. Consider adding dynamic validation based on the domain TLD.
+ methods: { + isBeExtension(domain) { + return domain.toLowerCase().endsWith('.be'); + }, + validateRequiredFields() { + const { domain, phone, address1, city, country, state, postalCode } = this; + if (this.isBeExtension(domain)) return; + + const required = { phone, address1, city, country, state, postalCode }; + const missing = Object.entries(required) + .filter(([, value]) => !value) + .map(([key]) => key); + + if (missing.length > 0) { + throw new Error( + `Missing required fields for non-.BE domains: ${missing.join(', ')}` + ); + } + },Likely invalid or redundant comment.
137-139: 🛠️ Refactor suggestionAdd response validation in initiateDomainTransfer.
The method should validate the response from the API call to ensure the transfer was initiated successfully.
-initiateDomainTransfer(args = {}) { - return this.app.post(args); +async initiateDomainTransfer(args = {}) { + const response = await this.app.post(args); + const success = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_code")?.value === "200"; + + if (!success) { + const error = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_text")?.value; + throw new Error(`Domain transfer failed: ${error || 'Unknown error'}`); + } + + return response; },Likely invalid or redundant comment.
141-156: 🛠️ Refactor suggestionAdd response validation and error handling in run method.
The method should validate the response and handle errors appropriately.
async run({ $ }) { const { initiateDomainTransfer, getJsonBody, jsonOutput, } = this; + try { const response = await initiateDomainTransfer({ $, jsonBody: getJsonBody(), jsonOutput, }); + if (!response?.OPS_envelope?.body?.data_block) { + throw new Error('Invalid response format from API'); + } + $.export("$summary", "Successfully initiated domain transfer."); return response; + } catch (error) { + throw new Error(`Failed to initiate domain transfer: ${error.message}`); + } },Likely invalid or redundant comment.
components/opensrs/actions/update-dns-records/update-dns-records.mjs (2)
11-77: LGTM! Props are well-structured and documented.The properties are properly defined with appropriate types, labels, descriptions, and validation options.
125-211: LGTM! Record property definitions are consistent and well-structured.Each record type's property definitions follow a consistent pattern and include appropriate validation.
components/opensrs/actions/register-domain/register-domain.mjs (1)
313-349: LGTM! Well-structured nameserver configuration.The implementation properly handles both default and custom nameserver scenarios with appropriate validation and conditional property rendering.
jcortes
force-pushed
the
opensrs-new-components
branch
from
26dae10 to
0dfee11
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/opensrs/actions/update-dns-records/update-dns-records.mjs (2)
376-378:⚠️ Potential issueAdd response validation in updateDnsRecords.
The method should validate the response from the API call.
- updateDnsRecords(args) { - return this.app.post(args); + async updateDnsRecords(args) { + const response = await this.app.post(args); + const success = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_code")?.value === "200"; + + if (!success) { + const error = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_text")?.value; + throw new Error(`DNS records update failed: ${error || 'Unknown error'}`); + } + + return response; },
380-395:⚠️ Potential issueAdd error handling in run method.
The method should handle errors appropriately and validate the response.
async run({ $ }) { const { updateDnsRecords, getJsonBody, jsonOutput, } = this; + try { const response = await updateDnsRecords({ $, jsonBody: getJsonBody(), jsonOutput, }); + if (!response?.OPS_envelope?.body?.data_block) { + throw new Error('Invalid response format from API'); + } + $.export("$summary", "Successfully updated DNS records."); return response; + } catch (error) { + throw new Error(`Failed to update DNS records: ${error.message}`); + } },
🧹 Nitpick comments (4)
components/opensrs/actions/register-domain/register-domain.mjs (1)
24-28: Enhance password validation.Add regex validation for the password to ensure it meets the specified requirements.
regPassword: { type: "string", label: "Reseller Password", description: "Passwords must be 10-20 characters in length. You can use any of the following alphanumeric characters and symbols: `A-Z`, `a-z`, `0-9`, `! @$^,.~|=-+_{}#`.", + validate: (value) => { + if (!value?.match(/^[A-Za-z0-9!@$^,.~|=\-+_{}#]{10,20}$/)) { + throw new Error("Password must be 10-20 characters and contain only allowed characters"); + } + }, },components/opensrs/actions/update-dns-records/update-dns-records.mjs (3)
19-34: Use boolean type fornameserversOkproperty.Consider using native boolean type instead of string for better type safety and cleaner code.
nameserversOk: { - type: "string", + type: "boolean", label: "Nameservers OK", description: "Indicates whether the domain is set up to use the OpenSRS nameservers.", - default: "0", + default: false, options: [ { label: "Domain is not set up to use the OpenSRS nameservers", - value: "0", + value: false, }, { label: "Domain is set up to use the OpenSRS nameservers", - value: "1", + value: true, }, ], },
151-156: Make subdomain optional for AAAA records.The subdomain field is optional for A records but required for AAAA records. This inconsistency might cause confusion. Consider making it optional for AAAA records as well to maintain consistency.
[`${prefix}subdomain`]: { type: "string", label: `${label} - Subdomain`, description: "The subdomain for the AAAA record. The third level of the domain name, such as `www` or `ftp`.", + optional: true, },
350-367: Consider extracting the records check to a constant.The empty records check could be made more readable by extracting it to a constant.
+ const hasRecords = records.length > 0; ...(records.length ? [ ...utils.addItem("nameservers_ok", nameserversOk), { "@_key": "records", "dt_assoc": { item: [ ...utils.addDnsRecord("A", aRecords), ...utils.addDnsRecord("AAAA", aaaaRecords), ...utils.addDnsRecord("CNAME", cnameRecords), ...utils.addDnsRecord("MX", mxRecords), ...utils.addDnsRecord("TXT", txtRecords), ], }, }, ] : [] ),
📜 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 (15)
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs(1 hunks)components/opensrs/actions/register-domain/register-domain.mjs(1 hunks)components/opensrs/actions/update-dns-records/update-dns-records.mjs(1 hunks)components/opensrs/common/constants.mjs(1 hunks)components/opensrs/common/utils.mjs(1 hunks)components/opensrs/opensrs.app.mjs(1 hunks)components/opensrs/package.json(2 hunks)components/opensrs/sources/common/events.mjs(1 hunks)components/opensrs/sources/common/polling.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/test-event.mjs(1 hunks)components/opensrs/sources/new-domain-registration/new-domain-registration.mjs(1 hunks)components/opensrs/sources/new-domain-registration/test-event.mjs(1 hunks)components/opensrs/sources/new-transfer-status/new-transfer-status.mjs(1 hunks)components/opensrs/sources/new-transfer-status/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- components/opensrs/sources/new-transfer-status/test-event.mjs
- components/opensrs/package.json
- components/opensrs/sources/new-domain-registration/test-event.mjs
- components/opensrs/sources/new-dns-zone-change/test-event.mjs
- components/opensrs/sources/common/events.mjs
- components/opensrs/sources/new-domain-registration/new-domain-registration.mjs
- components/opensrs/sources/new-transfer-status/new-transfer-status.mjs
- components/opensrs/common/constants.mjs
- components/opensrs/sources/common/polling.mjs
- components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (8)
components/opensrs/opensrs.app.mjs (3)
1-1: Use of MD5 hashing may pose a security risk.Although OpenSRS may require MD5, it's worth noting that MD5 is not cryptographically secure by modern standards. If the provider supports a more secure hashing algorithm, consider switching.
15-21: LGTM! Well-structured property definition.The domain property is clearly defined with appropriate type, label, and description.
61-79: 🛠️ Refactor suggestionEnhance request error handling.
The request handling should include timeout and network error handling.
async _makeRequest({ $ = this, jsonBody, headers, jsonOutput = true, ...args } = {}) { + if (!jsonBody) { + throw new Error("Request body is required"); + } const data = utils.buildXmlData(jsonBody); - const response = await axios($, { - ...args, - url: this.getUrl(), - headers: this.getHeaders(data, headers), - data, - }); + try { + const response = await axios($, { + ...args, + url: this.getUrl(), + headers: this.getHeaders(data, headers), + data, + timeout: 30000, // 30 second timeout + }); + const jsonResponse = parser.parse(response); + this.throwIfError(jsonResponse); + return jsonOutput ? jsonResponse : response; + } catch (error) { + if (error.code === "ECONNABORTED") { + throw new Error("Request timed out"); + } + if (error.response) { + throw new Error(`HTTP ${error.response.status}: ${error.response.statusText}`); + } + throw new Error(`Network error: ${error.message}`); + } - const jsonResponse = parser.parse(response); - this.throwIfError(jsonResponse); - - return jsonOutput - ? jsonResponse - : response; }Likely invalid or redundant comment.
components/opensrs/common/utils.mjs (1)
164-166: 🛠️ Refactor suggestionAdd error handling for XML building.
The buildXmlData function should handle potential XML building errors.
function buildXmlData(data) { + if (!data) { + throw new Error("Data is required for XML building"); + } + try { return builder.build(getXml(data)); + } catch (error) { + throw new Error(`Failed to build XML data: ${error.message}`); + } }Likely invalid or redundant comment.
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs (1)
141-156: 🛠️ Refactor suggestionAdd response validation and error handling.
The run method should validate the response and handle errors appropriately.
async run({ $ }) { const { initiateDomainTransfer, getJsonBody, jsonOutput, } = this; + try { const response = await initiateDomainTransfer({ $, jsonBody: getJsonBody(), jsonOutput, }); + if (!response?.OPS_envelope?.body?.data_block) { + throw new Error("Invalid response format from API"); + } + $.export("$summary", "Successfully initiated domain transfer."); return response; + } catch (error) { + throw new Error(`Failed to initiate domain transfer: ${error.message}`); + } },Likely invalid or redundant comment.
components/opensrs/actions/register-domain/register-domain.mjs (1)
514-529: 🛠️ Refactor suggestionAdd response validation and error handling.
The run method should validate the response and handle errors appropriately.
async run({ $ }) { const { registerDomain, getJsonBody, jsonOutput, } = this; + try { const response = await registerDomain({ $, jsonBody: getJsonBody(), jsonOutput, }); + if (!response?.OPS_envelope?.body?.data_block) { + throw new Error("Invalid response format from API"); + } + $.export("$summary", "Successfully registered domain."); return response; + } catch (error) { + throw new Error(`Failed to register domain: ${error.message}`); + } },Likely invalid or redundant comment.
components/opensrs/actions/update-dns-records/update-dns-records.mjs (2)
78-123: LGTM!The additional properties handling is well-structured and follows best practices.
212-283: LGTM!The record mappers are well-implemented and follow a consistent pattern across all record types.
There was a problem hiding this comment.
Hi @jcortes, I just added some suggestions, but I'm still moving them to ready for QA.
jcortes
dismissed stale reviews from luancazarine and michelle0927
via
f9888ab
jcortes
force-pushed
the
opensrs-new-components
branch
from
0dfee11 to
f9888ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
components/opensrs/common/utils.mjs (2)
11-14: Add input validation for string transformation.The
toPascalCasefunction should validate its input.function toPascalCase(str) { + if (!str || typeof str !== 'string') return ''; return str.replace(/(\w)(\w*)/g, (_, group1, group2) => group1.toUpperCase() + group2.toLowerCase()); }
116-125: Add input validation for addItem.The
addItemfunction should validate its key parameter.function addItem(key, value) { + if (!key || typeof key !== 'string') { + throw new Error('Key must be a non-empty string'); + } return value ? [ { "@_key": key, value, }, ] : []; }
📜 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 (15)
components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs(1 hunks)components/opensrs/actions/register-domain/register-domain.mjs(1 hunks)components/opensrs/actions/update-dns-records/update-dns-records.mjs(1 hunks)components/opensrs/common/constants.mjs(1 hunks)components/opensrs/common/utils.mjs(1 hunks)components/opensrs/opensrs.app.mjs(1 hunks)components/opensrs/package.json(2 hunks)components/opensrs/sources/common/events.mjs(1 hunks)components/opensrs/sources/common/polling.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs(1 hunks)components/opensrs/sources/new-dns-zone-change/test-event.mjs(1 hunks)components/opensrs/sources/new-domain-registration/new-domain-registration.mjs(1 hunks)components/opensrs/sources/new-domain-registration/test-event.mjs(1 hunks)components/opensrs/sources/new-transfer-status/new-transfer-status.mjs(1 hunks)components/opensrs/sources/new-transfer-status/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- components/opensrs/sources/new-transfer-status/test-event.mjs
- components/opensrs/package.json
- components/opensrs/sources/new-dns-zone-change/test-event.mjs
- components/opensrs/sources/new-domain-registration/new-domain-registration.mjs
- components/opensrs/sources/common/polling.mjs
- components/opensrs/sources/new-domain-registration/test-event.mjs
- components/opensrs/common/constants.mjs
- components/opensrs/actions/initiate-domain-transfer/initiate-domain-transfer.mjs
- components/opensrs/sources/common/events.mjs
- components/opensrs/sources/new-dns-zone-change/new-dns-zone-change.mjs
- components/opensrs/sources/new-transfer-status/new-transfer-status.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
components/opensrs/opensrs.app.mjs (1)
62-80: 🛠️ Refactor suggestionAdd explicit network error handling and timeout.
The
_makeRequestmethod should handle network errors and timeouts explicitly.async _makeRequest({ $ = this, jsonBody, headers, jsonOutput = true, ...args } = {}) { const data = utils.buildXmlData(jsonBody); + try { const response = await axios($, { ...args, url: this.getUrl(), headers: this.getHeaders(data, headers), data, + timeout: 30000, // Add reasonable timeout }); const jsonResponse = parser.parse(response); this.throwIfError(jsonResponse); return jsonOutput ? jsonResponse : response; + } catch (error) { + if (error.code === "ECONNABORTED") { + throw new Error("Request timed out while connecting to OpenSRS API"); + } + if (error.response) { + throw new Error(`HTTP ${error.response.status}: ${error.response.statusText}`); + } + throw new Error(`Network error: ${error.message}`); + } }Likely invalid or redundant comment.
components/opensrs/common/utils.mjs (1)
164-166: 🛠️ Refactor suggestionAdd error handling for XML building.
The
buildXmlDatafunction should handle potential XML building errors.function buildXmlData(data) { - return builder.build(getXml(data)); + try { + return builder.build(getXml(data)); + } catch (error) { + throw new Error(`Failed to build XML data: ${error.message}`); + } }Likely invalid or redundant comment.
components/opensrs/actions/update-dns-records/update-dns-records.mjs (2)
376-378: 🛠️ Refactor suggestionAdd response validation in updateDnsRecords.
The method should validate the response from the API call.
- updateDnsRecords(args) { - return this.app.post(args); + async updateDnsRecords(args) { + const response = await this.app.post(args); + const success = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_code")?.value === "200"; + + if (!success) { + const error = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_text")?.value; + throw new Error(`DNS records update failed: ${error || 'Unknown error'}`); + } + + return response; },Likely invalid or redundant comment.
380-395: 🛠️ Refactor suggestionAdd error handling in run method.
The method should handle errors appropriately and validate the response.
async run({ $ }) { const { updateDnsRecords, getJsonBody, jsonOutput, } = this; + try { const response = await updateDnsRecords({ $, jsonBody: getJsonBody(), jsonOutput, }); + if (!response?.OPS_envelope?.body?.data_block) { + throw new Error('Invalid response format from API'); + } + $.export("$summary", "Successfully updated DNS records."); return response; + } catch (error) { + throw new Error(`Failed to update DNS records: ${error.message}`); + } },Likely invalid or redundant comment.
components/opensrs/actions/register-domain/register-domain.mjs (2)
514-529: 🛠️ Refactor suggestionAdd error handling in run method.
The method should handle errors appropriately and validate the response.
async run({ $ }) { const { registerDomain, getJsonBody, jsonOutput, } = this; + try { const response = await registerDomain({ $, jsonBody: getJsonBody(), jsonOutput, }); + if (!response?.OPS_envelope?.body?.data_block) { + throw new Error('Invalid response format from API'); + } + $.export("$summary", "Successfully registered domain."); return response; + } catch (error) { + throw new Error(`Failed to register domain: ${error.message}`); + } },Likely invalid or redundant comment.
510-512: 🛠️ Refactor suggestionAdd response validation in registerDomain.
The method should validate the response from the API call.
- registerDomain(args = {}) { - return this.app.post(args); + async registerDomain(args = {}) { + const response = await this.app.post(args); + const success = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_code")?.value === "200"; + + if (!success) { + const error = response?.OPS_envelope?.body?.data_block?.dt_assoc?.item + ?.find(item => item["@_key"] === "response_text")?.value; + throw new Error(`Domain registration failed: ${error || 'Unknown error'}`); + } + + return response; },Likely invalid or redundant comment.
jcortes
force-pushed
the
opensrs-new-components
branch
from
f9888ab to
a06fb3f
Compare
|
/approve |
WHY
Resolves #15235
Summary by CodeRabbit
Release Notes v0.1.0
New Features
Improvements
Technical Updates