2160 internal user can view access requests #2469

Sepehr-Sobhani · 2024-11-14T01:20:25Z

ISSUE 2160

NOTE:

Updated the dashboard tiles and removed unnecessary ones.
Added user operator tables to allow internal users to view access requests.
Improved the data in the grid by updating the fixtures.
Additionally, modified the Django admin tables for user and user-operator data models to enhance their usability.(non-related)

TEST:
Log in as an internal user. Click the Operator Administrators and Access Requests tile to view the data grid.

BCerki

Nice work!

BCerki · 2024-11-20T23:20:26Z

bc_obps/registration/tests/endpoints/v2/test_user_operators.py

+        assert response.status_code == 200
+        response_items_1 = response.json().get('items')
+        assert response_items_1[0].get('user__first_name') == "Jane"
+        # Test with a type filter that doesn't exist


Leftover comment? This test looks like it's checking the name filter

BCerki · 2024-11-20T23:22:22Z

bc_obps/registration/tests/endpoints/v2/test_user_operators.py

+        assert response.status_code == 200
+        response_data = response.json()
+        assert len(response_data) == 2
+        # assert one of the approved admin user operators is in the response


Assert "one is in the response" as in we're checking one of the two as a sample? Or we actually only expect one?

I only wanted to test one of them as a sample.

BCerki · 2024-11-20T23:24:30Z

bc_obps/registration/tests/endpoints/v2/test_user_operators.py

+            == approved_admin_user_operator_to_check.user.bceid_business_name
+        )
+        assert (
+            approved_admin_user_operator_in_response["operator__legal_name"]


If we check them one by one like this, it's possible there could be an extra value in there that we don't want and this test isn't catching. We could check that the whole object == expected object, or that the keys match what we expect

Added a test to make sure we get the expected keys.

BCerki · 2024-11-20T23:37:06Z

bc_obps/service/data_access_service/user_operator_service.py

+            operator__status=Operator.Statuses.DECLINED
+        )
+        # Subquery to check if an approved admin user exists for the operator
+        approved_admin_operator_exists = UserOperator.objects.filter(


this is cool, nice and readable

BCerki · 2024-11-20T23:38:00Z

bc_obps/service/error_service/handle_exception.py

@@ -14,6 +15,11 @@ def handle_exception(error: Exception) -> Tuple[Literal[400, 401, 403, 404, 422]
    """
    This function handles exceptions for BCEIRS. Returns a 4xx status.
    """
+    if DEBUG == "True":
+        # Print the error in the console for easier debugging
+        print("---------------------------------------------ERROR START-----------------------------------------------")


Thanks for doing this!

BCerki · 2024-11-20T23:52:35Z

bc_obps/service/tests/data_access_service/test_data_access_user_operator_service.py

+                )
+            )
+
+        # Add approved admin user for the approved operator (to prevent showing pending user operators for this operator)


You're probably more in the loop about this than me, but I thought the plan for reg2 was to show all the user operators in the table regardless of if there's already an admin or who approved them.

I discussed this with Andrea and Zoey, but I forgot to document it in this ticket. Sorry for that. Here’s the conversation: #2059 (comment)

Gotcha, thanks. Internal users don't get to approve/deny after the first admin, but are they supposed to get to view everyone?

I believe they prefer to have a minimal number of requests in this table. From what I understand, seeing a record in the table without being able to change it might be annoying for them. Calling @andrea-williams to take radical decisions lol

I'm overruling a comment that one SME said (that I believe was just casual and not something they were committing to). IMO it doesn't make sense to show ALL industry admins in that grid when internal users are only expected to action some of them. Internal users can view all users for a specific operator and view the admins from there if they need to.

Boom. Radical Decision made.

BCerki · 2024-11-21T19:29:15Z

bc_obps/service/tests/test_user_operator_service_v2.py

+        # make sure only irc user can access this
+        industry_user = baker.make_recipe('utils.industry_operator_user')
+        with pytest.raises(Exception, match=UNAUTHORIZED_MESSAGE):
+            UserOperatorServiceV2.list_user_operators_v2(


This is a long test, could move some parts (e.g. testing for exceptions) into their own tests

BCerki · 2024-11-21T19:29:59Z

bc_obps/service/tests/test_user_operator_service_v2.py

+        assert user_operators_with_admin_access_status.filter(status=UserOperator.Statuses.APPROVED).count() == 5
+
+        # Check sorting
+        filters_3 = filters_2.model_copy(


BCerki · 2024-11-21T19:38:24Z

bciers/apps/administration/tests/components/userOperators/UserOperatorsPage.test.tsx

+
+  it("renders UserOperatorsPage with the note on top of the page", async () => {
+    getUserOperatorsPageData.mockReturnValueOnce({
+      data: [],


Could add a third test that does have data (there's already in-depth testing of the grid in the grid test, so the one here could be super simple and just check number of rows or something to make sure the page does correctly handle a non-empty data array)

BCerki · 2024-11-21T20:11:18Z

bc_obps/registration/api/v2/user_operators.py

+def list_user_operators_v2(
+    request: HttpRequest,
+    filters: UserOperatorFilterSchema = Query(...),
+    sort_field: Optional[str] = "created_at",


created_at isn't one of the grid columns, does that matter?

Yes, created_at serves as the default value for sorting when users do not specify their own sorting preferences.(It's used for the query)

Signed-off-by: SeSo <[email protected]>

…tier Signed-off-by: SeSo <[email protected]>

Signed-off-by: SeSo <[email protected]>

BCerki

🤸

Sepehr-Sobhani force-pushed the 2160-internal-user-can-view-access-requests branch 3 times, most recently from 18ab093 to 80ef128 Compare November 19, 2024 23:55

BCerki reviewed Nov 21, 2024

View reviewed changes

Sepehr-Sobhani added 15 commits November 21, 2024 13:56

feat: internal users can view access requests

7e3514d

Signed-off-by: SeSo <[email protected]>

test: update endpoint permission test

58bae19

Signed-off-by: SeSo <[email protected]>

chore: update dashboard tiles

1afdbb6

Signed-off-by: SeSo <[email protected]>

chore: update fixtures to have better data in the grid

230a980

Signed-off-by: SeSo <[email protected]>

chore: more informative errors when debugging

06c1312

Signed-off-by: SeSo <[email protected]>

chore: fix eslint errors

987087e

Signed-off-by: SeSo <[email protected]>

chore: limit service access to irc users

ae61bb7

Signed-off-by: SeSo <[email protected]>

chore: pass user guid to service

a12ba25

Signed-off-by: SeSo <[email protected]>

chore: modify data access service and add tests for it

3981d17

Signed-off-by: SeSo <[email protected]>

test: add service and endpoint tests

98cac1f

Signed-off-by: SeSo <[email protected]>

test: add frontend tests

c165656

Signed-off-by: SeSo <[email protected]>

chore: use the related data model for filtering status

fa79846

Signed-off-by: SeSo <[email protected]>

chore: non-related - make admin table for user and user-operator pret…

e6186b6

…tier Signed-off-by: SeSo <[email protected]>

test: add more endpoint tests

a8e0aff

Signed-off-by: SeSo <[email protected]>

chore: implement PR review suggestions

5f1c36c

Signed-off-by: SeSo <[email protected]>

Sepehr-Sobhani force-pushed the 2160-internal-user-can-view-access-requests branch from 9c23643 to 5f1c36c Compare November 21, 2024 21:57

BCerki approved these changes Nov 21, 2024

View reviewed changes

Sepehr-Sobhani merged commit 7f9684f into develop Nov 21, 2024
40 checks passed

Sepehr-Sobhani deleted the 2160-internal-user-can-view-access-requests branch November 21, 2024 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2160 internal user can view access requests #2469

2160 internal user can view access requests #2469

Sepehr-Sobhani commented Nov 14, 2024 •

edited

Loading

BCerki left a comment

BCerki Nov 20, 2024

BCerki Nov 20, 2024

Sepehr-Sobhani Nov 21, 2024

BCerki Nov 20, 2024

Sepehr-Sobhani Nov 21, 2024

BCerki Nov 20, 2024

BCerki Nov 20, 2024

BCerki Nov 20, 2024

Sepehr-Sobhani Nov 21, 2024

BCerki Nov 21, 2024

Sepehr-Sobhani Nov 21, 2024

andrea-williams Nov 21, 2024

BCerki Nov 21, 2024

BCerki Nov 21, 2024

BCerki Nov 21, 2024

BCerki Nov 21, 2024

Sepehr-Sobhani Nov 21, 2024

BCerki left a comment

2160 internal user can view access requests #2469

2160 internal user can view access requests #2469

Conversation

Sepehr-Sobhani commented Nov 14, 2024 • edited Loading

BCerki 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BCerki left a comment

Choose a reason for hiding this comment

Sepehr-Sobhani commented Nov 14, 2024 •

edited

Loading