Skip to content

Conversation

Copy link

Copilot AI commented Nov 1, 2025

Reviewed PHPStan configuration changes made in f67e21d that address code review feedback about overly broad error suppressions in PEST test files.

Changes Verified

  • HTTP method suppressions: Now explicitly lists Laravel TestCase methods (get|post|put|patch|delete|getJson|postJson|putJson|patchJson|deleteJson) instead of only getJson()
  • Assertion suppressions: Changed from wildcard assert.* to explicit list (assertStatus|assertJson|assertJsonStructure|assertJsonFragment|assertJsonPath|assertExactJson|assertSee|assertDontSee)
  • Documentation: Added comment explaining PEST framework's PHPStan inference limitations

The suppressions are now appropriately scoped to known Laravel testing methods while maintaining PHPStan level max compliance.

Before (ea92bfe):

- message: '#Call to an undefined method PHPUnit\\Framework\\TestCase::getJson\(\)#'
  path: tests/Feature/*
- message: '#Cannot call method assert.*\(\) on mixed#'  # Too broad
  path: tests/Feature/*

After (f67e21d):

# PEST testing framework limitations: PHPStan cannot infer Laravel TestCase methods
# in PEST's closure-based tests. These are valid Laravel testing methods.
- message: '#Call to an undefined method PHPUnit\\Framework\\TestCase::(get|post|...|deleteJson)\(\)#'
  path: tests/Feature/*
- message: '#Cannot call method (assertStatus|assertJson|...|assertDontSee)\(\) on mixed#'
  path: tests/Feature/*

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

kevalyq and others added 3 commits November 1, 2025 19:58
- Install and configure spatie/laravel-permission with teams support
- Configure tenant_id as team_foreign_key for multi-tenancy
- Create SetTenant middleware extracting tenant from path/header
- Validate tenant existence and set PermissionRegistrar team ID
- Register middleware alias in bootstrap/app.php
- Add 9 comprehensive PEST tests for middleware functionality
- Test tenant isolation, missing tenant, invalid tenant scenarios
- Configure PHPStan ignores for vendor migration and test methods

Resolves #50 (PR-3: Tenant Middleware & RBAC Wiring)

Quality checks:
✅ PSR-12 compliant (Pint)
✅ PHPStan level max (0 errors)
✅ 39 tests passing (89 assertions)
✅ 593 LOC (under 600 limit)
- Replace broad Feature/* suppressions with targeted method patterns
- Only ignore known Laravel TestCase HTTP methods (getJson, etc.)
- Only ignore known Laravel assertion methods (assertStatus, etc.)
- Add detailed comment explaining PEST framework PHPStan limitations
- Addresses Copilot review feedback for narrower suppressions
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Base automatically changed from feat/pr3-tenant-middleware-rbac to main November 1, 2025 19:11
Copilot AI changed the title [WIP] Add tenant middleware and RBAC wiring Review and confirm PHPStan suppression specificity improvements Nov 1, 2025
Copilot AI requested a review from kevalyq November 1, 2025 19:13
Copilot finished work on behalf of kevalyq November 1, 2025 19:13
@kevalyq kevalyq closed this Nov 1, 2025
@kevalyq kevalyq deleted the copilot/sub-pr-55 branch November 10, 2025 18:44
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.

3 participants