Skip to content

Commit

Permalink
portalicious: fix request caching
Browse files Browse the repository at this point in the history
AB#31036

On some requests in the export functionality, the same request + http parameters were being sent to the backend, even when the http parameters being used to make the request in the service were different.

I started by changing the `generateQueryOptions` function to have an exhaustive list of query keys, thinking that the lack of `requestOptions` in this array was the culprit.

It was not.

So then I refactored the `HttpWrapper.performRequest` to only accept HttpParam _objects_ instead of HttpParam _class instances_. My thinking here was that tanstack-query was struggling to serialize class instances as opposed to objects.

This was partially true, but did not solve the problem entirely.

I ultimately added the `paginateQuery` option to the `generateQueryOptions`, because I realised that the issue was that the call to the Signal `paginateQuery` needed to happen _inside_ the `queryFn` in order to allow the "signals to signal".

I'm not sure any of my thought process actually makes any sense, but hopefully you find the resulting code cleaner either way, given that now we don't have to worry about creating HttpParams anywhere anymore, and we just deal with objects.
  • Loading branch information
aberonni committed Oct 31, 2024
1 parent 0f66838 commit b82d27a
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 121 deletions.
30 changes: 28 additions & 2 deletions interfaces/Portalicious/src/app/domains/domain-api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import {
HttpWrapperService,
Perform121ServiceRequestParams,
} from '~/services/http-wrapper.service';
import {
PaginateQuery,
PaginateQueryService,
} from '~/services/paginate-query.service';

export abstract class DomainApiService {
protected httpWrapperService = inject(HttpWrapperService);
protected paginateQueryService = inject(PaginateQueryService);
protected queryClient = injectQueryClient();

protected pathToQueryKey = (
Expand All @@ -28,6 +33,7 @@ export abstract class DomainApiService {
processResponse,
requestOptions = {},
method = 'GET',
paginateQuery,
...opts
}: {
path: Parameters<typeof DomainApiService.prototype.pathToQueryKey>[0];
Expand All @@ -37,16 +43,36 @@ export abstract class DomainApiService {
'endpoint' | 'method'
>;
method?: Perform121ServiceRequestParams['method'];
paginateQuery?: Signal<PaginateQuery | undefined>;
} & Partial<UndefinedInitialDataOptions<ProcessedResponseShape>>) {
return () => {
const queryKey = this.pathToQueryKey(path);
const endpoint = queryKey.join('/');

return queryOptions({
...opts,
// eslint-disable-next-line @tanstack/query/exhaustive-deps
queryKey,
queryKey: [
...queryKey,
method,
endpoint,
JSON.stringify(requestOptions),
paginateQuery && paginateQuery(),
processResponse,
],
queryFn: async () => {
if (paginateQuery) {
const { params } = requestOptions;
const paginateQueryParams =
this.paginateQueryService.paginateQueryToHttpParamsObject(
paginateQuery(),
);

requestOptions.params = {
...params,
...paginateQueryParams,
};
}

const response =
await this.httpWrapperService.perform121ServiceRequest<BackendDataShape>(
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Injectable, Signal } from '@angular/core';

import { DomainApiService } from '~/domains/domain-api.service';
import { ProjectMetrics } from '~/domains/metric/metric.model';

const BASE_ENDPOINT = (projectId: Signal<number>) => [
'programs',
projectId,
'metrics',
];

@Injectable({
providedIn: 'root',
})
export class MetricApiService extends DomainApiService {
getProjectSummaryMetrics(projectId: Signal<number>) {
return this.generateQueryOptions<ProjectMetrics>({
path: [...BASE_ENDPOINT(projectId), 'program-stats-summary'],
});
}

public invalidateCache(projectId: Signal<number>): Promise<void> {
return this.queryClient.invalidateQueries({
queryKey: this.pathToQueryKey(BASE_ENDPOINT(projectId)),
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { ProgramStats } from '@121-service/src/metrics/dto/program-stats.dto';

import { Dto } from '~/utils/dto-type';
// TODO: AB#30152 This type should be refactored to use Dto121Service
export type ProjectMetrics = Dto<ProgramStats>;
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import { HttpParams } from '@angular/common/http';
import { inject, Injectable, Signal } from '@angular/core';

import { uniqBy } from 'lodash';

import { ActionReturnDto } from '@121-service/src/actions/dto/action-return.dto';
import { ExportType } from '@121-service/src/metrics/enum/export-type.enum';

import { DomainApiService } from '~/domains/domain-api.service';
import { ATTRIBUTE_LABELS } from '~/domains/project/project.helper';
import {
Attribute,
AttributeWithTranslatedLabel,
Project,
ProjectMetrics,
ProjectUser,
ProjectUserAssignment,
ProjectUserWithRolesLabel,
} from '~/domains/project/project.model';
import { Role } from '~/domains/role/role.model';
import { TranslatableStringService } from '~/services/translatable-string.service';
import { Dto } from '~/utils/dto-type';

const BASE_ENDPOINT = 'programs';

Expand Down Expand Up @@ -54,12 +56,6 @@ export class ProjectApiService extends DomainApiService {
});
}

getProjectSummaryMetrics(projectId: Signal<number>) {
return this.generateQueryOptions<ProjectMetrics>({
path: [BASE_ENDPOINT, projectId, 'metrics/program-stats-summary'],
});
}

getProjectUsers(projectId: Signal<number>) {
return this.generateQueryOptions<
ProjectUser[],
Expand Down Expand Up @@ -95,23 +91,19 @@ export class ProjectApiService extends DomainApiService {
includeTemplateDefaultAttributes?: boolean;
filterShowInPeopleAffectedTable?: boolean;
}) {
const params = new HttpParams({
fromObject: {
includeCustomAttributes,
includeProgramQuestions,
includeFspQuestions,
includeTemplateDefaultAttributes,
filterShowInPeopleAffectedTable,
},
});

return this.generateQueryOptions<
Attribute[],
AttributeWithTranslatedLabel[]
>({
path: [BASE_ENDPOINT, projectId, 'attributes'],
requestOptions: {
params,
params: {
includeCustomAttributes,
includeProgramQuestions,
includeFspQuestions,
includeTemplateDefaultAttributes,
filterShowInPeopleAffectedTable,
},
},
processResponse: (attributes) => {
return uniqBy(attributes, 'name').map((attribute) => {
Expand Down Expand Up @@ -224,12 +216,10 @@ export class ProjectApiService extends DomainApiService {
'financial-service-providers/intersolve-voucher/vouchers',
],
requestOptions: {
params: new HttpParams({
fromObject: {
referenceId: voucherReferenceId,
payment: paymentId.toString(),
},
}),
params: {
referenceId: voucherReferenceId,
payment: paymentId.toString(),
},
responseAsBlob: true,
},
});
Expand All @@ -244,18 +234,34 @@ export class ProjectApiService extends DomainApiService {
registrationReferenceId: string;
paymentId: number;
}) {
let params = new HttpParams();
params = params.append('referenceId', registrationReferenceId);
params = params.append('payment', paymentId);

return this.generateQueryOptions<number>({
path: [
BASE_ENDPOINT,
projectId,
'financial-service-providers/intersolve-voucher/vouchers/balance',
],
requestOptions: {
params,
params: {
referenceId: registrationReferenceId,
payment: paymentId,
},
},
});
}

getLatestAction({
projectId,
actionType,
}: {
projectId: Signal<number>;
actionType: ExportType;
}) {
return this.generateQueryOptions<Dto<ActionReturnDto>>({
path: [BASE_ENDPOINT, projectId, 'actions'],
requestOptions: {
params: {
actionType,
},
},
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { ProgramStats } from '@121-service/src/metrics/dto/program-stats.dto';
import { FoundProgramDto } from '@121-service/src/programs/dto/found-program.dto';
import { ProgramController } from '@121-service/src/programs/programs.controller';
import { UserController } from '@121-service/src/user/user.controller';
Expand All @@ -9,9 +8,6 @@ import { ArrayElement } from '~/utils/type-helpers';
// TODO: AB#30152 This type should be refactored to use Dto121Service
export type Project = Dto<FoundProgramDto>;

// TODO: AB#30152 This type should be refactored to use Dto121Service
export type ProjectMetrics = Dto<ProgramStats>;

export type ProjectUser = ArrayElement<
Dto121Service<UserController['getUsersInProgram']>
>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { HttpParams } from '@angular/common/http';
import { inject, Injectable, Signal } from '@angular/core';
import { Injectable, Signal } from '@angular/core';

import { queryOptions } from '@tanstack/angular-query-experimental';

Expand All @@ -17,10 +16,7 @@ import {
VisaCardAction,
WalletWithCards,
} from '~/domains/registration/registration.model';
import {
PaginateQuery,
PaginateQueryService,
} from '~/services/paginate-query.service';
import { PaginateQuery } from '~/services/paginate-query.service';

const BASE_ENDPOINT = (projectId: Signal<number>) => [
'programs',
Expand All @@ -32,31 +28,14 @@ const BASE_ENDPOINT = (projectId: Signal<number>) => [
providedIn: 'root',
})
export class RegistrationApiService extends DomainApiService {
paginateQueryService = inject(PaginateQueryService);

getManyByQuery(
projectId: Signal<number>,
paginateQuery: Signal<PaginateQuery | undefined>,
) {
return () => {
const path = [...BASE_ENDPOINT(projectId)];

return queryOptions({
queryKey: [path, paginateQuery()],
queryFn: async () =>
this.httpWrapperService.perform121ServiceRequest<FindAllRegistrationsResult>(
{
method: 'GET',
endpoint: this.pathToQueryKey(path).join('/'),
params:
this.paginateQueryService.paginateQueryToHttpParams(
paginateQuery(),
),
},
),
enabled: () => !!paginateQuery(),
});
};
return this.generateQueryOptions<FindAllRegistrationsResult>({
path: [...BASE_ENDPOINT(projectId)],
paginateQuery,
});
}

getRegistrationById(
Expand Down Expand Up @@ -100,7 +79,9 @@ export class RegistrationApiService extends DomainApiService {
]).join('/'),
body,
params:
this.paginateQueryService.paginateQueryToHttpParams(paginateQuery),
this.paginateQueryService.paginateQueryToHttpParamsObject(
paginateQuery,
),
});
}

Expand Down Expand Up @@ -223,11 +204,9 @@ export class RegistrationApiService extends DomainApiService {
return this.httpWrapperService.perform121ServiceRequest({
method: 'PATCH',
endpoint,
params: new HttpParams({
fromObject: {
pause: pauseStatus,
},
}),
params: {
pause: pauseStatus,
},
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { InfoTooltipComponent } from '~/components/info-tooltip/info-tooltip.component';
import { PageLayoutComponent } from '~/components/page-layout/page-layout.component';
import { SkeletonInlineComponent } from '~/components/skeleton-inline/skeleton-inline.component';
import { MetricApiService } from '~/domains/metric/metric.api.service';
import { PaymentApiService } from '~/domains/payment/payment.api.service';
import { ProjectApiService } from '~/domains/project/project.api.service';
import { MetricTileComponent } from '~/pages/project-monitoring/components/metric-tile/metric-tile.component';
Expand Down Expand Up @@ -52,13 +53,14 @@ export class ProjectMonitoringPageComponent {
projectId = input.required<number>();

readonly locale = inject(LOCALE_ID);
readonly metricApiService = inject(MetricApiService);
readonly projectApiService = inject(ProjectApiService);
readonly paymentApiService = inject(PaymentApiService);
readonly translatableStringService = inject(TranslatableStringService);

project = injectQuery(this.projectApiService.getProject(this.projectId));
metrics = injectQuery(() => ({
...this.projectApiService.getProjectSummaryMetrics(this.projectId)(),
...this.metricApiService.getProjectSummaryMetrics(this.projectId)(),
enabled: !!this.project.data()?.id,
}));
payments = injectQuery(() => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { SkeletonModule } from 'primeng/skeleton';

import { AppRoutes } from '~/app.routes';
import { SkeletonInlineComponent } from '~/components/skeleton-inline/skeleton-inline.component';
import { MetricApiService } from '~/domains/metric/metric.api.service';
import { PaymentApiService } from '~/domains/payment/payment.api.service';
import { ProjectApiService } from '~/domains/project/project.api.service';
import { ProjectMetricContainerComponent } from '~/pages/projects-overview/components/project-metric-container/project-metric-container.component';
Expand All @@ -37,14 +38,15 @@ import { TranslatableStringPipe } from '~/pipes/translatable-string.pipe';
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class ProjectSummaryCardComponent {
private metricApiService = inject(MetricApiService);
private projectApiService = inject(ProjectApiService);
private paymentApiService = inject(PaymentApiService);

public id = input.required<number>();

public project = injectQuery(this.projectApiService.getProject(this.id));
public metrics = injectQuery(() => ({
...this.projectApiService.getProjectSummaryMetrics(this.id)(),
...this.metricApiService.getProjectSummaryMetrics(this.id)(),
enabled: !!this.project.data()?.id,
}));
public payments = injectQuery(() => ({
Expand Down
10 changes: 3 additions & 7 deletions interfaces/Portalicious/src/app/services/http-wrapper.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
HttpErrorResponse,
HttpHeaders,
HttpParams,
HttpParamsOptions,
HttpStatusCode,
} from '@angular/common/http';
import { inject, Injectable } from '@angular/core';
Expand All @@ -22,12 +23,7 @@ interface PerformRequestParams {
body?: unknown;
responseAsBlob?: boolean;
isUpload?: boolean;
params?:
| HttpParams
| Record<
string,
boolean | number | readonly (boolean | number | string)[] | string
>;
params?: HttpParamsOptions['fromObject'];
}

export type Perform121ServiceRequestParams = { endpoint: string } & Omit<
Expand Down Expand Up @@ -158,7 +154,7 @@ export class HttpWrapperService {
headers: this.createHeaders(isUpload),
responseType: responseAsBlob ? 'blob' : undefined,
withCredentials: true,
params,
params: new HttpParams({ fromObject: params }),
body,
})
.pipe(
Expand Down
Loading

0 comments on commit b82d27a

Please sign in to comment.