Skip to content
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.

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

#3260 - CAS Integration 3A - Create new Supplier and Site - Retry on Error and Scheduler Time #3830

Merged
merged 8 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { MigrationInterface, QueryRunner } from "typeorm";
import { getSQLFileData } from "../utilities/sqlLoader";

export class UpdateCASSupplierIntegrationScheduler1729785100207
implements MigrationInterface
{
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
getSQLFileData("Update-cas-supplier-integration.sql", "Queue"),
);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
getSQLFileData("Rollback-update-cas-supplier-integration.sql", "Queue"),
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
UPDATE
sims.queue_configurations
SET
queue_configuration = '{
"cron": "0 7 * * *",
"retry": 3,
"cleanUpPeriod": 2592000000,
"retryInterval": 180000,
"dashboardReadonly": false
}' :: json
WHERE
queue_name = 'cas-supplier-integration';
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
UPDATE
sims.queue_configurations
SET
queue_configuration = '{
"cron": "0 20 * * 1-5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Please update the PR description as well.

"retry": 3,
"cleanUpPeriod": 2592000000,
"retryInterval": 180000,
"dashboardReadonly": false
}' :: json
WHERE
queue_name = 'cas-supplier-integration';
Original file line number Diff line number Diff line change
Expand Up @@ -365,4 +365,68 @@ describe(describeProcessorRootTest(QueueNames.CASSupplierIntegration), () => {
);
},
);

it("Should throw an error for the first student and process the second one when the CAS API call failed for the first student but worked for the second one.", async () => {
// Arrange
// Create two mocks where the first one will throw an error
// and the second one is expected to work.
casServiceMock.getSupplierInfoFromCAS = jest
.fn()
.mockRejectedValueOnce("Unknown error")
.mockResolvedValue(
Promise.resolve(createFakeCASNotFoundSupplierResponse()),
);
// Mock to be also used for processing the second student.
casServiceMock.createSupplierAndSite = jest.fn(() =>
Promise.resolve(createFakeCASCreateSupplierAndSiteResponse()),
);
// Student CAS pending request expected to fail.
const savedCASSupplierToFail = await saveFakeCASSupplier(db);
// Student CAS pending request expected to succeed.
const studentToSucceed = await saveFakeStudent(db.dataSource, undefined, {
initialValue: {
contactInfo: {
address: {
addressLine1: "3350 Douglas St",
city: "Victoria",
country: "Canada",
selectedCountry: COUNTRY_CANADA,
provinceState: "BC",
postalCode: "V8Z 7X9",
},
} as ContactInfo,
},
});
const savedCASSupplierToSucceed = await saveFakeCASSupplier(db, {
student: studentToSucceed,
});

// Queued job.
const mockedJob = mockBullJob<void>();

// Act
await expect(
processor.processCASSupplierInformation(mockedJob.job),
).rejects.toStrictEqual(
new Error(
"One or more errors were reported during the process, please see logs for details.",
),
);

// Assert
expect(
mockedJob.containLogMessages([
"Executing CAS supplier integration...",
"Found 2 records to be updated.",
`Processing student CAS supplier ID: ${savedCASSupplierToFail.id}.`,
'Unexpected error while processing supplier. "Unknown error"',
`Processing student CAS supplier ID: ${savedCASSupplierToSucceed.id}.`,
`CAS evaluation result status: ${CASEvaluationStatus.NotFound}.`,
`No active CAS supplier found. Reason: ${NotFoundReason.SupplierNotFound}.`,
"Created supplier and site on CAS.",
"Updated CAS supplier and site for the student.",
"CAS supplier integration executed.",
]),
).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
logProcessSummaryToJobLogger,
getSuccessMessageWithAttentionCheck,
} from "../../../utilities";
import { CAS_AUTH_ERROR } from "@sims/integrations/constants";

@Processor(QueueNames.CASSupplierIntegration)
export class CASSupplierIntegrationScheduler extends BaseScheduler<void> {
Expand Down Expand Up @@ -63,19 +62,16 @@ export class CASSupplierIntegrationScheduler extends BaseScheduler<void> {
`Records updated: ${suppliersUpdated}.`,
],
processSummary,
{ throwOnError: true },
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

);
} catch (error: unknown) {
// Throw an error to force the queue to retry.
if (error instanceof CustomNamedError) {
if (error.name === CAS_AUTH_ERROR) {
// Throw an error to force the queue to retry.
throw new Error(
`Unable to request data to CAS due to an authentication error.`,
);
}
const errorMessage = "Unexpected error while executing the job.";
processSummary.error(errorMessage, error);
return [errorMessage];
throw new Error(error.message);
}
throw new Error("Unexpected error while executing the job.", {
cause: error,
});
} finally {
this.logger.logProcessSummary(processSummary);
await logProcessSummaryToJobLogger(processSummary, job);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CustomNamedError } from "@sims/utilities";
import { ProcessSummary } from "@sims/utilities/logger";
import * as dayjs from "dayjs";

Expand All @@ -7,6 +8,8 @@ import * as dayjs from "dayjs";
*/
const LOG_INDENTATION = "--";

export const PROCESS_SUMMARY_CONTAINS_ERROR = "PROCESS_SUMMARY_CONTAINS_ERROR";

/**
* Log capability provided by a queue job.
*/
Expand Down Expand Up @@ -41,13 +44,18 @@ export async function logProcessSummaryToJobLogger(
* @param successMessages generic success message.
* @param processSummary processSummary to check if an
* attention message is required.
* @param options options.
* - `throwOnError`: when true, throws an exception if the
* error summary contains some error.
* @returns generic success message or success message with
* attention messages appended.
*/
export function getSuccessMessageWithAttentionCheck(
successMessages: string[],
processSummary: ProcessSummary,
options?: { throwOnError?: boolean },
): string[] {
const throwOnError = options?.throwOnError ?? false;
const message: string[] = [];
message.push(...successMessages);
const logsSum = processSummary.getLogLevelSum();
Expand All @@ -58,6 +66,12 @@ export function getSuccessMessageWithAttentionCheck(
message.push(
`Error(s): ${logsSum.error}, Warning(s): ${logsSum.warn}, Info: ${logsSum.info}`,
);
if (logsSum.error && throwOnError) {
throw new CustomNamedError(
"One or more errors were reported during the process, please see logs for details.",
PROCESS_SUMMARY_CONTAINS_ERROR,
);
}
}
return message;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { convertToASCII } from "@sims/utilities";
import { convertToASCIIString } from "@sims/utilities";

const CAS_SUPPLIER_NAME_MAX_LENGTH = 80;
const CAS_ADDRESS_MAX_LENGTH = 35;
Expand All @@ -17,7 +17,7 @@ export function formatUserName(firstName: string, lastName: string): string {
0,
CAS_SUPPLIER_NAME_MAX_LENGTH,
);
return convertToASCII(formattedName).toUpperCase();
return convertToASCIIString(formattedName).toUpperCase();
}

/**
Expand All @@ -27,7 +27,7 @@ export function formatUserName(firstName: string, lastName: string): string {
* @returns formatted address.
*/
export function formatAddress(address: string): string {
return convertToASCII(
return convertToASCIIString(
address.substring(0, CAS_ADDRESS_MAX_LENGTH),
).toUpperCase();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CASIntegrationConfig, ConfigService } from "@sims/utilities/config";
import { stringify } from "querystring";
import {
CustomNamedError,
convertToASCII,
convertToASCIIString,
parseJSONError,
} from "@sims/utilities";
import { CAS_AUTH_ERROR } from "@sims/integrations/constants";
Expand Down Expand Up @@ -98,7 +98,7 @@ export class CASService {
sin: string,
lastName: string,
): Promise<CASSupplierResponse> {
const convertedLastName = convertToASCII(lastName).toUpperCase();
const convertedLastName = convertToASCIIString(lastName).toUpperCase();
const url = `${this.casIntegrationConfig.baseUrl}/cfs/supplier/${convertedLastName}/lastname/${sin}/sin`;
let response: { data: CASSupplierResponse };
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { convertToASCII } from "@sims/utilities/string-utils";
import { convertToASCIIString } from "@sims/utilities/string-utils";

describe("StringUtils-convertToASCII", () => {
it("Should replace the special characters when equivalent ones are present.", () => {
Expand All @@ -7,7 +7,7 @@ describe("StringUtils-convertToASCII", () => {
"Some text with special characters: ÀÁÂÃÄÅÇÈÉÊËÌÍÎÏÑÒÓÔÕÖÙÚÛÜ—àáâãäåçèéêëìíîï-ñóòôõöùúûüýÿ";

// Act
const translatedData = convertToASCII(textWithSpecialCharacters);
const translatedData = convertToASCIIString(textWithSpecialCharacters);

// Assert
expect(translatedData).toBe(
Expand All @@ -20,7 +20,7 @@ describe("StringUtils-convertToASCII", () => {
const textWithSpecialCharacters = null;

// Act
const translatedData = convertToASCII(textWithSpecialCharacters);
const translatedData = convertToASCIIString(textWithSpecialCharacters);

// Assert
expect(translatedData).toBeNull();
Expand All @@ -31,7 +31,7 @@ describe("StringUtils-convertToASCII", () => {
const textWithSpecialCharacters = undefined;

// Act
const translatedData = convertToASCII(textWithSpecialCharacters);
const translatedData = convertToASCIIString(textWithSpecialCharacters);

// Assert
expect(translatedData).toBeNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { AddressInfo } from "@sims/sims-db";
export const OTHER_COUNTRY = "other";
// 'selectedCountry' in the student contact info will have the value 'canada',
// when 'Canada' is selected.
export const COUNTRY_CANADA = "canada";
export const COUNTRY_CANADA = "Canada";

/**
* Inspects the address info to determine if the address is from Canada.
Expand Down
18 changes: 15 additions & 3 deletions sources/packages/backend/libs/utilities/src/string-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ const ASCII_INDIRECT_CONVERSIONS: Record<string, string> = {
* Converts input into an ASCII 7 bit buffer replacing special characters
* by equivalent ASCII characters when possible.
* @param rawContent raw content in string format.
* @returns a string of the input with extended ASCII characters (ISO-8859-1)
* @returns a buffer of the input with extended ASCII characters (ISO-8859-1)
* converted to equivalent ASCII characters. If null or undefined is provided,
* null will be returned.
*/
export function convertToASCII(rawContent?: string): string | null {
export function convertToASCII(rawContent?: string): Buffer | null {
if (rawContent === null || rawContent === undefined) {
return null;
}
Expand Down Expand Up @@ -140,5 +140,17 @@ export function convertToASCII(rawContent?: string): string | null {
}
}
}
return content.toString();
return content;
}

/**
* Converts input into an ASCII 7 bit buffer replacing special characters
* by equivalent ASCII characters when possible.
* @param rawContent raw content in string format.
* @returns a string of the input with extended ASCII characters (ISO-8859-1)
* converted to equivalent ASCII characters. If null or undefined is provided,
* null will be returned.
*/
export function convertToASCIIString(rawContent?: string): string | null {
return convertToASCII(rawContent)?.toString() ?? null;
Comment on lines +154 to +155
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for raising the issue 😉

}