-
Notifications
You must be signed in to change notification settings - Fork 115
Change state of applicabilities #641
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
Open
SachaProbo
wants to merge
1
commit into
main
Choose a base branch
from
add-soa-2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gearnode
reviewed
Jan 5, 2026
gearnode
reviewed
Jan 5, 2026
gearnode
reviewed
Jan 5, 2026
gearnode
reviewed
Jan 5, 2026
1c68fb0 to
975dcde
Compare
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.
11 issues found across 65 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="pkg/authz/permissions.go">
<violation number="1" location="pkg/authz/permissions.go:320">
P2: Permission inconsistency: `ActionListStateOfApplicabilities` uses `CoreRoles` at the Organization level, but entity-level read permissions use `NonEmployeeRoles`. This means Auditors can access individual SoA records but cannot list them. Consider using `NonEmployeeRoles` for consistency with entity-level permissions, similar to how Snapshots handle this.</violation>
</file>
<file name="apps/console/src/pages/organizations/state-of-applicabilities/StateOfApplicabilitiesPage.tsx">
<violation number="1" location="apps/console/src/pages/organizations/state-of-applicabilities/StateOfApplicabilitiesPage.tsx:66">
P2: `hasAnyAction` checks for `updateStateOfApplicability` permission, but the ActionDropdown only contains a delete action. If a user has update permission but not delete permission, they will see an Actions column with an empty dropdown. Either add an edit action to the dropdown, or remove the update permission check from `hasAnyAction`.</violation>
</file>
<file name="apps/console/src/routes/stateOfApplicabilityRoutes.ts">
<violation number="1" location="apps/console/src/routes/stateOfApplicabilityRoutes.ts:29">
P2: Inconsistent route path naming: `state-of-applicabilities` vs `states-of-applicability`. The snapshot routes use a different naming convention than the non-snapshot routes. Consider using consistent naming across all routes (e.g., all use `state-of-applicabilities`).</violation>
</file>
<file name="pkg/probo/state_of_applicability_service.go">
<violation number="1" location="pkg/probo/state_of_applicability_service.go:494">
P1: PDF generation is performed inside a database transaction, keeping the connection open during slow I/O. Move PDF generation outside the transaction by first fetching all required data, then closing the transaction, and finally generating the PDF.
(Based on your team's feedback about splitting data fetching and PDF generation.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/console/src/components/form/StateOfApplicabilityControlsField.tsx">
<violation number="1" location="apps/console/src/components/form/StateOfApplicabilityControlsField.tsx:281">
P1: `useMemo` should not be used for side effects. This hook is meant for memoizing computed values, not triggering state updates. The `setFrameworkDataMap` call here may not execute reliably in React's StrictMode or with concurrent features. Use `useEffect` instead.</violation>
</file>
<file name="pkg/coredata/state_of_applicability_control_state.go">
<violation number="1" location="pkg/coredata/state_of_applicability_control_state.go:49">
P2: The `Scan` method does not validate the input against valid enum values. Unlike `MeasureState` which validates via `UnmarshalText`, this implementation silently accepts any string from the database. Consider adding validation to return an error for invalid values.</violation>
</file>
<file name="pkg/coredata/state_of_applicability_control.go">
<violation number="1" location="pkg/coredata/state_of_applicability_control.go:85">
P1: Query selects `tenant_id` but `StateOfApplicabilityControl` struct has no `TenantID` field. This will cause a runtime error with `pgx.RowToAddrOfStructByName`.</violation>
<violation number="2" location="pkg/coredata/state_of_applicability_control.go:136">
P0: Table name typo: `state_of_applicabilities_controls` should be `states_of_applicability_controls`. This will cause runtime errors as the table doesn't exist.</violation>
</file>
<file name="apps/console/src/pages/organizations/state-of-applicabilities/StateOfApplicabilityDetailPage.tsx">
<violation number="1" location="apps/console/src/pages/organizations/state-of-applicabilities/StateOfApplicabilityDetailPage.tsx:87">
P1: Navigation is called immediately after `deleteStateOfApplicability()`, but the delete shows a confirmation dialog and performs the mutation asynchronously. The user will be navigated away before they can even interact with the confirmation dialog. The navigation should only happen after successful deletion, not immediately when `onDelete` is triggered. Consider modifying `useDeleteStateOfApplicability` to accept an `onSuccess` callback, or handle navigation within the hook itself.</violation>
</file>
<file name="apps/console/src/pages/organizations/state-of-applicabilities/dialogs/EditControlDialog.tsx">
<violation number="1" location="apps/console/src/pages/organizations/state-of-applicabilities/dialogs/EditControlDialog.tsx:159">
P1: Cancel button inside form will trigger form submission. Add `type="button"` to prevent the default submit behavior.</violation>
</file>
<file name="pkg/coredata/migrations/20260102T134633Z.sql">
<violation number="1" location="pkg/coredata/migrations/20260102T134633Z.sql:33">
P2: Unique constraint on `(name, snapshot_id)` won't prevent duplicate names when `snapshot_id` is NULL because PostgreSQL treats NULL values as distinct. Consider using a partial unique index or `NULLS NOT DISTINCT` (PostgreSQL 15+):
```sql
CREATE UNIQUE INDEX states_of_applicability_name_tenant_uniq
ON states_of_applicability (name, tenant_id)
WHERE snapshot_id IS NULL;
Also consider including tenant_id in uniqueness constraints for proper multi-tenant isolation.
</details>
<sub>Reply with feedback, questions, or to request a fix. Tag `@cubic-dev-ai` to re-run a review.</sub>
apps/console/src/components/form/StateOfApplicabilityControlsField.tsx
Outdated
Show resolved
Hide resolved
.../console/src/pages/organizations/state-of-applicabilities/StateOfApplicabilityDetailPage.tsx
Outdated
Show resolved
Hide resolved
apps/console/src/pages/organizations/state-of-applicabilities/StateOfApplicabilitiesPage.tsx
Outdated
Show resolved
Hide resolved
1417c4f to
fc18a7a
Compare
Signed-off-by: Sacha Al Himdani <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test-35.pdf
Summary by cubic
Introduces States of Applicability (SoA) to manage control states (implemented, not implemented, excluded) per framework. SoA can be snapshotted.
New Features
Migration
Written for commit 0b4aea9. Summary will update on new commits.