test: Add tests for email notifications in services #1322
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #1322 +/- ##
=================================================
+ Coverage 56.63% 57.78% +1.14%
Complexity 2268 2268
=================================================
Files 207 207
Lines 8733 8733
=================================================
+ Hits 4946 5046 +100
+ Misses 3787 3687 -100
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:
|
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage for email notifications across the application and fixes critical bugs in mail classes that were preventing proper rendering. The changes ensure mailable classes work correctly with both array and object data structures while adding proper validation through tests.
- Adds 167 lines of new tests for Study-related email notifications
- Adds 168 lines of new tests for Project-related email notifications
- Adds 180 lines of new tests for Draft processing email notifications
- Fixes bugs in StudyPublish and DraftProcessedNotifyAdmins mail classes
- Updates model fillable attributes to support proper invitation creation in tests
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/Study/StudyMailTest.php | New test file covering StudyInvitation and StudyPublish mail rendering, properties, and queueable behavior |
| tests/Feature/Project/ProjectMailTest.php | New test file covering ProjectInvitation, ProjectArchival, ProjectDeletion, and related notification mail classes |
| tests/Feature/DraftFeatureTest.php | Adds tests for DraftProcessed and DraftProcessedNotifyAdmins mail classes with various scenarios including release dates and identifier handling |
| app/Mail/StudyPublish.php | Fixes bug where studies array was accessed with array syntax instead of object property syntax |
| app/Mail/DraftProcessedNotifyAdmins.php | Fixes critical bug where release_date was parsed before checking if project exists, preventing null pointer errors |
| app/Models/ProjectInvitation.php | Adds project_id to fillable attributes to support invitation creation in tests |
| app/Models/TeamInvitation.php | Adds team_id to fillable attributes to support invitation creation in tests |
| tests/Unit/Models/ProjectInvitationModelTest.php | Updates test to verify project_id is in fillable attributes |
| ]); | ||
|
|
||
| // Update identifier via database query | ||
| \DB::table('projects')->where('id', $project->id)->update(['identifier' => 123]); |
There was a problem hiding this comment.
The DB facade is used without being imported. Add use Illuminate\Support\Facades\DB; at the top of the file, and then use DB::table() instead of \DB::table().
| $this->project->deleted_on = Carbon::now(); | ||
| putenv('COOL_OFF_PERIOD=45'); | ||
|
|
||
| $mailable = new ProjectDeletion($this->project); | ||
| $content = $mailable->render(); | ||
|
|
||
| $this->assertNotEmpty($content); |
There was a problem hiding this comment.
Using putenv() to change environment variables in tests can cause side effects and pollution between tests. Consider using Config::set() instead (which is already used elsewhere in this test file), or if you need to test environment-specific behavior, use the withoutMix() or withEnv() test helpers. Also, the changed environment variable is not cleaned up after the test, which could affect subsequent tests.
| $this->project->deleted_on = Carbon::now(); | |
| putenv('COOL_OFF_PERIOD=45'); | |
| $mailable = new ProjectDeletion($this->project); | |
| $content = $mailable->render(); | |
| $this->assertNotEmpty($content); | |
| $this->withEnv(['COOL_OFF_PERIOD' => 45], function (): void { | |
| $this->project->deleted_on = Carbon::now(); | |
| $mailable = new ProjectDeletion($this->project); | |
| $content = $mailable->render(); | |
| $this->assertNotEmpty($content); | |
| }); |
| public function test_project_invitation_mail_can_be_rendered(): void | ||
| { | ||
| $invitation = ProjectInvitationModel::create([ | ||
| 'project_id' => $this->project->id, | ||
| 'email' => '[email protected]', | ||
| 'role' => 'collaborator', | ||
| ]); | ||
|
|
||
| $mailable = new ProjectInvitation($invitation); | ||
| $content = $mailable->render(); | ||
|
|
||
| $this->assertNotEmpty($content); | ||
| } | ||
|
|
||
| public function test_project_invitation_mail_has_correct_properties(): void | ||
| { | ||
| $invitation = ProjectInvitationModel::create([ | ||
| 'project_id' => $this->project->id, | ||
| 'email' => '[email protected]', | ||
| 'role' => 'collaborator', | ||
| ]); | ||
|
|
||
| $mailable = new ProjectInvitation($invitation); | ||
|
|
||
| $this->assertSame($invitation->id, $mailable->invitation->id); | ||
| $this->assertSame($invitation->email, $mailable->invitation->email); | ||
| } |
There was a problem hiding this comment.
The ProjectInvitation mailable implements ShouldQueue, but unlike the StudyInvitation tests (line 76-87 in StudyMailTest.php), there's no test verifying this queueable behavior. Consider adding a test similar to test_study_invitation_mail_is_queueable to ensure consistency in test coverage.
No description provided.