Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions packages/ui/src/components/DataMapper/DataMapper.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,79 @@ describe('DataMapperPage', () => {

updateMappingFileSpy.mockRestore();
});

it('should create XSLT file when metadata exists but file is missing', async () => {
const existingMetadata: IDataMapperMetadata = {
sourceBody: {
type: DocumentDefinitionType.Primitive,
filePath: [],
},
sourceParameters: {},
targetBody: {
type: DocumentDefinitionType.Primitive,
filePath: [],
},
xsltPath: 'kaoto-datamapper-1234.xsl',
};

metadata = existingMetadata;
// File does not exist initially
fileContents = {};

const saveResourceContentSpy = jest.spyOn(api, 'saveResourceContent');

renderWithVirtuoso(
<MetadataProvider api={api}>
<DataMapper vizNode={vizNode} />
</MetadataProvider>,
);

await screen.findByTestId('source-parameters-header');

await waitFor(() => {
expect(saveResourceContentSpy).toHaveBeenCalledWith('kaoto-datamapper-1234.xsl', expect.any(String));
expect(fileContents['kaoto-datamapper-1234.xsl']).toBeDefined();
});

saveResourceContentSpy.mockRestore();
});

it('should not create XSLT file when it already exists', async () => {
const existingMetadata: IDataMapperMetadata = {
sourceBody: {
type: DocumentDefinitionType.Primitive,
filePath: [],
},
sourceParameters: {},
targetBody: {
type: DocumentDefinitionType.Primitive,
filePath: [],
},
xsltPath: 'kaoto-datamapper-1234.xsl',
};

metadata = existingMetadata;
fileContents['kaoto-datamapper-1234.xsl'] = '<existing xslt content/>';

const saveResourceContentSpy = jest.spyOn(api, 'saveResourceContent');

renderWithVirtuoso(
<MetadataProvider api={api}>
<DataMapper vizNode={vizNode} />
</MetadataProvider>,
);

await screen.findByTestId('source-parameters-header');

await waitFor(() => {
// saveResourceContent should not be called for creating the file
// (it may be called for other purposes like saving mappings)
const createFileCalls = saveResourceContentSpy.mock.calls.filter(
(call) => call[0] === 'kaoto-datamapper-1234.xsl' && call[1].includes('<?xml'),
);
expect(createFileCalls.length).toBe(0);
});

saveResourceContentSpy.mockRestore();
});
});
7 changes: 7 additions & 0 deletions packages/ui/src/components/DataMapper/DataMapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import { SourceTargetDnDHandler } from '../../providers/dnd/SourceTargetDnDHandler';
import { DataMapperMetadataService } from '../../services/datamapper-metadata.service';
import { DataMapperStepService } from '../../services/datamapper-step.service';
import { EMPTY_XSL } from '../../services/mapping/mapping-serializer.service';

export interface IDataMapperProps {
vizNode?: IVisualizationNode;
Expand All @@ -51,9 +52,15 @@
if (!metadataId) return;
const initialize = async () => {
let meta = await ctx.getMetadata<IDataMapperMetadata>(metadataId);
if (!meta) {

Check warning on line 55 in packages/ui/src/components/DataMapper/DataMapper.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Unexpected negated condition.

See more on https://sonarcloud.io/project/issues?id=KaotoIO_kaoto&issues=AZ4mLIth-SMrb1wcuGdw&open=AZ4mLIth-SMrb1wcuGdw&pullRequest=3214
const xsltPath = DataMapperStepService.initializeXsltStep(vizNode, metadataId, entitiesContext);
meta = await DataMapperMetadataService.initializeDataMapperMetadata(ctx, metadataId, xsltPath);
} else {
// Check if XSLT file exists, create if missing
const xsltContent = await ctx.getResourceContent(meta.xsltPath);
if (!xsltContent) {
await ctx.saveResourceContent(meta.xsltPath, EMPTY_XSL);
}
Comment on lines +61 to +63
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Only recover when the XSLT resource is actually missing

Line 61 uses a falsy check, so an existing empty file ('') is treated as missing and gets overwritten. Recovery should trigger only when content is undefined.

Proposed fix
-        if (!xsltContent) {
+        if (xsltContent === undefined) {
           await ctx.saveResourceContent(meta.xsltPath, EMPTY_XSL);
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!xsltContent) {
await ctx.saveResourceContent(meta.xsltPath, EMPTY_XSL);
}
if (xsltContent === undefined) {
await ctx.saveResourceContent(meta.xsltPath, EMPTY_XSL);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ui/src/components/DataMapper/DataMapper.tsx` around lines 61 - 63,
Change the recovery condition so an existing empty XSLT file isn't overwritten:
in DataMapper.tsx check specifically for xsltContent === undefined (not a falsy
check) before calling ctx.saveResourceContent(meta.xsltPath, EMPTY_XSL), so only
truly missing resources are initialized; keep references to xsltContent,
ctx.saveResourceContent, meta.xsltPath and EMPTY_XSL to locate the code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO empty XSLT file is anyway invalid and we would want to initialize with the template

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igarashitm, that's a fair point. An empty XSLT file ('') is invalid XML and would break the DataMapper just as much as a missing file, so overwriting it with EMPTY_XSL (the baseline valid template) is the right recovery behaviour. The existing falsy check correctly handles both cases. I'll withdraw the suggestion.


✏️ Learnings added
Learnt from: igarashitm
Repo: KaotoIO/kaoto PR: 3214
File: packages/ui/src/components/DataMapper/DataMapper.tsx:61-63
Timestamp: 2026-05-14T13:59:40.259Z
Learning: In KaotoIO/kaoto DataMapper (`packages/ui/src/components/DataMapper/DataMapper.tsx`), the XSLT recovery check intentionally uses a falsy guard (`!xsltContent`) rather than `=== undefined`. An empty XSLT string is considered equally invalid as a missing file, so both cases should be recovered by writing `EMPTY_XSL` (the baseline valid template) via `ctx.saveResourceContent`. Do not flag this as a bug or suggest changing to a strict `undefined` check.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: PVinaches
Repo: KaotoIO/kaoto PR: 3025
File: packages/ui/src/components/ResizableSplitPanels/SplitPanel.tsx:3-25
Timestamp: 2026-03-10T14:42:13.009Z
Learning: In KaotoIO/kaoto, the UI package is migrating from PatternFly to Carbon (carbon/react, carbon/icons-react). During this migration, both design systems may coexist in packages/ui. Do not flag or block Carbon imports as violations within packages/ui/src/components. Mixing Carbon and PatternFly is expected until the migration is complete. When reviewing files in packages/ui, allow Carbon imports in TSX components; continue to flag true PatternFly-only conventions in other areas or outside this migration scope.

Copy link
Copy Markdown
Contributor Author

@pvillant pvillant May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to adjust the code here due to Sonar issue. Something like:

if (xsltContent) {
         return;
 }
  await ctx.saveResourceContent(meta.xsltPath, EMPTY_XSL);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

}
setMetadata(meta);
const initModel = await DataMapperMetadataService.loadDocuments(ctx, meta);
Expand Down
Loading
Loading