refactor: migrate signal-based services from HttpClient to ApiService by gugu · Pull Request #1608 · rocket-admin/rocketadmin

gugu · 2026-02-16T14:43:06Z

Replace direct HttpClient (_http) usage with centralized ApiService (_api) in DashboardsService and SavedQueriesService. Convert Observable-based methods to async/await Promises and update all callers accordingly.

Replace direct HttpClient (_http) usage with centralized ApiService (_api) in DashboardsService and SavedQueriesService. Convert Observable-based methods to async/await Promises and update all callers accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR migrates DashboardsService and SavedQueriesService off direct HttpClient usage onto the centralized ApiService, and updates affected components/tests to use Promise-based async/await instead of Observables.

Changes:

Refactor SavedQueriesService and DashboardsService to use ApiService (including httpResource-backed resources).
Convert saved-query call sites to async/await (DashboardWidgetComponent, ChartEditComponent, ChartDeleteDialogComponent).
Update unit tests to mock Promise-returning service methods and await async component methods.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
frontend/src/app/services/saved-queries.service.ts	Replace `HttpClient`/RxJS resource + Observable APIs with `ApiService.resource` and Promise-based CRUD/execute/test methods
frontend/src/app/services/dashboards.service.ts	Replace `HttpClient` + `resource` loaders with `ApiService.resource` and Promise-based CRUD/widget methods
frontend/src/app/components/dashboards/widget-renderers/dashboard-widget/dashboard-widget.component.ts	Update saved query fetch/execute to async/await
frontend/src/app/components/charts/chart-edit/chart-edit.component.ts	Convert query load/test/save flows from Observable subscriptions to async/await with `finally` cleanup
frontend/src/app/components/charts/chart-edit/chart-edit.component.spec.ts	Update mocks/tests for Promise-based service methods and async component methods
frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.ts	Convert delete flow from subscription to async/await
frontend/src/app/components/charts/chart-delete-dialog/chart-delete-dialog.component.spec.ts	Update mocks/tests for Promise-based delete and async handler

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-16T14:46:51Z

 			const [query, result] = await Promise.all([
-				firstValueFrom(this._savedQueries.fetchSavedQuery(this.connectionId, this.widget.query_id)),
-				firstValueFrom(this._savedQueries.executeSavedQuery(this.connectionId, this.widget.query_id)),
+				this._savedQueries.fetchSavedQuery(this.connectionId, this.widget.query_id),
+				this._savedQueries.executeSavedQuery(this.connectionId, this.widget.query_id),
 			]);

 			this.savedQuery.set(query);
-			this.queryData.set(result.data);
+			if (result) {


fetchSavedQuery/executeSavedQuery now resolve null on error (via ApiService) instead of rejecting. In this flow, a null result leaves error unset and the widget can render with empty data/name and no failure state. Add an explicit if (!query || !result) branch (set an error message and return) so errors don't become silent successes.

Copilot · 2026-02-16T14:46:52Z

@@ -169,17 +126,7 @@ export class DashboardsService {
 		widgetId: string,
 		payload: UpdateWidgetPayload,
 	): Promise<DashboardWidget | null> {
-		try {
-			const widget = await firstValueFrom(
-				this._http.put<DashboardWidget>(`/dashboard/${dashboardId}/widget/${widgetId}/${connectionId}`, payload),
-			);
-			return widget;
-		} catch (err: unknown) {
-			const error = err as { error?: { message?: string } };
-			console.log(err);
-			this._notifications.showErrorSnackbar(error?.error?.message || 'Failed to update widget');
-			return null;
-		}
+		return this._api.put<DashboardWidget>(`/dashboard/${dashboardId}/widget/${widgetId}/${connectionId}`, payload);
 	}


These _api.put(...) calls don't provide an errorMessage, so failures can degrade to a generic "Unknown error" snackbar compared to the previous explicit "Failed to update widget" fallback. Pass an errorMessage option to _api.put to preserve the prior behavior and make errors actionable.

Copilot · 2026-02-16T14:46:52Z

-			this._notifications.showErrorSnackbar(error?.error?.message || 'Failed to update widget position');
-			return null;
-		}
+		return this._api.put<DashboardWidget>(`/dashboard/${dashboardId}/widget/${widgetId}/${connectionId}`, payload);


Same as updateWidget: consider providing an errorMessage option (e.g., "Failed to update widget position") so user-facing errors remain specific when the backend doesn't return error.message.

Suggested change

return this._api.put<DashboardWidget>(`/dashboard/${dashboardId}/widget/${widgetId}/${connectionId}`, payload);

return this._api.put<DashboardWidget>(

`/dashboard/${dashboardId}/widget/${widgetId}/${connectionId}`,

payload,

{

errorMessage: 'Failed to update widget position',

},

);

Copilot · 2026-02-16T14:46:52Z

 		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);
 		});


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 · 2026-02-16T14:46:53Z

-			}),
-		);
+	async fetchSavedQuery(connectionId: string, queryId: string): Promise<SavedQuery | null> {
+		return this._api.get<SavedQuery>(`/connection/${connectionId}/saved-query/${queryId}`);


fetchSavedQuery no longer supplies a fallback errorMessage. Since ApiService will otherwise default to the server-provided message (or "Unknown error" when missing), this is a UX regression vs the previous "Failed to fetch saved query" behavior. Pass { errorMessage: 'Failed to fetch saved query' } to _api.get (and keep the server message as details if desired).

Suggested change

return this._api.get<SavedQuery>(`/connection/${connectionId}/saved-query/${queryId}`);

return this._api.get<SavedQuery>(`/connection/${connectionId}/saved-query/${queryId}`, {

errorMessage: 'Failed to fetch saved query',

});

Copilot · 2026-02-16T14:46:53Z

+	async createSavedQuery(connectionId: string, payload: CreateSavedQueryPayload): Promise<SavedQuery | null> {
+		const query = await this._api.post<SavedQuery>(`/connection/${connectionId}/saved-query`, payload, {
+			successMessage: 'Saved query created successfully',
+		});


The create call only sets successMessage; on failures, ApiService may surface "Unknown error" if the backend doesn't provide error.message. To preserve the previous explicit fallback, provide an errorMessage (e.g., "Failed to create saved query") in the ApiRequestOptions here (and similarly for update/delete).

Copilot · 2026-02-16T14:46:53Z

+		return this._api.post<QueryExecutionResult>(
+			`/connection/${connectionId}/saved-query/${queryId}/execute`,
+			{},
+			tableName ? { params: { tableName } } : undefined,


executeSavedQuery no longer provides a fallback error message. Because ApiService.post resolves to null on error (rather than throwing), callers may not enter catch blocks and you can end up with silent failures unless an errorMessage is provided and/or the caller checks for null. Consider passing { errorMessage: 'Failed to execute query' } and ensuring callers handle a null result.

Suggested change

return this._api.post<QueryExecutionResult>(

`/connection/${connectionId}/saved-query/${queryId}/execute`,

{},

tableName ? { params: { tableName } } : undefined,

const options = tableName

? { params: { tableName }, errorMessage: 'Failed to execute query' }

: { errorMessage: 'Failed to execute query' };

return this._api.post<QueryExecutionResult>(

`/connection/${connectionId}/saved-query/${queryId}/execute`,

{},

options,

Copilot · 2026-02-16T14:46:53Z

-			}),
-		);
+	async testQuery(connectionId: string, payload: TestQueryPayload): Promise<TestQueryResult | null> {
+		return this._api.post<TestQueryResult>(`/connection/${connectionId}/query/test`, payload);


testQuery no longer passes an errorMessage, so failures can degrade to a generic "Unknown error" snackbar depending on backend response shape. Add an explicit errorMessage option (e.g., "Failed to test query") to preserve the previous fallback behavior.

Suggested change

return this._api.post<TestQueryResult>(`/connection/${connectionId}/query/test`, payload);

return this._api.post<TestQueryResult>(`/connection/${connectionId}/query/test`, payload, {

errorMessage: 'Failed to test query',

});

Resolve merge conflicts in chart-edit component: keep async/await pattern from signal migration, adopt /panels/ route rename from main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings February 16, 2026 14:43

Copilot started reviewing on behalf of gugu February 16, 2026 14:43 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

gugu requested a review from lyubov-voloshko February 17, 2026 11:17

gugu and others added 4 commits February 19, 2026 11:05

Merge branch 'main' into frontend_signal_services_api_migration

c856499

Merge branch 'main' into frontend_signal_services_api_migration

c7ec073

Merge branch 'main' into frontend_signal_services_api_migration

ff34f91

Resolve merge conflicts in chart-edit component: keep async/await pattern from signal migration, adopt /panels/ route rename from main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Merge branch 'main' into frontend_signal_services_api_migration

caca347

lyubov-voloshko merged commit 414cce2 into main Feb 20, 2026
15 of 17 checks passed

lyubov-voloshko deleted the frontend_signal_services_api_migration branch February 20, 2026 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

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

refactor: migrate signal-based services from HttpClient to ApiService#1608
lyubov-voloshko merged 5 commits into
mainfrom
frontend_signal_services_api_migration

gugu commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Copilot AI Feb 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-		return this._api.get<SavedQuery>(`/connection/${connectionId}/saved-query/${queryId}`);
+		return this._api.get<SavedQuery>(`/connection/${connectionId}/saved-query/${queryId}`, {
+			errorMessage: 'Failed to fetch saved query',
+		});

-		return this._api.post<TestQueryResult>(`/connection/${connectionId}/query/test`, payload);
+		return this._api.post<TestQueryResult>(`/connection/${connectionId}/query/test`, payload, {
+			errorMessage: 'Failed to test query',
+		});

Uh oh!

Uh oh!

Conversation

gugu commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants