Skip to content

refactor: migrate signal-based services from HttpClient to ApiService #1608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lyubov-voloshko merged 5 commits into from
Feb 20, 2026
Merged

refactor: migrate signal-based services from HttpClient to ApiService #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ba929b2
refactor: migrate signal-based services from HttpClient to ApiService
gugu Feb 16, 2026
c856499
Merge branch 'main' into frontend_signal_services_api_migration
gugu Feb 19, 2026
c7ec073
Merge branch 'main' into frontend_signal_services_api_migration
gugu Feb 19, 2026
ff34f91
Merge branch 'main' into frontend_signal_services_api_migration
gugu Feb 19, 2026
caca347
Merge branch 'main' into frontend_signal_services_api_migration
lyubov-voloshko Feb 20, 2026
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
37 changes: 16 additions & 21 deletions frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { MAT_DIALOG_DATA, MatDialogModule, MatDialogRef } from '@angular/materia
import { MatSnackBarModule } from '@angular/material/snack-bar';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { Angulartics2Module } from 'angulartics2';
import { of, throwError } from 'rxjs';
import { SavedQuery } from 'src/app/models/saved-query';
import { SavedQueriesService } from 'src/app/services/saved-queries.service';
import { ChartDeleteDialogComponent } from './chart-delete-dialog.component';
Expand Down Expand Up @@ -36,7 +35,7 @@ describe('ChartDeleteDialogComponent', () => {

beforeEach(async () => {
mockSavedQueriesService = {
deleteSavedQuery: vi.fn().mockReturnValue(of(mockSavedQuery)),
deleteSavedQuery: vi.fn().mockResolvedValue(mockSavedQuery),
};

mockDialogRef = {
Expand Down Expand Up @@ -78,43 +77,39 @@ describe('ChartDeleteDialogComponent', () => {
});

describe('onDelete', () => {
it('should call deleteSavedQuery service method', () => {
component.onDelete();
it('should call deleteSavedQuery service method', async () => {
await component.onDelete();
expect(mockSavedQueriesService.deleteSavedQuery).toHaveBeenCalledWith('conn-1', '1');
});

it('should set submitting to true during delete', () => {
component.onDelete();
// The deleteSavedQuery returns synchronously in the mock, so submitting would be false after
// In real usage, submitting would be true while the request is in flight
expect(mockSavedQueriesService.deleteSavedQuery).toHaveBeenCalled();
// The deleteSavedQuery returns asynchronously, so submitting should be true while in flight
const testable = component as ChartDeleteDialogComponentTestable;
expect(testable.submitting()).toBe(true);
});
Comment on lines 85 to 90

Copilot AI Feb 16, 2026

Copy link

Choose a reason for hiding this comment

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

This test calls the async onDelete() without awaiting the returned Promise. That leaves a pending async task that can leak into subsequent tests and cause flakiness. Capture the Promise, assert submitting() is true, then await the Promise before the test completes.

Copilot uses AI. Check for mistakes.

it('should close dialog with true on success', () => {
component.onDelete();
it('should close dialog with true on success', async () => {
await component.onDelete();
expect(mockDialogRef.close).toHaveBeenCalledWith(true);
});

it('should set submitting to false after successful delete', () => {
it('should set submitting to false after successful delete', async () => {
const testable = component as ChartDeleteDialogComponentTestable;
component.onDelete();
await component.onDelete();
expect(testable.submitting()).toBe(false);
});

it('should set submitting to false on error', () => {
it('should set submitting to false on error', async () => {
const testable = component as ChartDeleteDialogComponentTestable;
vi.mocked(mockSavedQueriesService.deleteSavedQuery!).mockReturnValue(
throwError(() => new Error('Delete failed')),
);
component.onDelete();
vi.mocked(mockSavedQueriesService.deleteSavedQuery!).mockResolvedValue(null);
await component.onDelete();
expect(testable.submitting()).toBe(false);
});

it('should not close dialog on error', () => {
vi.mocked(mockSavedQueriesService.deleteSavedQuery!).mockReturnValue(
throwError(() => new Error('Delete failed')),
);
component.onDelete();
it('should not close dialog on error', async () => {
vi.mocked(mockSavedQueriesService.deleteSavedQuery!).mockResolvedValue(null);
await component.onDelete();
expect(mockDialogRef.close).not.toHaveBeenCalled();
});
});
Expand Down
25 changes: 11 additions & 14 deletions frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,17 @@ export class ChartDeleteDialogComponent {
private angulartics2: Angulartics2,
) {}

onDelete(): void {
async onDelete(): Promise<void> {
this.submitting.set(true);
this._savedQueries.deleteSavedQuery(this.data.connectionId, this.data.query.id).subscribe({
next: () => {
this.angulartics2.eventTrack.next({
action: 'Charts: saved query deleted successfully',
});
posthog.capture('Charts: saved query deleted successfully');
this.submitting.set(false);
this.dialogRef.close(true);
},
error: () => {
this.submitting.set(false);
},
});
const result = await this._savedQueries.deleteSavedQuery(this.data.connectionId, this.data.query.id);
this.submitting.set(false);

if (result) {
this.angulartics2.eventTrack.next({
action: 'Charts: saved query deleted successfully',
});
posthog.capture('Charts: saved query deleted successfully');
this.dialogRef.close(true);
}
}
}
36 changes: 17 additions & 19 deletions frontend/src/app/components/charts/chart-edit/chart-edit.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,13 @@ describe('ChartEditComponent', () => {

beforeEach(async () => {
mockSavedQueriesService = {
fetchSavedQuery: vi.fn().mockReturnValue(of(mockSavedQuery)),
createSavedQuery: vi.fn().mockReturnValue(of(mockSavedQuery)),
updateSavedQuery: vi.fn().mockReturnValue(of(mockSavedQuery)),
testQuery: vi.fn().mockReturnValue(
of({
data: [{ name: 'John', count: 10 }],
execution_time_ms: 50,
}),
),
fetchSavedQuery: vi.fn().mockResolvedValue(mockSavedQuery),
createSavedQuery: vi.fn().mockResolvedValue(mockSavedQuery),
updateSavedQuery: vi.fn().mockResolvedValue(mockSavedQuery),
testQuery: vi.fn().mockResolvedValue({
data: [{ name: 'John', count: 10 }],
execution_time_ms: 50,
}),
};

mockConnectionsService = {
Expand Down Expand Up @@ -194,40 +192,40 @@ describe('ChartEditComponent', () => {
});

describe('testQuery', () => {
it('should call testQuery service method', () => {
it('should call testQuery service method', async () => {
const testable = component as ChartEditComponentTestable;
testable.queryText.set('SELECT * FROM users');
component.testQuery();
await component.testQuery();
expect(mockSavedQueriesService.testQuery).toHaveBeenCalledWith('conn-1', {
query_text: 'SELECT * FROM users',
});
});

it('should set results and columns after successful test', () => {
it('should set results and columns after successful test', async () => {
const testable = component as ChartEditComponentTestable;
testable.queryText.set('SELECT * FROM users');
component.testQuery();
await component.testQuery();
expect(testable.testResults()).toEqual([{ name: 'John', count: 10 }]);
expect(testable.resultColumns()).toEqual(['name', 'count']);
expect(testable.executionTime()).toBe(50);
});

it('should auto-select label column and first series', () => {
it('should auto-select label column and first series', async () => {
const testable = component as ChartEditComponentTestable;
testable.queryText.set('SELECT * FROM users');
component.testQuery();
await component.testQuery();
expect(testable.labelColumn()).toBe('name');
expect(testable.seriesList()).toEqual([{ value_column: 'count' }]);
});
});

describe('saveQuery', () => {
it('should call createSavedQuery in create mode', () => {
it('should call createSavedQuery in create mode', async () => {
const testable = component as ChartEditComponentTestable;
testable.isEditMode.set(false);
testable.queryName.set('New Query');
testable.queryText.set('SELECT 1');
component.saveQuery();
await component.saveQuery();
expect(mockSavedQueriesService.createSavedQuery).toHaveBeenCalledWith('conn-1', {
name: 'New Query',
description: undefined,
Expand All @@ -238,11 +236,11 @@ describe('ChartEditComponent', () => {
});
});

it('should navigate to charts list after save', () => {
it('should navigate to charts list after save', async () => {
const testable = component as ChartEditComponentTestable;
testable.queryName.set('New Query');
testable.queryText.set('SELECT 1');
component.saveQuery();
await component.saveQuery();
expect(router.navigate).toHaveBeenCalledWith(['/panels', 'conn-1']);
});
});
Expand Down
Loading
Loading