Skip to content

test: align PageController and validation page tests with the sanitized validation payload - #9064

Open
maia-andre wants to merge 2 commits into
LibreSign:mainfrom
maia-andre:test/page-controller-validation-policy-args
Open

maia-andre wants to merge 2 commits into
LibreSign:mainfrom
maia-andre:test/page-controller-validation-policy-args

Conversation

@maia-andre

Copy link
Copy Markdown
Contributor

📝 Summary

Since the validation page security fixes landed on main (20d806a, 37d92dd), every pull request against main fails the same two checks, whatever it changes:

  • PHPUnit (all databases, all PHP versions): the 11 PageControllerTest cases fail with ArgumentCountError: PageController::__construct(): Argument #10 ($validationEffectivePolicyService) not passed.
  • Node tests: Validation.spec.ts > handleValidationSuccess > updates the policies store when requester effective policies are returned still expects setPolicies, which the page no longer calls.

This PR only updates those tests to the new contract. No production code changes.

  • PageControllerTest passes a real ValidationEffectivePolicyService (the class is final) built around the PolicyService mock each test already configures. The public validation case now also asserts the policy payload the page receives (['policies' => []] when there is no legal information), instead of only checking that the key exists.
  • Validation.spec.ts checks that the legal information returned with the validation is shown and that the global policy store is not touched, which is the intent of 37d92dd.

🧪 How to test

composer test:integration -- --filter PageControllerTest
npx vitest run src/tests/views/Validation.spec.ts

Validation evidence

✅ Checklist

  • I have read and followed the contribution guide.
  • The diff does not contain unrelated refactors or generated changes.
  • Commits follow Conventional Commits where applicable.
  • All commits include the required DCO sign-off.

🤖 AI (if applicable)

  • The content of this PR was partially or fully generated using AI

20d806a added ValidationEffectivePolicyService to the PageController
constructor, and every PageControllerTest case has failed with an
ArgumentCountError since then. The service is final, so the tests build
it around the PolicyService mock they already configure.

The public validation case now also checks the payload it hands to the
page: the requester policies are resolved without the user scope, and
with no legal information the page receives an empty policy list.

Signed-off-by: André Maia <andrefnkmm@gmail.com>
…store

37d92dd stopped the validation page from writing the requester's
effective policies into the global policy store; it keeps only the
legal information from the payload. The test still expected
setPolicies to be called.

It now checks that the legal information returned with the validation
is shown and that the policy store is left untouched.

Signed-off-by: André Maia <andrefnkmm@gmail.com>
@maia-andre
maia-andre requested a review from a team as a code owner October 9, 2026 00:58
@maia-andre

Copy link
Copy Markdown
Contributor Author

@vitormattos @YvesCesar could one of you take a look when you have a moment? Since the validation-page security fix, main fails PageControllerTest and Validation.spec.ts, so the PHPUnit matrix is red on every open PR (e.g. #9063). This PR only aligns those tests with the sanitized validation payload; CI is green.

@YvesCesar YvesCesar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maia-andre !

I wrote a review. Could you take a look at it?

}),
})
expect(wrapper.vm.legalInformation).toBe('Requester legal copy')
expect(policiesStore.setPolicies).not.toHaveBeenCalled()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The view Validation.vue no longer imports store/policies (neither do its child components, and the spec uses shallowMount), so setPolicies can never be called here. This assertion always passes and doesn't protect anything from 37d92dd.

Please remove:

  • this assertion and the usePoliciesStore lookup at the top of the test (lines 1078-1079);
  • the vi.mock('../../store/policies', ...) block at line 187, which is only used by this test.

The legalInformation assertion is enough to cover the new behavior.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants