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.

Sign up for GitHub

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

#1791 - Enable Assessments tab view and NOA view for Public Institution Users - Part 4 #1976

Merged
merged 10 commits into from
Jun 2, 2023
+152 −34
Merged

#1791 - Enable Assessments tab view and NOA view for Public Institution Users - Part 4 #1976

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b4eb785
initail commit
ann-aot May 30, 2023
04bfebf
refactor
ann-aot May 31, 2023
b66e415
refactor
ann-aot May 31, 2023
55c6b38
Merge branch 'main' into feature/sims-#1791-part-4
ann-aot May 31, 2023
22446b8
feedback
ann-aot Jun 1, 2023
6c3484b
feedback
ann-aot Jun 1, 2023
6c01650
feedback
ann-aot Jun 1, 2023
67f4238
feedback
ann-aot Jun 1, 2023
deb97a1
feedback
ann-aot Jun 1, 2023
83c2111
fedback
ann-aot Jun 2, 2023
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
Prev Previous commit
Next Next commit
feedback
@ann-aot
ann-aot committed Jun 1, 2023
commit 67f423877065e26b51b510fc8e43ae7c5b109a90
2 changes: 1 addition & 1 deletion .../_tests_/e2e/assessment.institutions.controller.getRequestedAssessmentSummary.e2e-spec.ts
View file
Original file line number Diff line number Diff line change
@@ -157,7 +157,7 @@ describe("AssessmentInstitutionsController(e2e)-getRequestedAssessmentSummary",
.expect(HttpStatus.OK)
.expect([]);

// Checking the same scenario with the ministry user, to make sure that a fake offering change was created.
// Checking the same scenario with the ministry endpoint with a ministry user, to make sure that a fake offering change was created.
// Arrange.
const ministryEndpoint = `/aest/assessment/application/${application.id}/requests`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Testing a ministry endpoint at assessment.institutions.controller.*spec is against the pattern that we have been following in e2e.

I am not able to follow the reason for doing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As @andrewsignori-aot mentioned #1976 (comment) and the comment stated in the code, it's not a aest endpoint check, it is just to validate the scenario, whether the offering change was actually created and that's is the reason, the institution endpoint is returning empty list (result without an offering change)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not able to follow @ann-aot . To validate a scenario we are are calling an endpoint here which I feel is an E2E test for an endpoint even though the intention is to validate a scenario.

const ministryToken = await getAESTToken(