Conversation
- Add phpstan.neon.dist with level 4 configuration - Add phpstan-baseline.neon.dist for framework false positives - Fix PHPStan errors: * Correct @Property syntax in UserInvitation (add $ prefix) * Fix return type hints in UserController * Add missing return statement in Link() method * Fix Convert::array2json() call (use json_encode in SS6) * Update parameter type hints - Fix all PHPUnit tests (25/25 passing): * Update Groups format in fixtures to JSON * Add LastEdited to expired invitation fixtures * Fix URL assertions (add leading slashes) * Fix validation test to properly validate after config change * Update testGetCMSFields to check for readonly TempHash field - All tests passing: PHPUnit ✅ PHPStan level 4 ✅
- Added PHPStan level 4 configuration - Fixed all 36 PHPStan errors (code quality improvements) - Fixed all 25 PHPUnit tests (100% passing) - Updated Groups format to JSON throughout tests and fixtures - Fixed URL assertions (added leading slashes) - Fixed validation tests (create new controller after config change) - Added @config annotations to all static properties - Removed phpstan-baseline.neon.dist - Added ignoreErrors for property.onlyWritten identifier (SilverStripe config properties read via Config::inst()->get()) - Added testGetInvitationLink() to validate URL generation - Verified CMS custom action displays correctly - Tested email sending via Mailpit (invitation emails working) All tests passing: 26/26 tests, 60 assertions PHPStan: Clean at level 4
Coverage reporting not needed for this module
There was a problem hiding this comment.
Pull Request Overview
This PR improves code quality and test coverage for the user-invitation module by implementing PHPStan level 4 compliance and enhancing tests. The changes focus on fixing PHPDoc annotations, correcting data serialization formats, and adding comprehensive test coverage for invitation link generation.
- Configures PHPStan level 4 with appropriate ignores for SilverStripe patterns
- Fixes 36 PHPStan errors through improved type hints and annotations
- Updates test fixtures and code to use JSON format for group data serialization
- Adds new test for invitation URL generation
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| phpstan.neon.dist | Adds PHPStan level 4 configuration with ignores for optional dependencies and SilverStripe config property patterns |
| src/Model/UserInvitation.php | Adds @config annotations to static properties, fixes PHPDoc syntax (adds $ prefix and proper @method format), updates return type documentation |
| src/Control/UserController.php | Adds @config annotation, fixes PHPDoc parameter and return types, replaces deprecated Convert::array2json() with json_encode(), adds missing return statement |
| src/Admin/UserInvitationsAdmin.php | Adds @config annotations to all static configuration properties |
| tests/php/UserInvitationTest.yml | Adds LastEdited field to expired invitation fixture (required for isExpired() method) |
| tests/php/UserInvitationTest.php | Updates test comment to reflect readonly field behavior, adds new testGetInvitationLink() for URL generation validation |
| tests/php/UserControllerTest.yml | Updates Groups format from CSV to JSON, adds LastEdited to expired invitation |
| tests/php/UserControllerTest.php | Updates test assertions for JSON group format, fixes validation test to create new controller after config change, corrects URL assertions with leading slashes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @property string Groups | ||
| * @property int InvitedByID | ||
| * @property Member InvitedBy | ||
| * @property string $FirstName |
There was a problem hiding this comment.
There is trailing whitespace after $FirstName in the PHPDoc comment. This should be removed to maintain clean code formatting.
|
|
||
| /** | ||
| * @config | ||
| */ |
There was a problem hiding this comment.
There is trailing whitespace after @config in the PHPDoc comment. This should be removed to maintain clean code formatting.
| { | ||
| /** @var UserInvitation $joe */ | ||
| $joe = $this->objFromFixture(UserInvitation::class, 'joe'); | ||
|
|
There was a problem hiding this comment.
This line contains only whitespace characters. Empty lines should be completely empty with no trailing spaces or tabs.
Suggested change
PHPStan not found in CI - silverstan provides phpstan/phpstan
CmsActions is now a required dependency, so the class is found
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
1 participant
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.
This PR improves test coverage and code quality for the user-invitation module.
Changes
PHPStan Level 4
phpstan.neon.distwith level 4 configuration@configannotations to all static configuration propertiesignoreErrorsforproperty.onlyWrittenidentifier (SilverStripe framework pattern)Test Improvements
testGetInvitationLink()to validate URL generationLastEditedto expired invitation fixturesCode Quality
CMS Testing
Test Results
✅ All tests passing: 26/26 tests, 60 assertions
✅ PHPStan: Clean at level 4
✅ PHPCS: Clean
Verification
The changes have been tested in a live DDEV environment: