Hoftix/desintation specification mismatch#1701
Conversation
…le both old and new version of postgres atleast
WalkthroughModified destination form configuration handling to dynamically select specification endpoints based on edit state (warehouse present) versus creation state. Enhanced connector config input with improved discriminator detection logic across schema traversal, oneOf processing, and form field prefilling to support multiple discriminator formats. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1701 +/- ##
==========================================
+ Coverage 75.33% 75.34% +0.01%
==========================================
Files 107 107
Lines 7962 7966 +4
Branches 1866 1929 +63
==========================================
+ Hits 5998 6002 +4
+ Misses 1927 1840 -87
- Partials 37 124 +87 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Destinations/DestinationForm.tsx`:
- Around line 136-139: Guard against data being undefined before accessing
connectionSpecification: in the block that constructs ConnectorConfigInput in
DestinationForm (the line creating new ConnectorConfigInput('destination',
data.connectionSpecification || data)), check that data is defined first and
either early-return / show loading/error or pass a safe fallback; e.g., use
optional chaining or a fallback value when calling new ConnectorConfigInput
(e.g., data?.connectionSpecification ?? data ?? {}) or add an if (!data) { /*
handle missing data */ } before constructing ConnectorConfigInput to prevent
reading connectionSpecification on undefined.
In `@src/helpers/ConnectorConfigInput.tsx`:
- Around line 139-154: The lookup for the discriminator field can throw when a
oneOf branch lacks a properties object; change the access to use safe optional
chaining so the discriminator retrieval is tolerant (e.g., use
data?.properties?.[exclude[0]]), keep the existing fallback logic that checks
discriminatorField?.const, discriminatorField?.enum?.[0],
discriminatorField?.default and then data?.title, and push to dropdownEnums only
when enumValue is set; update the reference in the block that currently defines
discriminatorField and enumValue to use the safe access pattern to avoid runtime
errors.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/Destinations/DestinationForm.tsxsrc/helpers/ConnectorConfigInput.tsx
| const connectorConfigInput = new ConnectorConfigInput( | ||
| 'destination', | ||
| data.connectionSpecification || data | ||
| ); |
There was a problem hiding this comment.
Guard data before reading connectionSpecification.
At Line 138, data.connectionSpecification will throw if the GET returns undefined (e.g., unauthorized path from httpGet). Add a null guard before constructing ConnectorConfigInput.
💡 Proposed fix
- const connectorConfigInput = new ConnectorConfigInput(
- 'destination',
- data.connectionSpecification || data
- );
+ if (!data) {
+ errorToast('Unable to load destination specification', [], globalContext);
+ return;
+ }
+
+ const specification = data.connectionSpecification ?? data;
+ const connectorConfigInput = new ConnectorConfigInput('destination', specification);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Destinations/DestinationForm.tsx` around lines 136 - 139,
Guard against data being undefined before accessing connectionSpecification: in
the block that constructs ConnectorConfigInput in DestinationForm (the line
creating new ConnectorConfigInput('destination', data.connectionSpecification ||
data)), check that data is defined first and either early-return / show
loading/error or pass a safe fallback; e.g., use optional chaining or a fallback
value when calling new ConnectorConfigInput (e.g., data?.connectionSpecification
?? data ?? {}) or add an if (!data) { /* handle missing data */ } before
constructing ConnectorConfigInput to prevent reading connectionSpecification on
undefined.
| const discriminatorField = data?.properties[exclude[0]]; | ||
| let enumValue = discriminatorField?.const || discriminatorField?.enum?.[0]; | ||
|
|
||
| // Handle new format where discriminator might be the title or default value | ||
| if (!enumValue && discriminatorField?.default) { | ||
| enumValue = discriminatorField.default; | ||
| } | ||
|
|
||
| // For new object-type format, extract from title or use parent data title | ||
| if (!enumValue && data?.title) { | ||
| enumValue = data.title.toLowerCase(); | ||
| } | ||
|
|
||
| if (enumValue) { | ||
| dropdownEnums.push(enumValue); | ||
| } |
There was a problem hiding this comment.
Use safe optional access for properties in discriminator lookup.
At Line 139, data?.properties[exclude[0]] can throw when a oneOf branch has no properties (title/default-only variants). That prevents the new fallback logic from executing.
💡 Proposed fix
- if (exclude[0] in data?.properties) {
- const discriminatorField = data?.properties[exclude[0]];
- let enumValue = discriminatorField?.const || discriminatorField?.enum?.[0];
-
- // Handle new format where discriminator might be the title or default value
- if (!enumValue && discriminatorField?.default) {
- enumValue = discriminatorField.default;
- }
-
- // For new object-type format, extract from title or use parent data title
- if (!enumValue && data?.title) {
- enumValue = data.title.toLowerCase();
- }
-
- if (enumValue) {
- dropdownEnums.push(enumValue);
- }
- }
+ const discriminatorField = data?.properties?.[exclude[0]];
+ let enumValue =
+ discriminatorField?.const ??
+ discriminatorField?.enum?.[0] ??
+ discriminatorField?.default;
+
+ // For object-type format, extract from title when explicit discriminator is absent
+ if (!enumValue && data?.title) {
+ enumValue = data.title.toLowerCase();
+ }
+
+ if (enumValue) {
+ dropdownEnums.push(enumValue);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/helpers/ConnectorConfigInput.tsx` around lines 139 - 154, The lookup for
the discriminator field can throw when a oneOf branch lacks a properties object;
change the access to use safe optional chaining so the discriminator retrieval
is tolerant (e.g., use data?.properties?.[exclude[0]]), keep the existing
fallback logic that checks discriminatorField?.const,
discriminatorField?.enum?.[0], discriminatorField?.default and then data?.title,
and push to dropdownEnums only when enumValue is set; update the reference in
the block that currently defines discriminatorField and enumValue to use the
safe access pattern to avoid runtime errors.
| ); | ||
| let data; | ||
|
|
||
| if (warehouse) { |
There was a problem hiding this comment.
can you add -> if (warehouse && warehouse?.destinationId)
There was a problem hiding this comment.
It will fail in the Destinations.tsx component itself ?
we are doing this there
setWarehouse({
airbyteWorkspaceId: w_house.airbyte_destination.workspaceId,
airbyteDockerRepository: w_house.airbyte_docker_repository,
tag: w_house.airbyte_docker_image_tag,
destinationId: w_house.airbyte_destination.destinationId,
destinationDefinitionId: w_house.airbyte_destination.destinationDefinitionId,
name: w_house.name,
wtype: w_house.wtype,
icon: w_house.airbyte_destination.icon,
connectionConfiguration: w_house.airbyte_destination.connectionConfiguration,
ssl: w_house.ssl,
} as Warehouse);
so this would fail w_house.airbyte_destination.destinationId
should i add it here also ?
2 participants
Summary by CodeRabbit