#1964 - Added role to restrict MSFAA reissue #1985

andrepestana-aot · 2023-06-01T23:01:51Z

Added role to aest-business-administrators group on DEV and TEST;
Added role to enums;
Added decorator on the api call to restrict only to business administrator role;
Passed value to hide button when user has not business administrator role;
Updated User profile matrix spreadsheet;
Added the following tests:
Auth Ministry
√ Should have specific roles when ministry user is a business administrator. (400 ms)
ApplicationAESTController(e2e)-reissueMSFAA
√ Should not reissue an MSFAA when user is not a business administrator.

sources/packages/backend/apps/api/src/auth/roles.enum.ts

sources/packages/backend/apps/api/test/auth.aest.e2e-spec.ts

andrewsignori-aot

Nice start, please take a look at the comments.
I believe that we need to include in this ticket the API endpoint decorator and disable/remove the MSFAA button from the form when the user does not have the role.

…for the endpoint;

dheepak-aot · 2023-06-05T15:39:41Z

sources/packages/web/src/types/contracts/aest/roles.ts

@@ -30,4 +30,5 @@ export enum Role {
  InstitutionCreateNote = "institution-create-note",
  InstitutionApproveDeclineDesignation = "institution-approve-decline-designation",
  InstitutionApproveDeclineOfferingChanges = "institution-approve-decline-offering-changes",
+  StudentReissueMSFAA = "student-reissue-msfaa",


What about adding <check-permission-role> in the vue side. I know the Vue button is inside form.io. But is there a way to disable the button when the user does not have right permission?

Yes, I'm working on that. Thanks.

ann-aot · 2023-06-05T22:16:35Z

sources/packages/backend/apps/api/test/auth.aest.e2e-spec.ts

shouldn't the test/ dir be named as _tests_?

I added it to the same directory as auth.e2e-spec.ts and institution.e2e-spec.ts. What do you guys think @dheepak-aot @andrewsignori-aot @guru-aot @sh16011993 ?

In our application we do not follow the test package at root pattern like typically what OOTB code template creates.
I would suggest this e2e-spec to be in $same_directory_of_code_which_is_tested/tests/auth.aest.e2e-spec.ts

Following the application structure, I believe the directory should be renamed to _tests_ and the test should be moved to the directory of the code.

ok, I moved that test to the same directory of aest-token-helpers.ts.

ann-aot

Added a minor comment

...apps/api/src/route-controllers/application/_tests_/application.aest.reissueMSFAA.e2e-spec.ts

dheepak-aot · 2023-06-05T23:06:38Z

Looks good @andrepestana-aot . Just one comment on have auth.aest.e2e-spec.ts on appropriate folder.

Also, please add the role name student-reissue-msfaa to the ticket.

...apps/api/src/route-controllers/application/_tests_/application.aest.reissueMSFAA.e2e-spec.ts

...ackend/apps/api/src/testHelpers/auth/_tests_/e2e/aest-token-helpers.getAESTToken.e2e-spec.ts

andrewsignori-aot · 2023-06-06T00:28:55Z

sources/packages/web/src/views/aest/student/applicationDetails/NoticeOfAssessment.vue

@@ -12,7 +12,7 @@
    </template>
    <notice-of-assessment-form-view
      :assessment-id="assessmentId"
-      :can-reissue-m-s-f-a-a="true"
+      :can-reissue-m-s-f-a-a="hasStudentReissueMSFAARole"


I believe that usually, the vue is disabling the component instead of hiding it. For this case to keep it simple I would no mind having it removed.

...apps/api/src/route-controllers/application/_tests_/application.aest.reissueMSFAA.e2e-spec.ts

ann-aot

Good work @andrepestana-aot 👍

andrewsignori-aot · 2023-06-06T16:48:31Z

...ackend/apps/api/src/testHelpers/auth/_tests_/e2e/aest-token-helpers.getAESTToken.e2e-spec.ts

+
+const jwtService = new JwtService();
+
+describe("(e2e)-getAESTToken()", () => {


I would recommend using the "Auth" as a prefix, like Auth(e2e)-getAESTToken().

andrewsignori-aot · 2023-06-06T16:49:25Z

...ackend/apps/api/src/testHelpers/auth/_tests_/e2e/aest-token-helpers.getAESTToken.e2e-spec.ts

+
+describe("(e2e)-getAESTToken()", () => {
+  it("Should have all roles when ministry user is a business administrator.", async () => {
+    //Act


Can you please change it to "// Act"?

andrewsignori-aot · 2023-06-06T16:49:35Z

...ackend/apps/api/src/testHelpers/auth/_tests_/e2e/aest-token-helpers.getAESTToken.e2e-spec.ts

+    decodedToken.resource_access.aest.roles.sort((a, b) => a.localeCompare(b));
+    const allAESTRoles = Object.values(Role).sort((a, b) => a.localeCompare(b));
+
+    //Assert


Can you please change it to "// Assert"?

andrewsignori-aot

Thanks for doing the changes, look good 👍

sonarqubecloud · 2023-06-06T17:16:48Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
3.8% Duplication

github-actions · 2023-06-06T17:18:29Z

Backend Unit Tests Coverage Report

Totals
Statements:	17.92% ( 2092 / 11673 )
Methods:	8.23% ( 123 / 1494 )
Lines:	20.7% ( 1833 / 8854 )
Branches:	10.26% ( 136 / 1325 )

github-actions · 2023-06-06T17:19:36Z

E2E Workflow Workers Coverage Report

Totals
Statements:	49.81% ( 267 / 536 )
Methods:	41.56% ( 32 / 77 )
Lines:	55.33% ( 218 / 394 )
Branches:	26.15% ( 17 / 65 )

github-actions · 2023-06-06T17:19:43Z

E2E Queue Consumers Coverage Report

Totals
Statements:	69.4% ( 390 / 562 )
Methods:	59.15% ( 42 / 71 )
Lines:	71.52% ( 344 / 481 )
Branches:	40% ( 4 / 10 )

dheepak-aot

Thanks for the changes and adding role name to ticket. 👍

sh16011993

Thank You for making the changes. LGTM 👍

github-actions · 2023-06-06T17:25:45Z

E2E SIMS API Coverage Report

Totals
Statements:	43.54% ( 3042 / 6986 )
Methods:	37.65% ( 340 / 903 )
Lines:	48.62% ( 2535 / 5214 )
Branches:	19.22% ( 167 / 869 )

guru-aot

LGTM, nice work @andrepestana-aot

andrepestana-aot added 2 commits June 1, 2023 15:59

added role to enum and added a e2e test

9e7ab30

sonar bug

857bb46

andrepestana-aot self-assigned this Jun 2, 2023

andrepestana-aot requested review from sh16011993, dheepak-aot, andrewsignori-aot, guru-aot, ann-aot and JasonCTang June 2, 2023 00:37

andrepestana-aot marked this pull request as ready for review June 2, 2023 00:37

andrepestana-aot linked an issue Jun 2, 2023 that may be closed by this pull request

Add role to restrict MSFAA Reissue #1964

Closed

3 tasks

andrewsignori-aot reviewed Jun 2, 2023

View reviewed changes

sources/packages/backend/apps/api/src/auth/roles.enum.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 2, 2023

View reviewed changes

sources/packages/backend/apps/api/test/auth.aest.e2e-spec.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 2, 2023

View reviewed changes

typed user token

06ccd2b

andrepestana-aot marked this pull request as draft June 2, 2023 18:04

changed role name; added decorator to the API endpoint; created test …

e756baa

…for the endpoint;

dheepak-aot reviewed Jun 5, 2023

View reviewed changes

andrepestana-aot added 2 commits June 5, 2023 13:47

hid button when user does not have business administrator role;

80ab394

code smell

a06332c

andrepestana-aot marked this pull request as ready for review June 5, 2023 21:57

code smell

4fad687

ann-aot reviewed Jun 5, 2023

View reviewed changes

dheepak-aot reviewed Jun 5, 2023

View reviewed changes

...apps/api/src/route-controllers/application/_tests_/application.aest.reissueMSFAA.e2e-spec.ts Outdated Show resolved Hide resolved

andrewsignori-aot reviewed Jun 6, 2023

View reviewed changes

...apps/api/src/route-controllers/application/_tests_/application.aest.reissueMSFAA.e2e-spec.ts Outdated Show resolved Hide resolved

review issues

eee6bdf

andrewsignori-aot reviewed Jun 6, 2023

View reviewed changes

...ackend/apps/api/src/testHelpers/auth/_tests_/e2e/aest-token-helpers.getAESTToken.e2e-spec.ts Show resolved Hide resolved

review issues

9ba48c8

andrewsignori-aot reviewed Jun 6, 2023

View reviewed changes

...apps/api/src/route-controllers/application/_tests_/application.aest.reissueMSFAA.e2e-spec.ts Outdated Show resolved Hide resolved

review issues

988879c

ann-aot approved these changes Jun 6, 2023

View reviewed changes

andrewsignori-aot reviewed Jun 6, 2023

View reviewed changes

andrewsignori-aot approved these changes Jun 6, 2023

View reviewed changes

review issues

9eaed0b

dheepak-aot approved these changes Jun 6, 2023

View reviewed changes

sh16011993 approved these changes Jun 6, 2023

View reviewed changes

guru-aot approved these changes Jun 6, 2023

View reviewed changes

andrepestana-aot merged commit 144058f into main Jun 6, 2023

andrepestana-aot deleted the 1964_add_role_to_restrict_msfaa_reissue branch June 6, 2023 19:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1964 - Added role to restrict MSFAA reissue #1985

#1964 - Added role to restrict MSFAA reissue #1985

andrepestana-aot commented Jun 1, 2023 •

edited

Loading

andrewsignori-aot left a comment

dheepak-aot Jun 5, 2023

andrepestana-aot Jun 5, 2023

ann-aot Jun 5, 2023

andrepestana-aot Jun 5, 2023

dheepak-aot Jun 5, 2023

sh16011993 Jun 6, 2023

andrepestana-aot Jun 6, 2023

ann-aot left a comment

dheepak-aot commented Jun 5, 2023

andrewsignori-aot Jun 6, 2023

ann-aot left a comment

andrewsignori-aot Jun 6, 2023

andrewsignori-aot Jun 6, 2023

andrewsignori-aot Jun 6, 2023

andrewsignori-aot left a comment

sonarqubecloud bot commented Jun 6, 2023

github-actions bot commented Jun 6, 2023

github-actions bot commented Jun 6, 2023

github-actions bot commented Jun 6, 2023

dheepak-aot left a comment

sh16011993 left a comment

github-actions bot commented Jun 6, 2023

guru-aot left a comment


		const jwtService = new JwtService();

		describe("(e2e)-getAESTToken()", () => {

#1964 - Added role to restrict MSFAA reissue #1985

#1964 - Added role to restrict MSFAA reissue #1985

Conversation

andrepestana-aot commented Jun 1, 2023 • edited Loading

andrewsignori-aot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ann-aot left a comment

Choose a reason for hiding this comment

dheepak-aot commented Jun 5, 2023

Choose a reason for hiding this comment

ann-aot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsignori-aot left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 6, 2023

github-actions bot commented Jun 6, 2023

Backend Unit Tests Coverage Report

github-actions bot commented Jun 6, 2023

E2E Workflow Workers Coverage Report

github-actions bot commented Jun 6, 2023

E2E Queue Consumers Coverage Report

dheepak-aot left a comment

Choose a reason for hiding this comment

sh16011993 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 6, 2023

E2E SIMS API Coverage Report

guru-aot left a comment

Choose a reason for hiding this comment

andrepestana-aot commented Jun 1, 2023 •

edited

Loading