Skip to content

Commit a48a65d

Browse files
sleidigAbhinegi2
andauthored
fix(Import): gracefully skip values with errors during transformation (#3712)
--------- Co-authored-by: Abhinegi2 <negiabhishek253@gmail.com>
1 parent 7e9b42b commit a48a65d

7 files changed

Lines changed: 480 additions & 106 deletions

File tree

src/app/core/import/import-review-data/import-review-data.component.html

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@
2828
</app-hint-box>
2929
}
3030

31+
@if (transformationErrorColumns?.length > 0) {
32+
<app-hint-box>
33+
<div class="flex-row gap-small align-center warning-red">
34+
<span i18n>Some values could not be transformed and were skipped.</span>
35+
<span i18n>
36+
Affected column(s): {{ transformationErrorColumns.join(", ") }}
37+
</span>
38+
</div>
39+
</app-hint-box>
40+
}
41+
3142
<app-entities-table
3243
[entityType]="entityConstructor"
3344
[columnsToDisplay]="displayColumns"

src/app/core/import/import-review-data/import-review-data.component.spec.ts

Lines changed: 206 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,37 @@ import { MatDialog } from "@angular/material/dialog";
1111
import { of } from "rxjs";
1212
import { ImportService } from "../import.service";
1313
import { TestEntity } from "../../../utils/test-utils/TestEntity";
14+
import { Logging } from "../../logging/logging.service";
15+
import { ConfirmationDialogService } from "../../common-components/confirmation-dialog/confirmation-dialog.service";
1416

1517
describe("ImportReviewDataComponent", () => {
1618
let component: ImportReviewDataComponent;
1719
let fixture: ComponentFixture<ImportReviewDataComponent>;
1820

1921
let mockImportService: jasmine.SpyObj<ImportService>;
2022
let mockDialog: jasmine.SpyObj<MatDialog>;
23+
let mockConfirmationDialog: jasmine.SpyObj<ConfirmationDialogService>;
2124

2225
beforeEach(async () => {
2326
mockImportService = jasmine.createSpyObj(["transformRawDataToEntities"]);
24-
mockImportService.transformRawDataToEntities.and.resolveTo([]);
27+
mockImportService.transformRawDataToEntities.and.resolveTo({
28+
entities: [],
29+
errors: [],
30+
});
2531
mockDialog = jasmine.createSpyObj(["open"]);
2632
mockDialog.open.and.returnValue({ afterClosed: () => of({}) } as any);
33+
mockConfirmationDialog = jasmine.createSpyObj(["getConfirmation"]);
34+
mockConfirmationDialog.getConfirmation.and.resolveTo(true);
2735

2836
await TestBed.configureTestingModule({
2937
imports: [MockedTestingModule, ImportReviewDataComponent],
3038
providers: [
3139
{ provide: ImportService, useValue: mockImportService },
3240
{ provide: MatDialog, useValue: mockDialog },
41+
{
42+
provide: ConfirmationDialogService,
43+
useValue: mockConfirmationDialog,
44+
},
3345
],
3446
}).compileComponents();
3547

@@ -43,13 +55,20 @@ describe("ImportReviewDataComponent", () => {
4355

4456
it("should parse data whenever it changes", fakeAsync(() => {
4557
const testEntities = [new TestEntity("1")];
46-
mockImportService.transformRawDataToEntities.and.resolveTo(testEntities);
58+
mockImportService.transformRawDataToEntities.and.resolveTo({
59+
entities: testEntities,
60+
errors: [],
61+
});
4762
component.columnMapping = [
4863
{ column: "x", propertyName: "name" },
4964
{ column: "y", propertyName: undefined }, // unmapped property => not displayed
5065
];
66+
component.stepIsFocused = true;
5167

52-
component.ngOnChanges({});
68+
component.ngOnChanges({
69+
columnMapping: {} as any,
70+
showErrorDialog: {} as any,
71+
});
5372
tick();
5473

5574
expect(component.mappedEntities).toEqual(testEntities);
@@ -65,4 +84,188 @@ describe("ImportReviewDataComponent", () => {
6584

6685
expect(mockDialog.open).toHaveBeenCalled();
6786
}));
87+
88+
it("should handle errors from transformRawDataToEntities gracefully", fakeAsync(() => {
89+
mockImportService.transformRawDataToEntities.and.rejectWith(
90+
new Error("location lookup failed"),
91+
);
92+
spyOn(Logging, "error");
93+
94+
component.columnMapping = [{ column: "x", propertyName: "name" }];
95+
component.stepIsFocused = true;
96+
component.ngOnChanges({
97+
columnMapping: {} as any,
98+
showErrorDialog: {} as any,
99+
});
100+
tick();
101+
102+
expect(component.mappedEntities).toEqual([]);
103+
expect(component.isLoading).toBeFalse();
104+
expect(Logging.error).toHaveBeenCalled();
105+
}));
106+
107+
it("should show confirmation dialog and keep entities when user continues after transformation errors", fakeAsync(() => {
108+
const testEntities = [new TestEntity("1")];
109+
mockImportService.transformRawDataToEntities.and.resolveTo({
110+
entities: testEntities,
111+
errors: [
112+
{
113+
column: "address",
114+
propertyName: "location",
115+
rowIndex: 0,
116+
error: new Error("lookup failed"),
117+
},
118+
],
119+
});
120+
mockConfirmationDialog.getConfirmation.and.resolveTo(true);
121+
122+
component.columnMapping = [{ column: "x", propertyName: "name" }];
123+
component.stepIsFocused = true;
124+
component.ngOnChanges({
125+
columnMapping: {} as any,
126+
showErrorDialog: {} as any,
127+
});
128+
tick();
129+
130+
expect(mockConfirmationDialog.getConfirmation).toHaveBeenCalled();
131+
expect(component.mappedEntities).toEqual(testEntities);
132+
expect(component.isLoading).toBeFalse();
133+
}));
134+
135+
it("should continue preview even if transformation dialog resolves false", fakeAsync(() => {
136+
const testEntities = [new TestEntity("1")];
137+
mockImportService.transformRawDataToEntities.and.resolveTo({
138+
entities: testEntities,
139+
errors: [
140+
{
141+
column: "address",
142+
propertyName: "location",
143+
rowIndex: 0,
144+
error: new Error("lookup failed"),
145+
},
146+
],
147+
});
148+
mockConfirmationDialog.getConfirmation.and.resolveTo(false);
149+
150+
component.columnMapping = [{ column: "x", propertyName: "name" }];
151+
component.stepIsFocused = true;
152+
component.ngOnChanges({
153+
columnMapping: {} as any,
154+
showErrorDialog: {} as any,
155+
});
156+
tick();
157+
158+
expect(mockConfirmationDialog.getConfirmation).toHaveBeenCalled();
159+
expect(component.mappedEntities).toEqual(testEntities);
160+
expect(component.isLoading).toBeFalse();
161+
}));
162+
163+
it("should show confirmation dialog when preview becomes visible with errors", fakeAsync(() => {
164+
const testEntities = [new TestEntity("1")];
165+
mockImportService.transformRawDataToEntities.and.resolveTo({
166+
entities: testEntities,
167+
errors: [
168+
{
169+
column: "address",
170+
propertyName: "location",
171+
rowIndex: 0,
172+
error: new Error("lookup failed"),
173+
},
174+
],
175+
});
176+
component.stepIsFocused = false;
177+
178+
component.columnMapping = [{ column: "x", propertyName: "name" }];
179+
// Simulate data change and then making preview visible
180+
component.ngOnChanges({
181+
columnMapping: {} as any,
182+
});
183+
tick();
184+
185+
// Data changed but preview not visible yet - should NOT parse yet
186+
expect(mockImportService.transformRawDataToEntities).not.toHaveBeenCalled();
187+
expect(component.mappedEntities).toEqual([]);
188+
189+
// Now show the preview
190+
component.stepIsFocused = true;
191+
component.ngOnChanges({
192+
showErrorDialog: {} as any,
193+
});
194+
tick();
195+
196+
// Should parse now and show the confirmation dialog for errors
197+
expect(mockImportService.transformRawDataToEntities).toHaveBeenCalled();
198+
expect(mockConfirmationDialog.getConfirmation).toHaveBeenCalled();
199+
expect(component.mappedEntities).toEqual(testEntities);
200+
expect(component.isLoading).toBeFalse();
201+
}));
202+
203+
it("should delay parsing until preview is visible for performance", fakeAsync(() => {
204+
const testEntities = [new TestEntity("1")];
205+
mockImportService.transformRawDataToEntities.and.resolveTo({
206+
entities: testEntities,
207+
errors: [],
208+
});
209+
component.stepIsFocused = false;
210+
211+
// Change data while preview is not visible
212+
component.columnMapping = [{ column: "x", propertyName: "name" }];
213+
component.ngOnChanges({
214+
columnMapping: {} as any,
215+
});
216+
tick();
217+
218+
// Should NOT parse yet
219+
expect(mockImportService.transformRawDataToEntities).not.toHaveBeenCalled();
220+
expect(component.mappedEntities).toEqual([]);
221+
222+
// Make preview visible
223+
component.stepIsFocused = true;
224+
component.ngOnChanges({
225+
showErrorDialog: {} as any,
226+
});
227+
tick();
228+
229+
expect(mockImportService.transformRawDataToEntities).toHaveBeenCalled();
230+
expect(component.mappedEntities).toEqual(testEntities);
231+
}));
232+
233+
it("should not re-parse when only navigating away and back without data changes", fakeAsync(() => {
234+
const testEntities = [new TestEntity("1")];
235+
mockImportService.transformRawDataToEntities.and.resolveTo({
236+
entities: testEntities,
237+
errors: [],
238+
});
239+
component.stepIsFocused = true;
240+
component.columnMapping = [{ column: "x", propertyName: "name" }];
241+
242+
// Initial parse when data changes and preview is visible
243+
component.ngOnChanges({
244+
columnMapping: {} as any,
245+
showErrorDialog: {} as any,
246+
});
247+
tick();
248+
249+
expect(mockImportService.transformRawDataToEntities).toHaveBeenCalledTimes(
250+
1,
251+
);
252+
mockImportService.transformRawDataToEntities.calls.reset();
253+
254+
// Navigate away from preview
255+
component.stepIsFocused = false;
256+
component.ngOnChanges({
257+
showErrorDialog: {} as any,
258+
});
259+
tick();
260+
261+
// Navigate back to preview (no data changes)
262+
component.stepIsFocused = true;
263+
component.ngOnChanges({
264+
showErrorDialog: {} as any,
265+
});
266+
tick();
267+
268+
// Should NOT re-parse since data hasn't changed
269+
expect(mockImportService.transformRawDataToEntities).not.toHaveBeenCalled();
270+
}));
68271
});

0 commit comments

Comments
 (0)