-
Notifications
You must be signed in to change notification settings - Fork 14
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
#2107 - E2E test for offering bulk upload #2119
Conversation
// Arrange | ||
// Creating an institution location with same location code as that of the | ||
// first row of the multiple CSV file. | ||
const collegeFLocationYESK = createFakeInstitutionLocation( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API test suits run in parallel, hence there is no guarantee that the location with institution code YESK (in this case) could not be created before.
I had the same situation while creating e2e tests for ece response integration. I followed this approach and it worked.
We could possibility centralize and re-use the same method. Let me know if I am missing something.
|
||
// Act/Assert | ||
await request(app.getHttpServer()) | ||
.post(`${endpoint}?validation-only=true`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
it("Should return program related validation error when bulk offering CSV file with a non existing program SABC code is uploaded. ", async () => { | ||
// Arrange | ||
const randomSABCCode = `XXXX1`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Template literals not required.
* - `user` related user. | ||
* @param options dependencies. | ||
* - `initialValues` initial values. | ||
* @returns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment for returns.
Great Job. Thanks for doing the refactor of |
@@ -32,9 +46,9 @@ export function createFakeEducationProgram( | |||
program.hasTravel = "yes"; | |||
program.hasIntlExchange = "yes"; | |||
program.programDeclaration = true; | |||
program.institution = institution ?? createFakeInstitution(); | |||
program.institution = relations?.institution ?? createFakeInstitution(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @ann-aot
* @returns institution location | ||
*/ | ||
export function createFakeInstitutionLocation( | ||
institution?: Institution, | ||
relations?: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
export function createFakeEducationProgram( | ||
institution?: Institution, | ||
user?: User, | ||
relations?: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
"bulk-insert/multiple-upload.csv", | ||
); | ||
|
||
let [responseOfferingSBC1, responseOfferingSBC2] = [undefined, undefined]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggesttion: variables are implicitly undefined when not initialized.
We can use like this.
let responseOfferingSBC1: EducationProgramOffering,
responseOfferingSBC2: EducationProgramOffering;
offeringStatus: true, | ||
}, | ||
where: { | ||
id: In([responseOfferingSBC1.id, responseOfferingSBC2.id]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we need to use the order by id to guarantee the result order?
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing the changes 👍
createFakeEducationProgram
andcreateFakeInstitutionLocation