Skip to content

Commit 92f4fa6

Browse files
authored
Permissions settings for enforcements (#636)
* requirement source title related fix * permissions settings for enforcements
1 parent d8a3a70 commit 92f4fa6

File tree

19 files changed

+239
-118
lines changed

19 files changed

+239
-118
lines changed

compliance-api/src/compliance_api/schemas/inspection_approval.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from compliance_api.models.inspection_record_approval import InspectionRecordApproval as InspectionRecordApprovalModel
88
from compliance_api.models.inspection_record_approval import IRApprovalStatusEnum
9+
from compliance_api.schemas.common import KeyValueSchema
910
from compliance_api.utils.constant import INPUT_DATE_TIME_FORMAT
1011

1112
from .base_schema import AutoSchemaBase, BaseSchema

compliance-api/src/compliance_api/services/administrative_penalty.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,12 @@ def update_administrative_penalty(cls, administrative_penalty_id, update_data):
182182
inspection_id=update_data.get("inspection_id")
183183
or administrative_penalty.inspection_id
184184
)
185-
ServiceUtils.access_check_update_for_inspection(inspection)
186-
# If administrative penalty is closed, then check the inspection status
187-
# No more modification possible once the AP and IR is closed
185+
# Primary or super user can update ap if it is not closed
186+
if not administrative_penalty.is_closed:
187+
ServiceUtils.access_check_update_for_inspection(inspection)
188+
# Super user can update ap if it is closed
188189
if administrative_penalty.is_closed:
189-
ServiceUtils.inspection_status_check(inspection)
190+
ServiceUtils.access_check_for_super_user()
190191

191192
requirement_ids = update_data.get("inspection_requirement_ids", [])
192193
if requirement_ids:

compliance-api/src/compliance_api/services/charge_recommendation.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,12 @@ def update_charge_recommendation(
8787
inspection = ServiceUtils.inspection_exist_check(
8888
inspection_id=update_data.get("inspection_id")
8989
)
90-
ServiceUtils.access_check_update_for_inspection(inspection)
91-
# If charge recommendation is closed, then check the inspection status
92-
# No more modification possible once the CR and IR is closed
90+
# Primary or super user can update CR if it is not closed
91+
if not charge_recommendation.is_closed:
92+
ServiceUtils.access_check_update_for_inspection(inspection)
93+
# Super user can update CR if it is closed
9394
if charge_recommendation.is_closed:
94-
ServiceUtils.inspection_status_check(inspection)
95+
ServiceUtils.access_check_for_super_user()
9596
requirement_ids = update_data.get("inspection_requirement_ids", [])
9697
with session_scope() as session:
9798
extracted_update_data = _extract_cr_data(update_data)

compliance-api/src/compliance_api/services/inspection.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -497,14 +497,12 @@ def generate_inspections_excel(cls, args):
497497
return output
498498

499499
@classmethod
500-
def get_pending_items(cls, inspection_id: int):
500+
def get_pending_items(cls, inspection_id: int): # pylint: disable=too-many-locals
501501
"""Get pending items for an inspection (optimized with single query).
502502
503503
Returns a list of items (enforcement actions, inspection records) that are mapped to requirements
504504
or the inspection but not yet created or not in proper status.
505505
"""
506-
from sqlalchemy.orm import aliased
507-
508506
pending_items = []
509507
seen_item_numbers = set()
510508

@@ -521,7 +519,7 @@ def get_pending_items(cls, inspection_id: int):
521519
order_map = aliased(OrderInspectionRequirementMapModel)
522520
order = aliased(OrderModel)
523521
wl_map = aliased(WarningLetterInspectionRequirementMapModel)
524-
wl = aliased(WarningLetterModel)
522+
warning_letter = aliased(WarningLetterModel)
525523
ap_map = aliased(AdministrativePenaltyInspectionRequirementMapModel)
526524
vt_map = aliased(ViolationTicketInspectionRequirementMapModel)
527525
cr_map = aliased(ChargeRecommendationInspectionRequirementMapModel)
@@ -535,8 +533,8 @@ def get_pending_items(cls, inspection_id: int):
535533
enf_action.name.label("enforcement_action_name"),
536534
order.order_number.label("order_number"),
537535
order.order_status.label("order_status"),
538-
wl.warning_letter_number.label("warning_letter_number"),
539-
wl.status.label("warning_letter_status"),
536+
warning_letter.warning_letter_number.label("warning_letter_number"),
537+
warning_letter.status.label("warning_letter_status"),
540538
order_map.id.label("order_map_id"),
541539
wl_map.id.label("wl_map_id"),
542540
ap_map.id.label("ap_map_id"),
@@ -574,7 +572,7 @@ def get_pending_items(cls, inspection_id: int):
574572
wl_map.is_deleted.is_(False),
575573
),
576574
)
577-
.outerjoin(wl, wl.id == wl_map.warning_letter_id)
575+
.outerjoin(warning_letter, warning_letter.id == wl_map.warning_letter_id)
578576
.outerjoin(
579577
ap_map,
580578
and_(
@@ -1199,7 +1197,7 @@ def _set_charge_recommendation_enforcement_action_object(
11991197

12001198
def _build_query_for_enforcement_actions_and_requirement_details(inspection_ids):
12011199
"""Build the query for bulk fetching enforcement actions and requirement details."""
1202-
# pylint: disable=invalid-name
1200+
# pylint: disable=invalid-name,too-many-locals
12031201
# Create aliases for mapping tables
12041202
order_map_alias = aliased(OrderInspectionRequirementMapModel)
12051203
warning_letter_map_alias = aliased(WarningLetterInspectionRequirementMapModel)
@@ -1339,7 +1337,9 @@ def _build_query_for_enforcement_actions_and_requirement_details(inspection_ids)
13391337
return results
13401338

13411339

1342-
def _bulk_fetch_enforcement_actions_and_requirement_details(inspection_ids):
1340+
def _bulk_fetch_enforcement_actions_and_requirement_details(
1341+
inspection_ids,
1342+
): # pylint: disable=too-many-locals,too-many-branches
13431343
"""Bulk fetch all enforcement actions and build requirement details in a single optimized query.
13441344
13451345
Returns a dict with two keys per inspection:
@@ -1504,7 +1504,7 @@ def _bulk_fetch_enforcement_actions_and_requirement_details(inspection_ids):
15041504
return result_data
15051505

15061506

1507-
def _process_enforcement_status(
1507+
def _process_enforcement_status( # pylint: disable=too-many-return-statements,too-many-branches
15081508
enforcement_action_id: int,
15091509
order_map_id,
15101510
order_number,
@@ -1544,7 +1544,7 @@ def _process_enforcement_status(
15441544
return {"is_created": True, "item_number": order_number, "is_issued": False}
15451545
return None
15461546

1547-
elif enforcement_action_id == EnforcementActionOptionEnum.WARNING_LETTER.value:
1547+
if enforcement_action_id == EnforcementActionOptionEnum.WARNING_LETTER.value:
15481548
if not wl_map_id:
15491549
return {"is_created": False, "item_number": None}
15501550
if not warning_letter_number:
@@ -1557,20 +1557,20 @@ def _process_enforcement_status(
15571557
}
15581558
return None
15591559

1560-
elif (
1560+
if (
15611561
enforcement_action_id
15621562
== EnforcementActionOptionEnum.ADMINISTRATIVE_PENALTY_RECOMMENDATION.value
15631563
):
15641564
if not ap_map_id:
15651565
return {"is_created": False, "item_number": None}
15661566
return None
15671567

1568-
elif enforcement_action_id == EnforcementActionOptionEnum.VIOLATION_TICKET.value:
1568+
if enforcement_action_id == EnforcementActionOptionEnum.VIOLATION_TICKET.value:
15691569
if not vt_map_id:
15701570
return {"is_created": False, "item_number": None}
15711571
return None
15721572

1573-
elif (
1573+
if (
15741574
enforcement_action_id == EnforcementActionOptionEnum.CHARGE_RECOMMENDATION.value
15751575
):
15761576
if not cr_map_id:

compliance-api/src/compliance_api/services/inspection_requirement.py

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,30 +1323,29 @@ def _get_enforcement_number_by_type(result):
13231323
return ""
13241324

13251325

1326-
def _get_enforcement_status_by_type(result):
1326+
def _get_enforcement_status_by_type(result): # pylint: disable=too-many-return-statements
13271327
"""Get the correct enforcement status based on the enforcement action type."""
13281328
enforcement_action_id = result.enforcement_action_id
13291329

13301330
# Map enforcement action ID to the corresponding status field
13311331
if enforcement_action_id == EnforcementActionOptionEnum.ORDER.value:
13321332
return result.order_status
1333-
elif enforcement_action_id == EnforcementActionOptionEnum.WARNING_LETTER.value:
1333+
if enforcement_action_id == EnforcementActionOptionEnum.WARNING_LETTER.value:
13341334
return result.warning_letter_status
1335-
elif enforcement_action_id == EnforcementActionOptionEnum.VIOLATION_TICKET.value:
1335+
if enforcement_action_id == EnforcementActionOptionEnum.VIOLATION_TICKET.value:
13361336
return result.violation_ticket_status
1337-
elif (
1337+
if (
13381338
enforcement_action_id
13391339
== EnforcementActionOptionEnum.ADMINISTRATIVE_PENALTY_RECOMMENDATION.value
13401340
):
13411341
return result.admin_penalty_status
1342-
elif (
1342+
if (
13431343
enforcement_action_id == EnforcementActionOptionEnum.CHARGE_RECOMMENDATION.value
13441344
):
13451345
return result.charge_rec_status
1346-
elif enforcement_action_id == EnforcementActionOptionEnum.RESTORATIVE_JUSTICE.value:
1346+
if enforcement_action_id == EnforcementActionOptionEnum.RESTORATIVE_JUSTICE.value:
13471347
return result.restorative_justice_status
1348-
else:
1349-
return None
1348+
return None
13501349

13511350

13521351
def _get_enforcement_progress_by_type(result):
@@ -1356,10 +1355,9 @@ def _get_enforcement_progress_by_type(result):
13561355
# Only Order and Warning Letter have progress fields
13571356
if enforcement_action_id == EnforcementActionOptionEnum.ORDER.value:
13581357
return result.order_progress
1359-
elif enforcement_action_id == EnforcementActionOptionEnum.WARNING_LETTER.value:
1358+
if enforcement_action_id == EnforcementActionOptionEnum.WARNING_LETTER.value:
13601359
return result.warning_letter_progress
1361-
else:
1362-
return None
1360+
return None
13631361

13641362

13651363
def _apply_enforcement_number_sort(query, subq, sort_order):

compliance-api/src/compliance_api/services/restorative_justice.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,18 @@ def update_restorative_justice(
7777
) -> RestorativeJusticeModel:
7878
"""Update an existing restorative justice."""
7979
# Check if restorative justice exists
80-
cls.get_by_id(restorative_justice_id)
80+
restorative_justice = cls.get_by_id(restorative_justice_id)
8181

8282
# Extract inspection requirement IDs
8383
inspection_requirement_ids = update_data.pop("inspection_requirement_ids", None)
84+
inspection = ServiceUtils.inspection_exist_check(
85+
inspection_id=update_data.get("inspection_id")
86+
or restorative_justice.inspection_id
87+
)
88+
if not restorative_justice.status == RestorativeJusticeStatusEnum.CLOSED:
89+
ServiceUtils.access_check_update_for_inspection(inspection)
90+
if restorative_justice.status == RestorativeJusticeStatusEnum.CLOSED:
91+
ServiceUtils.access_check_for_super_user()
8492

8593
with session_scope() as session:
8694
# Update the restorative justice

compliance-api/src/compliance_api/services/service_utils.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ def access_check_update_for_inspection(inspection: dict):
5252
"Only the Superuser or the Primary officer on this inspection can perform this operation."
5353
)
5454

55+
@staticmethod
56+
def access_check_for_super_user():
57+
"""Access check for super user."""
58+
if not auth.has_permission([PermissionEnum.SUPERUSER]):
59+
raise PermissionDeniedError(
60+
"Only the Superuser can perform this operation."
61+
)
62+
5563
@staticmethod
5664
def inspection_exist_check(inspection_id: int):
5765
"""Check if the inspection exist or not."""

compliance-api/src/compliance_api/services/violation_ticket.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,14 @@ def update(
102102
raise ResourceNotFoundError(
103103
f"Violation ticket with ID {violation_ticket_id} not found"
104104
)
105-
106-
# Perform access and status checks
107-
ServiceUtils.access_check_update_for_inspection(violation_ticket.inspection)
108-
# If violation ticket is closed, then check the inspection status
109-
# No more modification possible once the VT and IR is closed
105+
inspection = ServiceUtils.inspection_exist_check(
106+
inspection_id=violation_ticket_data.get("inspection_id")
107+
or violation_ticket.inspection_id
108+
)
109+
if not violation_ticket.is_closed:
110+
ServiceUtils.access_check_update_for_inspection(inspection)
110111
if violation_ticket.is_closed:
111-
ServiceUtils.inspection_status_check(violation_ticket.inspection)
112+
ServiceUtils.access_check_for_super_user()
112113

113114
# Check if violation ticket can be updated based on status
114115
if violation_ticket.status == ViolationTicketStatusEnum.PAID:

compliance-web/cypress/components/_components/_App/_Inspections/_Profile/InspectionEnforcements.cy.tsx

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { AdministrativePenalty } from "@/models/AdministrativePenalty";
99
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
1010
import { Box } from "@mui/material";
1111
import { EnforcementActionEnum } from "@/utils/constants";
12+
import { AuthProvider } from "react-oidc-context";
13+
import { OidcConfig } from "@/utils/config";
1214

1315
describe("InspectionEnforcements", () => {
1416
let queryClient: QueryClient;
@@ -21,24 +23,26 @@ describe("InspectionEnforcements", () => {
2123
const mountComponent = (inspectionData = mockInspection) => {
2224
mount(
2325
<QueryClientProvider client={queryClient}>
24-
<Box
25-
sx={{
26-
width: "800px",
27-
height: "800px",
28-
overflow: "visible",
29-
position: "relative",
30-
}}
31-
>
32-
<InspectionEnforcements inspectionData={inspectionData} />
33-
</Box>
26+
<AuthProvider {...OidcConfig}>
27+
<Box
28+
sx={{
29+
width: "800px",
30+
height: "800px",
31+
overflow: "visible",
32+
position: "relative",
33+
}}
34+
>
35+
<InspectionEnforcements inspectionData={inspectionData} />
36+
</Box>
37+
</AuthProvider>
3438
</QueryClientProvider>
3539
);
3640
};
3741

3842
beforeEach(() => {
3943
// Set viewport to ensure proper dimensions
4044
cy.viewport(1200, 800);
41-
45+
4246
// Add CSS overrides to fix overflow issues in testing
4347
cy.document().then((doc) => {
4448
const style = doc.createElement("style");
@@ -62,6 +66,21 @@ describe("InspectionEnforcements", () => {
6266
},
6367
});
6468

69+
// Mock current user for useCurrentLoggedInUser hook
70+
const mockCurrentUser = { preferred_username: "test.user@gov.bc.ca" };
71+
queryClient.setQueryData(["current-user"], mockCurrentUser);
72+
73+
// Mock staff users data
74+
const mockStaffUsers = [
75+
{
76+
id: 1,
77+
name: "Test User",
78+
auth_user_guid: "test.user@gov.bc.ca",
79+
is_active: true,
80+
},
81+
];
82+
queryClient.setQueryData(["staff-users", true], mockStaffUsers);
83+
6584
// Default mock inspection data
6685
mockInspection = {
6786
id: 1,

compliance-web/src/components/App/Inspections/Profile/Enforcements/AdministrativePenalty/AdministrativePenaltyUpdateModal.tsx

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { notify } from "@/store/snackbarStore";
2323
import { useModal } from "@/store/modalStore";
2424
import dayjs, { Dayjs } from "dayjs";
2525
import { APReferralStatus, APDecisionStatus } from "@/utils/constants";
26+
import { KC_USER_GROUPS, useIsRolesAllowed } from "@/hooks/useAuthorization";
2627

2728
const administrativePenaltyUpdateSchema = yup.object().shape({
2829
referral_status: yup
@@ -76,20 +77,23 @@ const decisionOptions: DecisionOption[] = Object.values(APDecisionStatus).map(
7677
type AdministrativePenaltyUpdateModalProps = {
7778
administrativePenalty: AdministrativePenalty;
7879
inspectionData: Inspection;
80+
isPrimaryOfficerOrSuperUser: boolean;
7981
onSuccess?: (data: AdministrativePenalty) => void;
8082
};
8183

8284
const AdministrativePenaltyUpdateModal: FC<
8385
AdministrativePenaltyUpdateModalProps
84-
> = ({ administrativePenalty, inspectionData, onSuccess }) => {
86+
> = ({ administrativePenalty, inspectionData, isPrimaryOfficerOrSuperUser, onSuccess }) => {
8587
const queryClient = useQueryClient();
8688
const { setClose: setModalClose } = useModal();
87-
89+
const isSuperUser = useIsRolesAllowed([KC_USER_GROUPS.SUPERUSER]);
8890
// Calculate readonly mode based on inspection status and administrative penalty status
8991
const isReadonlyMode = useMemo(() => {
90-
const isInspectionClosed = inspectionData.inspection_status.toLowerCase() === "closed";
91-
return isInspectionClosed && administrativePenalty.is_closed;
92-
}, [inspectionData.inspection_status, administrativePenalty.is_closed]);
92+
if (administrativePenalty.is_closed) {
93+
return !isSuperUser;
94+
}
95+
return !isPrimaryOfficerOrSuperUser;
96+
}, [administrativePenalty.is_closed, isPrimaryOfficerOrSuperUser, isSuperUser]);
9397

9498
const defaultValues = useMemo(() => {
9599
const currentReferralStatus =

0 commit comments

Comments
 (0)