Skip to content

feat: policy signature flow#7518

Draft
vitormattos wants to merge 1793 commits into
mainfrom
feat/policy-signature-flow-phase1-groundwork
Draft

feat: policy signature flow#7518
vitormattos wants to merge 1793 commits into
mainfrom
feat/policy-signature-flow-phase1-groundwork

Conversation

@vitormattos
Copy link
Copy Markdown
Member

No description provided.

@vitormattos vitormattos added this to the Next Major (34) milestone Apr 14, 2026
@vitormattos vitormattos self-assigned this Apr 14, 2026
@github-project-automation github-project-automation Bot moved this to 0. Needs triage in Roadmap Apr 14, 2026
@vitormattos vitormattos marked this pull request as draft April 14, 2026 17:54
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 2 times, most recently from 955dec8 to 2b5c509 Compare April 14, 2026 20:24
@vitormattos vitormattos marked this pull request as ready for review April 14, 2026 20:34
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 3 times, most recently from e0949a0 to e843d15 Compare April 23, 2026 14:49
@vitormattos vitormattos marked this pull request as draft April 23, 2026 14:50
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 5 times, most recently from a34a632 to 6aa5f3b Compare April 28, 2026 13:46
vitormattos added a commit that referenced this pull request Apr 28, 2026
FIXES:
- Register all 6 signature_text individual policy keys in SignatureTextPolicy
- Each key now has proper normalizers and defaults matching backend
- Fix mock object type errors by converting willReturnMap to willReturnCallback
- Mock callbacks now always return string (never null) to match type hints
- Update mock expectations from once()/exactly(2) to atLeastOnce() for deleteKey()
- Align test expectations with actual migration call counts

DETAILS:
* SignatureTextPolicy: Now exposes 6 individual keys via keys() and get()
* Each key (template, template_font_size, signature_width, etc.) has proper specs
* Render mode key includes allowed values: 'default', 'graphic', 'text'
* Migration tests: Fixed mock return types to prevent TypeError
* All getValueString() calls now guaranteed to return string via callback
* Adjusted deleteKey() and setValueString() expectations to handle migration flow

Test Results Expected:
- 0 Unknown policy key errors (all 6 now registered)
- 0 Mock type errors (callbacks always return string)
- 0 Mock expectation violations (atLeastOnce accounts for cleanup calls)
vitormattos added a commit that referenced this pull request Apr 28, 2026
…olicy

- Fix useSignatureTextPolicy.ts accessing .effectiveValue instead of non-existent .value
- Simplify useSignatureTextPolicy return type annotation using ComputedRef
- Fix model.ts serializeSignatureTextPolicyConfig to return JSON string
- Fix model.ts normalizeSignatureTextPolicyConfig to handle JSON string inputs
- Fix SignatureTextRuleEditor.vue Emits type to use EffectivePolicyValue
- Remove trailing newline from SignatureTextPolicy.php

Fixes: TypeScript type checking and PHP-CS formatting failures in PR #7518
Tests: npm run ts:check passes, php-cs formatting validated
vitormattos added a commit that referenced this pull request Apr 28, 2026
FIXES:
- Register all 6 signature_text individual policy keys in SignatureTextPolicy
- Each key now has proper normalizers and defaults matching backend
- Fix mock object type errors by converting willReturnMap to willReturnCallback
- Mock callbacks now always return string (never null) to match type hints
- Update mock expectations from once()/exactly(2) to atLeastOnce() for deleteKey()
- Align test expectations with actual migration call counts

DETAILS:
* SignatureTextPolicy: Now exposes 6 individual keys via keys() and get()
* Each key (template, template_font_size, signature_width, etc.) has proper specs
* Render mode key includes allowed values: 'default', 'graphic', 'text'
* Migration tests: Fixed mock return types to prevent TypeError
* All getValueString() calls now guaranteed to return string via callback
* Adjusted deleteKey() and setValueString() expectations to handle migration flow

Test Results Expected:
- 0 Unknown policy key errors (all 6 now registered)
- 0 Mock type errors (callbacks always return string)
- 0 Mock expectation violations (atLeastOnce accounts for cleanup calls)

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
vitormattos added a commit that referenced this pull request Apr 28, 2026
…olicy

- Fix useSignatureTextPolicy.ts accessing .effectiveValue instead of non-existent .value
- Simplify useSignatureTextPolicy return type annotation using ComputedRef
- Fix model.ts serializeSignatureTextPolicyConfig to return JSON string
- Fix model.ts normalizeSignatureTextPolicyConfig to handle JSON string inputs
- Fix SignatureTextRuleEditor.vue Emits type to use EffectivePolicyValue
- Remove trailing newline from SignatureTextPolicy.php

Fixes: TypeScript type checking and PHP-CS formatting failures in PR #7518
Tests: npm run ts:check passes, php-cs formatting validated
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 3 times, most recently from 1c39cdb to 892fbd0 Compare April 28, 2026 17:08
@vitormattos vitormattos marked this pull request as ready for review April 28, 2026 20:48
@vitormattos vitormattos marked this pull request as draft April 28, 2026 20:52
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 3 times, most recently from c6cc732 to 0e665a6 Compare May 4, 2026 14:31
Comment thread playwright/support/nc-provisioning.ts Fixed
Comment thread playwright/support/nc-provisioning.ts Fixed
Comment thread playwright/support/nc-provisioning.ts Fixed
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
When a group admin creates or edits a group-scope rule for a group they
already manage, showing the Authorized requester groups selector is
misleading: the system administrator already controls who is authorized
at that scope via allowChildOverride, and the editor enforces that the
managed group cannot be removed from the allow list anyway.

Changes:
- Add  computed in RequestSignGroupsRuleEditor.vue that
  returns true when the user is a non-admin and ALL selected target groups
  are within their manageable scope. In that case only the Denied section
  is rendered, removing visual noise.
-  preserves the inherited allowGroups from props.modelValue
  when hideAllowGroups is active, so the allow list is never silently
  zeroed by a deny-only interaction.
- Hide the 'Your managed group must remain authorized' warning when
  hideAllowGroups is active (the warning is unreachable in that state).
- Fix syncCreateDraftValueFromTargets in realDefinition.ts to accept an
  isInstanceAdmin boolean (4th arg). Group admins no longer get their
  own group pre-filled in allowGroups when creating a rule for their
  managed group — that pre-fill only makes sense for sysadmins.
- Expand RealPolicySettingDefinition type and update the call site in
  useRealPolicyWorkbench.ts to pass isInstanceAdmin.
- Update RequestSignGroupsRuleEditor.spec.ts: replace the obsolete
  're-adds managed target group' test with three targeted tests covering
  the new conditional rendering and payload preservation behavior.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
When creating a group-scope rule for groups_request_sign the workbench
dialog showed a 'Scope groups' NcSelectUsers selector AND a separate
'Authorized groups' editor field — two independent group selectors for
the same concept.  This was confusing: the groups authorised to sign
ARE the scope targets.

Introduce an extensible  callback on
RealPolicySettingDefinition.  When a definition provides this callback:

* PolicyRuleEditorPanel hides the NcSelectUsers scope-group selector
  and always passes hasSelectedTargets=true to the editor component
  (so the editor is never blocked on a selector that no longer exists).
* useRealPolicyWorkbench.updateDraftValue automatically syncs
  editorDraft.targetIds from the callback result each time the editor
  emits a new value (allow groups → targetIds).
* canSaveDraft skips the 'targetIds must be non-empty' gate; the
  allow-groups content is what must be non-empty instead
  (hasSelectableDraftValue already enforces this).
* Catalog.vue computes hideTargetSelector from the definition and
  passes it to PolicyRuleEditorPanel.

requestSignGroupsRealDefinition registers
  extractScopeTargets: (_scope, value) => resolveRequestSignGroups(value)

so the dialog now shows only 'Authorized' and 'Denied' sections,
with no redundant scope-group selector.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
The previous groups_request_sign UX change made group-scope targetIds
derive from the current allowGroups selection. That fixed the redundant
Scope groups selector, but it also introduced a regression for group
admins: as soon as they selected an authorized group, the editor started
treating that live derived target as the original rule scope and hid the
Authorized field.

Track the original target IDs of the editor session separately from the
mutable derived targetIds. The RequestSignGroupsRuleEditor now bases its
hideAllowGroups and requiredManagedGroupId logic on the stable initial
scope targets instead of the live allowGroups-derived targetIds.

This keeps Authorized visible while creating a rule whose scope is being
derived from allowGroups, while preserving the existing hide behavior
for rules that truly started with a managed group target.

Add a regression test covering the create flow where derived targetIds
become manageable after selecting an authorized group.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
When a system administrator has already created a groups_request_sign
rule, the group-admin create flow should inherit that explicit allow
list so the rule can be saved without showing the redundant scope-group
selector or forcing the user to reselect their own managed group.

Seed the group create draft only from explicitly sourced system/global
policies, not from the generic fallback default.  That keeps the create
button enabled for deny-only edits on the managed group while avoiding
false prefill from unrelated fallback values.

Also mark request-sign groups as baseline-seedable when the inherited
value already contains allow groups, and add a regression test covering
the explicit-system-rule seed path.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…gression

When group admin creates a deny rule with system admin's baseline allow groups,
avoid extracting targetIds from baseline value. This prevents the Authorized
section from hiding when user edits allowGroups selection, while still:

- Enabling the Create button (value is seeded with allowGroups)
- Keeping the Authorized field visible (targetIds not auto-populated)
- Forcing explicit group selection (safer UX)

Updates test expectation to reflect new behavior: targetIds remain empty
after baseline seed, requiring user to explicitly select their group target.

Fixes regression where field disappeared while editing allow groups selector.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 0. Needs triage

Development

Successfully merging this pull request may close these issues.

2 participants