Skip to content

Commit 2b02171

Browse files
guguclaude
gugu
and
claude
committed
refactor: convert charts service to signals and fix test types
- Add rxResource to SavedQueriesService for reactive data fetching - Add savedQueries, savedQueriesLoading signals and setActiveConnection/refreshSavedQueries methods - Update ChartsListComponent to consume service signals instead of local state - Fix Monaco editor cursor position issue by using signal that only updates on load - Auto-execute test query when editing existing chart to show preview - Replace all `as any` type assertions in chart test files with proper typed interfaces - Use Partial<ServiceType> for mock services and component testable types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent a4e025e commit 2b02171

10 files changed

Lines changed: 512 additions & 364 deletions

‎frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.html‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ <h2 mat-dialog-title>Delete Saved Query</h2>
1313
</mat-dialog-content>
1414

1515
<mat-dialog-actions align="end">
16-
<button mat-button mat-dialog-close [disabled]="submitting">Cancel</button>
16+
<button mat-button mat-dialog-close [disabled]="submitting()">Cancel</button>
1717
<button mat-flat-button color="warn"
1818
(click)="onDelete()"
19-
[disabled]="submitting"
19+
[disabled]="submitting()"
2020
data-testid="delete-chart-confirm-button">
21-
{{submitting ? 'Deleting...' : 'Delete Query'}}
21+
{{submitting() ? 'Deleting...' : 'Delete Query'}}
2222
</button>
2323
</mat-dialog-actions>

‎frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.spec.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { provideHttpClient } from '@angular/common/http';
2+
import { provideHttpClientTesting } from '@angular/common/http/testing';
3+
import { WritableSignal } from '@angular/core';
4+
import { ComponentFixture, TestBed } from '@angular/core/testing';
5+
import { MAT_DIALOG_DATA, MatDialogModule, MatDialogRef } from '@angular/material/dialog';
6+
import { MatSnackBarModule } from '@angular/material/snack-bar';
7+
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
8+
import { Angulartics2Module } from 'angulartics2';
9+
import { of, throwError } from 'rxjs';
10+
import { SavedQuery } from 'src/app/models/saved-query';
11+
import { SavedQueriesService } from 'src/app/services/saved-queries.service';
12+
import { ChartDeleteDialogComponent } from './chart-delete-dialog.component';
13+
14+
type ChartDeleteDialogComponentTestable = ChartDeleteDialogComponent & {
15+
submitting: WritableSignal<boolean>;
16+
};
17+
18+
describe('ChartDeleteDialogComponent', () => {
19+
let component: ChartDeleteDialogComponent;
20+
let fixture: ComponentFixture<ChartDeleteDialogComponent>;
21+
let mockSavedQueriesService: Partial<SavedQueriesService>;
22+
let mockDialogRef: Partial<MatDialogRef<ChartDeleteDialogComponent>>;
23+
24+
const mockSavedQuery: SavedQuery = {
25+
id: '1',
26+
name: 'Test Query',
27+
description: 'Test description',
28+
query_text: 'SELECT * FROM users',
29+
connection_id: 'conn-1',
30+
created_at: '2024-01-01',
31+
updated_at: '2024-01-01',
32+
};
33+
34+
beforeEach(async () => {
35+
mockSavedQueriesService = {
36+
deleteSavedQuery: vi.fn().mockReturnValue(of(mockSavedQuery)),
37+
};
38+
39+
mockDialogRef = {
40+
close: vi.fn(),
41+
};
42+
43+
await TestBed.configureTestingModule({
44+
imports: [
45+
ChartDeleteDialogComponent,
46+
BrowserAnimationsModule,
47+
MatSnackBarModule,
48+
MatDialogModule,
49+
Angulartics2Module.forRoot(),
50+
],
51+
providers: [
52+
provideHttpClient(),
53+
provideHttpClientTesting(),
54+
{ provide: SavedQueriesService, useValue: mockSavedQueriesService },
55+
{ provide: MatDialogRef, useValue: mockDialogRef },
56+
{
57+
provide: MAT_DIALOG_DATA,
58+
useValue: { query: mockSavedQuery, connectionId: 'conn-1' },
59+
},
60+
],
61+
}).compileComponents();
62+
63+
fixture = TestBed.createComponent(ChartDeleteDialogComponent);
64+
component = fixture.componentInstance;
65+
fixture.detectChanges();
66+
});
67+
68+
it('should create', () => {
69+
expect(component).toBeTruthy();
70+
});
71+
72+
it('should have submitting signal initialized to false', () => {
73+
const testable = component as ChartDeleteDialogComponentTestable;
74+
expect(testable.submitting()).toBe(false);
75+
});
76+
77+
describe('onDelete', () => {
78+
it('should call deleteSavedQuery service method', () => {
79+
component.onDelete();
80+
expect(mockSavedQueriesService.deleteSavedQuery).toHaveBeenCalledWith('conn-1', '1');
81+
});
82+
83+
it('should set submitting to true during delete', () => {
84+
component.onDelete();
85+
// The deleteSavedQuery returns synchronously in the mock, so submitting would be false after
86+
// In real usage, submitting would be true while the request is in flight
87+
expect(mockSavedQueriesService.deleteSavedQuery).toHaveBeenCalled();
88+
});
89+
90+
it('should close dialog with true on success', () => {
91+
component.onDelete();
92+
expect(mockDialogRef.close).toHaveBeenCalledWith(true);
93+
});
94+
95+
it('should set submitting to false after successful delete', () => {
96+
const testable = component as ChartDeleteDialogComponentTestable;
97+
component.onDelete();
98+
expect(testable.submitting()).toBe(false);
99+
});
100+
101+
it('should set submitting to false on error', () => {
102+
const testable = component as ChartDeleteDialogComponentTestable;
103+
vi.mocked(mockSavedQueriesService.deleteSavedQuery!).mockReturnValue(
104+
throwError(() => new Error('Delete failed')),
105+
);
106+
component.onDelete();
107+
expect(testable.submitting()).toBe(false);
108+
});
109+
110+
it('should not close dialog on error', () => {
111+
vi.mocked(mockSavedQueriesService.deleteSavedQuery!).mockReturnValue(
112+
throwError(() => new Error('Delete failed')),
113+
);
114+
component.onDelete();
115+
expect(mockDialogRef.close).not.toHaveBeenCalled();
116+
});
117+
});
118+
});

‎frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { CommonModule } from '@angular/common';
2-
import { Component, Inject } from '@angular/core';
2+
import { Component, Inject, signal } from '@angular/core';
33
import { MatButtonModule } from '@angular/material/button';
44
import { MAT_DIALOG_DATA, MatDialogModule, MatDialogRef } from '@angular/material/dialog';
55
import { MatIconModule } from '@angular/material/icon';
@@ -14,7 +14,7 @@ import { SavedQueriesService } from 'src/app/services/saved-queries.service';
1414
imports: [CommonModule, MatDialogModule, MatButtonModule, MatIconModule],
1515
})
1616
export class ChartDeleteDialogComponent {
17-
public submitting = false;
17+
protected submitting = signal(false);
1818

1919
constructor(
2020
@Inject(MAT_DIALOG_DATA) public data: { query: SavedQuery; connectionId: string },
@@ -24,17 +24,17 @@ export class ChartDeleteDialogComponent {
2424
) {}
2525

2626
onDelete(): void {
27-
this.submitting = true;
27+
this.submitting.set(true);
2828
this._savedQueries.deleteSavedQuery(this.data.connectionId, this.data.query.id).subscribe({
2929
next: () => {
3030
this.angulartics2.eventTrack.next({
3131
action: 'Charts: saved query deleted successfully',
3232
});
33-
this.submitting = false;
33+
this.submitting.set(false);
3434
this.dialogRef.close(true);
3535
},
3636
error: () => {
37-
this.submitting = false;
37+
this.submitting.set(false);
3838
},
3939
});
4040
}

‎frontend/src/app/components/charts/chart-edit/chart-edit.component.html‎

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,20 @@
55
<button mat-icon-button (click)="cancel()" matTooltip="Back to list">
66
<mat-icon>arrow_back</mat-icon>
77
</button>
8-
<h1 class="mat-h1">{{ isEditMode ? 'Edit Saved Query' : 'Create Saved Query' }}</h1>
8+
<h1 class="mat-h1">{{ isEditMode() ? 'Edit Saved Query' : 'Create Saved Query' }}</h1>
99
</div>
1010

11-
<div *ngIf="loading" class="loading-container">
11+
<div *ngIf="loading()" class="loading-container">
1212
<mat-spinner diameter="40"></mat-spinner>
1313
</div>
1414

15-
<div *ngIf="!loading" class="chart-edit-content">
15+
<div *ngIf="!loading()" class="chart-edit-content">
1616
<div class="query-details">
1717
<mat-form-field appearance="outline" class="name-field">
1818
<mat-label>Query Name</mat-label>
1919
<input matInput
20-
[(ngModel)]="queryName"
20+
[ngModel]="queryName()"
21+
(ngModelChange)="queryName.set($event)"
2122
placeholder="Enter a name for this query"
2223
data-testid="query-name-input"
2324
required>
@@ -26,7 +27,8 @@ <h1 class="mat-h1">{{ isEditMode ? 'Edit Saved Query' : 'Create Saved Query' }}<
2627
<mat-form-field appearance="outline" class="description-field">
2728
<mat-label>Description (optional)</mat-label>
2829
<input matInput
29-
[(ngModel)]="queryDescription"
30+
[ngModel]="queryDescription()"
31+
(ngModelChange)="queryDescription.set($event)"
3032
placeholder="Add a description"
3133
data-testid="query-description-input">
3234
</mat-form-field>
@@ -38,35 +40,35 @@ <h1 class="mat-h1">{{ isEditMode ? 'Edit Saved Query' : 'Create Saved Query' }}<
3840
<h3>SQL Query</h3>
3941
<button mat-stroked-button
4042
(click)="testQuery()"
41-
[disabled]="!canTest"
43+
[disabled]="!canTest()"
4244
data-testid="test-query-button">
43-
<mat-icon *ngIf="!testing">play_arrow</mat-icon>
44-
<mat-spinner *ngIf="testing" diameter="18"></mat-spinner>
45-
{{ testing ? 'Testing...' : 'Test Query' }}
45+
<mat-icon *ngIf="!testing()">play_arrow</mat-icon>
46+
<mat-spinner *ngIf="testing()" diameter="18"></mat-spinner>
47+
{{ testing() ? 'Testing...' : 'Test Query' }}
4648
</button>
4749
</div>
4850
<div class="code-editor-box" data-hj-suppress>
4951
<ngs-code-editor
5052
[theme]="codeEditorTheme"
51-
[codeModel]="codeModel"
53+
[codeModel]="codeModel()"
5254
[options]="codeEditorOptions"
5355
(valueChanged)="onCodeChange($event)">
5456
</ngs-code-editor>
5557
</div>
5658
</div>
5759

58-
<div class="preview-section" *ngIf="showResults">
60+
<div class="preview-section" *ngIf="showResults()">
5961
<div class="section-header">
6062
<h3>Chart Preview</h3>
61-
<span class="execution-time" *ngIf="executionTime !== null">
62-
Executed in {{ executionTime }}ms
63+
<span class="execution-time" *ngIf="executionTime() !== null">
64+
Executed in {{ executionTime() }}ms
6365
</span>
6466
</div>
6567

6668
<div class="chart-config">
6769
<mat-form-field appearance="outline" class="chart-config-field">
6870
<mat-label>Chart Type</mat-label>
69-
<mat-select [(ngModel)]="chartType" data-testid="chart-type-select">
71+
<mat-select [ngModel]="chartType()" (ngModelChange)="chartType.set($event)" data-testid="chart-type-select">
7072
<mat-option *ngFor="let type of chartTypes" [value]="type.value">
7173
{{ type.label }}
7274
</mat-option>
@@ -75,56 +77,56 @@ <h3>Chart Preview</h3>
7577

7678
<mat-form-field appearance="outline" class="chart-config-field">
7779
<mat-label>Label Column</mat-label>
78-
<mat-select [(ngModel)]="labelColumn" data-testid="label-column-select">
79-
<mat-option *ngFor="let col of resultColumns" [value]="col">
80+
<mat-select [ngModel]="labelColumn()" (ngModelChange)="labelColumn.set($event)" data-testid="label-column-select">
81+
<mat-option *ngFor="let col of resultColumns()" [value]="col">
8082
{{ col }}
8183
</mat-option>
8284
</mat-select>
8385
</mat-form-field>
8486

8587
<mat-form-field appearance="outline" class="chart-config-field">
8688
<mat-label>Value Column</mat-label>
87-
<mat-select [(ngModel)]="valueColumn" data-testid="value-column-select">
88-
<mat-option *ngFor="let col of resultColumns" [value]="col">
89+
<mat-select [ngModel]="valueColumn()" (ngModelChange)="valueColumn.set($event)" data-testid="value-column-select">
90+
<mat-option *ngFor="let col of resultColumns()" [value]="col">
8991
{{ col }}
9092
</mat-option>
9193
</mat-select>
9294
</mat-form-field>
9395
</div>
9496

95-
<div class="chart-container" *ngIf="hasChartData">
97+
<div class="chart-container" *ngIf="hasChartData()">
9698
<app-chart-preview
97-
[chartType]="chartType"
98-
[data]="testResults"
99-
[labelColumn]="labelColumn"
100-
[valueColumn]="valueColumn">
99+
[chartType]="chartType()"
100+
[data]="testResults()"
101+
[labelColumn]="labelColumn()"
102+
[valueColumn]="valueColumn()">
101103
</app-chart-preview>
102104
</div>
103105

104-
<div class="no-chart-data" *ngIf="!hasChartData && testResults.length > 0">
106+
<div class="no-chart-data" *ngIf="!hasChartData() && testResults().length > 0">
105107
<p>Select label and value columns to display the chart</p>
106108
</div>
107109
</div>
108110
</div>
109111

110-
<div class="results-section" *ngIf="showResults && testResults.length > 0">
112+
<div class="results-section" *ngIf="showResults() && testResults().length > 0">
111113
<div class="section-header">
112-
<h3>Query Results ({{ testResults.length }} rows)</h3>
114+
<h3>Query Results ({{ testResults().length }} rows)</h3>
113115
</div>
114116
<div class="results-table-container">
115-
<table mat-table [dataSource]="testResults" class="results-table mat-elevation-z1">
116-
<ng-container *ngFor="let column of resultColumns" [matColumnDef]="column">
117+
<table mat-table [dataSource]="testResults()" class="results-table mat-elevation-z1">
118+
<ng-container *ngFor="let column of resultColumns()" [matColumnDef]="column">
117119
<th mat-header-cell *matHeaderCellDef>{{ column }}</th>
118120
<td mat-cell *matCellDef="let row">{{ row[column] }}</td>
119121
</ng-container>
120122

121-
<tr mat-header-row *matHeaderRowDef="resultColumns"></tr>
122-
<tr mat-row *matRowDef="let row; columns: resultColumns;"></tr>
123+
<tr mat-header-row *matHeaderRowDef="resultColumns()"></tr>
124+
<tr mat-row *matRowDef="let row; columns: resultColumns();"></tr>
123125
</table>
124126
</div>
125127
</div>
126128

127-
<div class="no-results" *ngIf="showResults && testResults.length === 0">
129+
<div class="no-results" *ngIf="showResults() && testResults().length === 0">
128130
<mat-icon>info</mat-icon>
129131
<p>Query executed successfully but returned no results.</p>
130132
</div>
@@ -135,10 +137,10 @@ <h3>Query Results ({{ testResults.length }} rows)</h3>
135137
</button>
136138
<button mat-flat-button color="primary"
137139
(click)="saveQuery()"
138-
[disabled]="!canSave"
140+
[disabled]="!canSave()"
139141
data-testid="save-query-button">
140-
<mat-spinner *ngIf="saving" diameter="18"></mat-spinner>
141-
{{ saving ? 'Saving...' : (isEditMode ? 'Update Query' : 'Save Query') }}
142+
<mat-spinner *ngIf="saving()" diameter="18"></mat-spinner>
143+
{{ saving() ? 'Saving...' : (isEditMode() ? 'Update Query' : 'Save Query') }}
142144
</button>
143145
</div>
144146
</div>

0 commit comments

Comments
 (0)