Skip to content

Commit

Permalink
Merge pull request #1753 from fecgov/feature/1722
Browse files Browse the repository at this point in the history
Feature/1722 - Add a preparePayload function to these three services to strip out unnecessary data from being sent to the api.
  • Loading branch information
toddlees authored Mar 8, 2024
2 parents f174ca0 + 9225e5c commit c3aec93
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 68 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Component, ElementRef } from '@angular/core';
import { TableListBaseComponent } from 'app/shared/components/table-list-base/table-list-base.component';
import { ConfirmationService, MessageService } from 'primeng/api';

import { ListRestResponse } from 'app/shared/models/rest-api.model';
import { LabelList, LabelUtils, PrimeOptions } from 'app/shared/utils/label.utils';
import { Contact, ContactTypeLabels, ContactTypes } from '../../shared/models/contact.model';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export interface TableListService<T> {
getTableData(
pageNumber: number,
ordering?: string,
params?: { [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean> }
params?: { [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean> },
): Observable<ListRestResponse>;

delete(item: T): Observable<null>;
Expand Down
4 changes: 2 additions & 2 deletions front-end/src/app/shared/models/contact.model.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { plainToClass } from 'class-transformer';
import { plainToClass, plainToInstance } from 'class-transformer';
import { SelectItem, SelectItemGroup } from 'primeng/api';
import { LabelList } from '../utils/label.utils';
import { BaseModel } from './base.model';
Expand Down Expand Up @@ -95,7 +95,7 @@ export class Contact extends BaseModel {

// prettier-ignore
static fromJSON(json: any): Contact { // eslint-disable-line @typescript-eslint/no-explicit-any
return plainToClass(Contact, json);
return plainToInstance(Contact, json);
}

getNameString(): string {
Expand Down
77 changes: 47 additions & 30 deletions front-end/src/app/shared/services/api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,23 @@ import { Observable } from 'rxjs';
import { environment } from '../../../environments/environment';
import { ALLOW_ERROR_CODES } from '../interceptors/http-error.interceptor';

export interface QueryParams {
[param: string]:
| string
| number
| boolean
| ReadonlyArray<string | number | boolean>
| readonly (string | number | boolean)[];
}

@Injectable({
providedIn: 'root',
})
export class ApiService {
constructor(private http: HttpClient, private cookieService: CookieService) { }
constructor(
private http: HttpClient,
private cookieService: CookieService,
) {}

getHeaders(headersToAdd: object = {}) {
const csrfToken = `${this.cookieService.get('csrftoken')}`;
Expand All @@ -20,31 +32,22 @@ export class ApiService {
return { ...baseHeaders, ...headersToAdd };
}

getQueryParams(
queryParams: { [param: string]: string | number | boolean | readonly (string | number | boolean)[] } = {}
) {
getQueryParams(queryParams: QueryParams = {}) {
return new HttpParams({ fromObject: queryParams });
}

public get<T>(endpoint: string, params?: QueryParams): Observable<T>;
public get<T>(endpoint: string, params?: QueryParams, allowedErrorCodes?: number[]): Observable<HttpResponse<T>>;
public get<T>(
endpoint: string,
params?: { [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean> }
): Observable<T>;
public get<T>(
endpoint: string,
params?: { [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean> },
allowedErrorCodes?: number[]
): Observable<HttpResponse<T>>;
public get<T>(
endpoint: string,
params: { [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean> } = {},
allowedErrorCodes?: number[]
params: QueryParams = {},
allowedErrorCodes?: number[],
): Observable<T> | Observable<HttpResponse<T>> {
const headers = this.getHeaders();
if (allowedErrorCodes) {
return this.http.get<T>(`${environment.apiUrl}${endpoint}`, {
headers: headers,
params: params,
headers,
params,
withCredentials: true,
observe: 'response',
responseType: 'json',
Expand All @@ -58,42 +61,56 @@ export class ApiService {
});
}

/* eslint-disable @typescript-eslint/no-explicit-any */
public post<T>(endpoint: string, payload: any, queryParams?: any): Observable<T>;
public post<T>(endpoint: string, payload: any, queryParams?: any, allowedErrorCodes?: number[]): Observable<HttpResponse<T>>;
public post<T>(endpoint: string, payload: any, queryParams: any = {}, allowedErrorCodes?: number[]): Observable<T> | Observable<HttpResponse<T>> {
public post<T>(endpoint: string, payload: unknown, queryParams?: QueryParams): Observable<T>;
public post<T>(
endpoint: string,
payload: unknown,
queryParams?: QueryParams,
allowedErrorCodes?: number[],
): Observable<HttpResponse<T>>;
public post<T>(
endpoint: string,
payload: unknown,
queryParams: QueryParams = {},
allowedErrorCodes?: number[],
): Observable<T> | Observable<HttpResponse<T>> {
const headers = this.getHeaders();
const params = this.getQueryParams(queryParams);
if (allowedErrorCodes) {
return this.http.post<T>(
`${environment.apiUrl}${endpoint}`, payload, {
return this.http.post<T>(`${environment.apiUrl}${endpoint}`, payload, {
headers: headers,
params: params,
withCredentials: true,
observe: 'response',
context: new HttpContext().set(ALLOW_ERROR_CODES, allowedErrorCodes),
});
}
return this.http.post<T>(`${environment.apiUrl}${endpoint}`, payload, { headers: headers, params: params, withCredentials: true });
return this.http.post<T>(`${environment.apiUrl}${endpoint}`, payload, {
headers: headers,
params: params,
withCredentials: true,
});
}
/* eslint-enable @typescript-eslint/no-explicit-any */

// prettier-ignore
public postAbsoluteUrl<T>(endpoint: string, payload: any, queryParams: any = {}): Observable<T> { // eslint-disable-line @typescript-eslint/no-explicit-any
public postAbsoluteUrl<T>(endpoint: string, payload: unknown, queryParams: QueryParams = {}): Observable<T> {
const headers = this.getHeaders();
const params = this.getQueryParams(queryParams);
return this.http.post<T>(`${endpoint}`, payload, { headers: headers, params: params, withCredentials: true });
}

// prettier-ignore
public put<T>(endpoint: string, payload: any, queryParams: any = {}): Observable<T> { // eslint-disable-line @typescript-eslint/no-explicit-any
public put<T>(endpoint: string, payload: unknown, queryParams: QueryParams = {}): Observable<T> {
const headers = this.getHeaders();
const params = this.getQueryParams(queryParams);
return this.http.put<T>(`${environment.apiUrl}${endpoint}`, payload, { headers: headers, params: params, withCredentials: true });
return this.http.put<T>(`${environment.apiUrl}${endpoint}`, payload, {
headers,
params,
withCredentials: true,
});
}

public delete<T>(endpoint: string): Observable<T> {
const headers = this.getHeaders();
return this.http.delete<T>(`${environment.apiUrl}${endpoint}`, { headers: headers, withCredentials: true });
return this.http.delete<T>(`${environment.apiUrl}${endpoint}`, { headers, withCredentials: true });
}
}
12 changes: 6 additions & 6 deletions front-end/src/app/shared/services/contact.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ describe('ContactService', () => {
httpTestingController.verify();
});

it('#checkFecIdForUniqness should return true if contact matches', () => {
it('#checkFecIdForUniqueness should return true if contact matches', () => {
const fecId = 'fecId';
const contactId = 'contactId';
spyOn(testApiService, 'get')
Expand All @@ -265,31 +265,31 @@ describe('ContactService', () => {
})
.and.returnValue(of('contactId' as any)); // eslint-disable-line @typescript-eslint/no-explicit-any

service.checkFecIdForUniqness(fecId, contactId).subscribe((isUnique) => {
service.checkFecIdForUniqueness(fecId, contactId).subscribe((isUnique) => {
expect(isUnique).toBeTrue();
});
});
it('#checkFecIdForUniqness should return false if server comes back with differnt contact id', () => {
it('#checkFecIdForUniqueness should return false if server comes back with differnt contact id', () => {
const fecId = 'fecId';
const contactId = 'contactId';
spyOn(testApiService, 'get')
.withArgs('/contacts/get_contact_id/', {
fec_id: fecId,
})
.and.returnValue(of('different id' as any)); // eslint-disable-line @typescript-eslint/no-explicit-any
service.checkFecIdForUniqness(fecId, contactId).subscribe((isUnique) => {
service.checkFecIdForUniqueness(fecId, contactId).subscribe((isUnique) => {
expect(isUnique).toBeFalse();
});
});
it('#checkFecIdForUniqness should return true if server comes back no id', () => {
it('#checkFecIdForUniqueness should return true if server comes back no id', () => {
const fecId = 'fecId';
const contactId = 'contactId';
spyOn(testApiService, 'get')
.withArgs('/contacts/get_contact_id/', {
fec_id: fecId,
})
.and.returnValue(of('' as any)); // eslint-disable-line @typescript-eslint/no-explicit-any
service.checkFecIdForUniqness(fecId, contactId).subscribe((isUnique) => {
service.checkFecIdForUniqueness(fecId, contactId).subscribe((isUnique) => {
expect(isUnique).toBeTrue();
});
});
Expand Down
49 changes: 26 additions & 23 deletions front-end/src/app/shared/services/contact.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,25 @@ import { ApiService } from './api.service';
export class ContactService implements TableListService<Contact> {
constructor(private apiService: ApiService) {}

/**
* Given the type of contact given, return the appropriate JSON schema doc
* @param {ContactTypes} type
* @returns {JsonSchema} schema
*/
public static getSchemaByType(type: ContactTypes): JsonSchema {
let schema: JsonSchema = contactIndividualSchema;
if (type === ContactTypes.CANDIDATE) {
schema = contactCandidateSchema;
}
if (type === ContactTypes.COMMITTEE) {
schema = contactCommitteeSchema;
}
if (type === ContactTypes.ORGANIZATION) {
schema = contactOrganizationSchema;
}
return schema;
}

public getTableData(pageNumber = 1, ordering = ''): Observable<ListRestResponse> {
if (!ordering) {
ordering = 'name';
Expand All @@ -43,14 +62,13 @@ export class ContactService implements TableListService<Contact> {
}

public create(contact: Contact): Observable<Contact> {
const payload = contact.toJson();
const payload = this.preparePayload(contact);
return this.apiService.post<Contact>(`/contacts/`, payload).pipe(map((response) => Contact.fromJSON(response)));
}

public update(contact: Contact): Observable<Contact> {
const payload = contact.toJson();
public update(updated: Contact): Observable<Contact> {
return this.apiService
.put<Contact>(`/contacts/${contact.id}/`, payload)
.put<Contact>(`/contacts/${updated.id}/`, updated)
.pipe(map((response) => Contact.fromJSON(response)));
}

Expand Down Expand Up @@ -96,7 +114,7 @@ export class ContactService implements TableListService<Contact> {
.pipe(map((response) => CommitteeLookupResponse.fromJSON(response)));
}

public checkFecIdForUniqness(fecId: string, contactId?: string): Observable<boolean> {
public checkFecIdForUniqueness(fecId: string, contactId?: string): Observable<boolean> {
if (fecId) {
return this.apiService
.get<string>(`/contacts/get_contact_id/`, { fec_id: fecId })
Expand All @@ -109,7 +127,7 @@ export class ContactService implements TableListService<Contact> {
return (control: AbstractControl) => {
return of(control.value).pipe(
switchMap((fecId) =>
this.checkFecIdForUniqness(fecId, contactId).pipe(
this.checkFecIdForUniqueness(fecId, contactId).pipe(
map((isUnique: boolean) => {
return isUnique ? null : { fecIdMustBeUnique: true };
}),
Expand Down Expand Up @@ -147,23 +165,8 @@ export class ContactService implements TableListService<Contact> {
.pipe(map((response) => OrganizationLookupResponse.fromJSON(response)));
}

/**
* Given the type of contact given, return the appropriate JSON schema doc
* @param {ContactTypes} type
* @returns {JsonSchema} schema
*/
public static getSchemaByType(type: ContactTypes): JsonSchema {
let schema: JsonSchema = contactIndividualSchema;
if (type === ContactTypes.CANDIDATE) {
schema = contactCandidateSchema;
}
if (type === ContactTypes.COMMITTEE) {
schema = contactCommitteeSchema;
}
if (type === ContactTypes.ORGANIZATION) {
schema = contactOrganizationSchema;
}
return schema;
private preparePayload(contact: Contact): Record<string, unknown> {
return contact.toJson();
}
}

Expand Down
10 changes: 8 additions & 2 deletions front-end/src/app/shared/services/report.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ export class ReportService implements TableListService<Report> {
}

public create(report: Report, fieldsToValidate: string[] = []): Observable<Report> {
const payload = report.toJson();
const payload = this.preparePayload(report);
return this.apiService
.post<Report>(`${this.apiEndpoint}/`, payload, { fields_to_validate: fieldsToValidate.join(',') })
.pipe(map((response) => getReportFromJSON(response)));
}

public update(report: Report, fieldsToValidate: string[] = []): Observable<Report> {
const payload = report.toJson();
const payload = this.preparePayload(report);
return this.apiService
.put<Report>(`${this.apiEndpoint}/${report.id}/`, payload, { fields_to_validate: fieldsToValidate.join(',') })
.pipe(map((response) => getReportFromJSON(response)));
Expand Down Expand Up @@ -101,4 +101,10 @@ export class ReportService implements TableListService<Report> {
public startAmendment(report: Report): Observable<string> {
return this.apiService.post(`${this.apiEndpoint}/${report.id}/amend/`, {});
}

preparePayload(item: Report): Record<string, unknown> {
const payload = item.toJson();
delete payload['schema'];
return payload;
}
}
3 changes: 1 addition & 2 deletions front-end/src/app/shared/services/transaction.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ export class TransactionService implements TableListService<Transaction> {
* @param transaction
*/
private preparePayload(transaction: Transaction) {
if (typeof transaction === 'string') return transaction;
const payload = transaction.toJson();

// Add flags to the payload used for API processing
Expand All @@ -208,7 +207,7 @@ export class TransactionService implements TableListService<Transaction> {
delete payload['transactionType'];
delete payload['report'];

if (transaction.children) {
if (payload['children']) {
payload['children'] = transaction.children.map((transaction) => this.preparePayload(transaction));
}

Expand Down
2 changes: 1 addition & 1 deletion front-end/src/app/shared/utils/schema.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class SchemaUtils {
/**
* Convert the form input value to the appropriate type.
* @param {string} property
* @param {FromGroup} form
* @param {FormGroup} form
* @param {JsonSchema} jsonSchema - the schema to use in the form element custom validator.
* @returns
*/
Expand Down

0 comments on commit c3aec93

Please sign in to comment.