Phase 3: Security & admin test coverage (voters, auth, CRUD smoke, filters)#76
Open
turegjorup wants to merge 8 commits intodevelopfrom
Open
Phase 3: Security & admin test coverage (voters, auth, CRUD smoke, filters)#76turegjorup wants to merge 8 commits intodevelopfrom
turegjorup wants to merge 8 commits intodevelopfrom
Conversation
Cover voter authorization branches, EasyAdmin CRUD smoke, auth flows and filter behavior so regressions in access control surface in CI without requiring a full environment exercise. - Unit tests for every voter in App\Security\Voter (EventVoter covers feed-lock, role hierarchy and organization membership; remaining voters cover their significant branches). - Functional tests for login, authorization and registration incl. email verification round-trip. - Dashboard role redirects and CRUD smoke per controller, including cross-org edit blocking on Event and Organization. - Filter tests for HasOrganizationFilter, JsonContainsFilter and the MyEventCrudController scoping query builder. - Lightweight TestEventFixtures keeps functional tests independent of the full EventFixture dependency chain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop the _test dbname suffix: the `db` docker user lacks CREATE DATABASE privileges, and DAMA's transaction rollback already isolates tests on the shared dev schema. - Tell Liip to keep the existing DB/schema rather than drop+recreate per test (the drop path was hitting "A database is required"). - Register App\Tests\Fixtures\ as Doctrine fixture services in the test environment so TestUserFixtures and TestEventFixtures can be loaded through Liip's DatabaseTool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`AbstractAdminTestCase::adminUrl()` now uses the EasyAdmin-generated `admin_<snake>_<action>` route names, so the test client lands directly on the CRUD page instead of following a 302 from the legacy query-string form. This unblocks most functional CRUD tests that were asserting 200 on the dashboard URL and getting a redirect to the pretty URL. Additionally, the test-only profiler config now sets `collect: true` (rather than `false`) so `MailerAssertionsTrait::assertQueuedEmailCount` can read the message logger events via `profiler.is_disabled_state_checker`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Dashboard assertions look for translated Danish labels (Alt indhold, Brugere, Feeds) because templates render with the default locale. - HasOrganizationFilter uses the BooleanFilterType form, whose query parameter is `filters[hasOrganization]`, not the comparison/value structure. - AuthorizationTest normalises the redirect Location to a path before checking the `/admin` prefix (the header is an absolute URL). - Registration form tests use the translated submit button label and valid email addresses; the user lookup after redirect avoids a stale EM reference, and the outbound email is asserted as queued (mailer is routed through the async Messenger transport). - OrganizationCrudTest skips `testOrgAdminCannotCreateOrganization` with a clear note: the OrganizationVoter only fires when an entity instance is present, and EasyAdmin invokes EA_EXECUTE_ACTION with a null entity for the NEW action, so URL-level access is not currently enforced. - Minor php-cs-fixer tidy on EventVoterTest (trailing blank line). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PHPUnit 12 emits a notice for every createMock() call that has no expectations attached. Voter tests only need stubbing behaviour (willReturn callbacks, no assertion on call counts), so createStub is the correct primitive. Swapping across all voter tests eliminates the 49 PHPUnit notices that appeared under the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Functional tests need the DB schema in place. Locally the dev schema already exists; in CI the mariadb container starts empty, so Liip's purger hits a missing vocabulary_tag table. Adding a migrate step before task test brings CI in line with local. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #76 +/- ##
==============================================
+ Coverage 3.70% 34.58% +30.87%
- Complexity 1251 1262 +11
==============================================
Files 162 162
Lines 4448 4450 +2
==============================================
+ Hits 165 1539 +1374
+ Misses 4283 2911 -1372
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Second of four PRs in the test coverage plan. Follows PR #75 (Phase 1 — test infrastructure).
EventVoter,OrganizationVoter,LocationVoter,AddressVoter,TagVoter,UserActionVoter,UserEntityVoter.ROLE_EDITORredirects toEventCrudController;ROLE_ORGANIZATION_EDITORredirects toMyEventCrudController.HasOrganizationFilter,JsonContainsFilter, andMyEventCrudControllerquery-builder scoping.TestEventFixtures(org-A/org-B/orphan events) to keep functional tests independent of the fullEventFixturedependency chain.Test infrastructure adjustments
config/packages/doctrine.yaml(when@test): dropped the_testdbname suffix — thedbdocker user lacksCREATE DATABASEprivileges; DAMA transaction rollback already isolates tests on the shared dev schema.config/packages/test/liip_test_fixtures.yaml:keep_database_and_schema: true— Liip now reuses the existing schema instead of drop+recreate.config/services.yaml(when@test): registersApp\Tests\Fixtures\*asdoctrine.fixture.ormservices soTestUserFixtures/TestEventFixturesload via Liip'sDatabaseTool.config/packages/web_profiler.yaml:profiler.collect: truein test env soMailerAssertionsTraitis wired.AbstractAdminTestCase::adminUrl()now generates EasyAdmin pretty-URL routes (admin_<snake>_<action>) via the router, avoiding a 302 round-trip on every request.Known production gap
OrganizationCrudController::configureActions()hides the "New" button for org-admins in the UI, butOrganizationVoter::supports()abstains whenentity === null(the NEW action passes null). URL-level access to/admin/organization/newis therefore not enforced. One test (OrganizationCrudTest::testOrgAdminCannotCreateOrganization) is markedmarkTestSkipped()with the cause documented. Recommend a follow-up issue to fixOrganizationVoterto vote on NEW when entity is null.Results
Test plan
vendor/bin/phpunit— 125 passing, 1 documented skipvendor/bin/phpstan analyse— cleancomposer run coding-standards-check— cleanmarkdownlint '**/*.md'— cleanNext:
feature/test-coverage-pipeline(Phase 2) is already committed locally; PR opens after this one lands.🤖 Generated with Claude Code