-
Notifications
You must be signed in to change notification settings - Fork 3
Added controller method that returns not accepted policies by user #1207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
779c6d8
c7780da
3c96fd5
0bacc5c
f2a65e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,9 @@ | |
|
|
||
| use App\Http\Resources\PoliciesCollection; | ||
| use App\Policy; | ||
| use App\PolicyAcceptance; | ||
| use Carbon\CarbonImmutable; | ||
| use Illuminate\Http\Request; | ||
|
|
||
| class PoliciesController extends Controller { | ||
| public function getCurrentPolicies(): PoliciesCollection { | ||
|
|
@@ -20,4 +22,23 @@ public function getCurrentPolicies(): PoliciesCollection { | |
|
|
||
| return new PoliciesCollection($currentPolicies); | ||
| } | ||
|
|
||
| public function getMissingPolicies(Request $request): PoliciesCollection { | ||
| $now = CarbonImmutable::now(); | ||
|
|
||
| $activePolicyIds = Policy::where('active_from', '<=', $now) | ||
| ->selectRaw('MAX(id) as id') | ||
| ->groupBy('policy_type') | ||
| ->pluck('id'); | ||
|
|
||
| $acceptedPolicyIds = PolicyAcceptance::where('user_id', $request->user()->id) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think of renaming this to |
||
| ->whereIn('policy_id', $activePolicyIds) | ||
| ->pluck('policy_id'); | ||
|
|
||
| $missingPolicies = Policy::whereIn('id', $activePolicyIds) | ||
| ->whereNotIn('id', $acceptedPolicyIds) | ||
| ->get(); | ||
|
|
||
| return new PoliciesCollection($missingPolicies); | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there are now two |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| <?php | ||
|
|
||
| namespace Tests\Routes; | ||
|
|
||
| use App\Policy; | ||
| use App\PolicyAcceptance; | ||
| use App\User; | ||
| use Carbon\CarbonImmutable; | ||
| use Illuminate\Foundation\Testing\DatabaseTransactions; | ||
| use Tests\TestCase; | ||
|
|
||
| class PoliciesControllerTest extends TestCase { | ||
| use DatabaseTransactions; | ||
|
|
||
| private string $missingPoliciesRoute = 'v1/policies/missing'; | ||
|
|
||
| private function createPolicy(string $type, CarbonImmutable $activeFrom, string $content): Policy { | ||
| return Policy::create([ | ||
| 'policy_type' => $type, | ||
| 'active_from' => $activeFrom, | ||
| 'content_vue_file' => $content, | ||
| ]); | ||
| } | ||
|
|
||
| public function testMissingPoliciesRequiresAuthentication(): void { | ||
| $this->json('GET', $this->missingPoliciesRoute) | ||
| ->assertStatus(401); | ||
| } | ||
|
|
||
| public function testMissingPoliciesReturnsOnlyLatestCurrentPolicyPerType(): void { | ||
| $user = User::factory()->create(); | ||
| $now = CarbonImmutable::now(); | ||
|
|
||
| // Future policy (should be ignored because it is not active yet) | ||
| $this->createPolicy('terms-of-use', $now->addDays(10), 'terms-of-use/version-future.vue'); | ||
|
|
||
| $olderTerms = $this->createPolicy('terms-of-use', $now->subDays(10), 'terms-of-use/version-1.vue'); | ||
| $latestTerms = $this->createPolicy('terms-of-use', $now->subDays(1), 'terms-of-use/version-2.vue'); | ||
| $this->createPolicy('terms-of-use', $now->addDays(1), 'terms-of-use/version-3.vue'); | ||
|
|
||
| $olderHosting = $this->createPolicy('hosting-policy', $now->subDays(20), 'hosting-policy/version-1.vue'); | ||
| $latestHosting = $this->createPolicy('hosting-policy', $now->subDays(2), 'hosting-policy/version-2.vue'); | ||
|
|
||
| $response = $this->actingAs($user, 'api') | ||
| ->json('GET', $this->missingPoliciesRoute) | ||
| ->assertStatus(200) | ||
| ->assertJsonStructure([ | ||
| 'items' => [ | ||
| ['metadata' => ['policy_id', 'type', 'active_from', 'content_vue_file']], | ||
| ], | ||
| ]); | ||
|
|
||
| $items = collect($response->json('items')); | ||
|
|
||
| $this->assertCount(2, $items); | ||
|
|
||
| $this->assertTrue($items->contains(function (array $item) use ($latestTerms): bool { | ||
| return $item['metadata']['policy_id'] === $latestTerms->id | ||
| && $item['metadata']['type'] === 'terms-of-use' | ||
| && $item['metadata']['active_from'] === $latestTerms->active_from->format('Y-m-d') | ||
| && $item['metadata']['content_vue_file'] === 'terms-of-use/version-2.vue'; | ||
| })); | ||
|
|
||
| $this->assertTrue($items->contains(function (array $item) use ($latestHosting): bool { | ||
| return $item['metadata']['policy_id'] === $latestHosting->id | ||
| && $item['metadata']['type'] === 'hosting-policy' | ||
| && $item['metadata']['active_from'] === $latestHosting->active_from->format('Y-m-d') | ||
| && $item['metadata']['content_vue_file'] === 'hosting-policy/version-2.vue'; | ||
| })); | ||
|
|
||
| $this->assertFalse($items->contains(function (array $item) use ($olderTerms): bool { | ||
| return $item['metadata']['policy_id'] === $olderTerms->id; | ||
| })); | ||
|
|
||
| $this->assertFalse($items->contains(function (array $item) use ($olderHosting): bool { | ||
| return $item['metadata']['policy_id'] === $olderHosting->id; | ||
| })); | ||
|
|
||
| $this->assertCount(1, $items->where('metadata.type', 'terms-of-use')); | ||
| $this->assertCount(1, $items->where('metadata.type', 'hosting-policy')); | ||
| } | ||
|
|
||
| public function testMissingPoliciesExcludesAlreadyAcceptedPoliciesForCurrentUser(): void { | ||
| $user = User::factory()->create(); | ||
| $anotherUser = User::factory()->create(); | ||
| $now = CarbonImmutable::now(); | ||
|
|
||
| $terms = $this->createPolicy('terms-of-use', $now->subDays(1), 'terms-of-use/version-2.vue'); | ||
| $hosting = $this->createPolicy('hosting-policy', $now->subDays(1), 'hosting-policy/version-1.vue'); | ||
|
|
||
| PolicyAcceptance::create([ | ||
| 'user_id' => $user->id, | ||
| 'policy_id' => $terms->id, | ||
| 'accepted_at' => $now, | ||
| ]); | ||
|
|
||
| // Acceptance by another user must not affect current user response | ||
| PolicyAcceptance::create([ | ||
| 'user_id' => $anotherUser->id, | ||
| 'policy_id' => $hosting->id, | ||
| 'accepted_at' => $now, | ||
| ]); | ||
|
|
||
| $response = $this->actingAs($user, 'api') | ||
| ->json('GET', $this->missingPoliciesRoute) | ||
| ->assertStatus(200); | ||
|
|
||
| $items = collect($response->json('items')); | ||
|
|
||
| $this->assertCount(1, $items); | ||
| $this->assertSame($hosting->id, $items->first()['metadata']['policy_id']); | ||
| $this->assertSame('hosting-policy', $items->first()['metadata']['type']); | ||
| } | ||
|
|
||
| public function testMissingPoliciesReturnsEmptyListWhenAllCurrentPoliciesAccepted(): void { | ||
| $user = User::factory()->create(); | ||
| $now = CarbonImmutable::now(); | ||
|
|
||
| $terms = $this->createPolicy('terms-of-use', $now->subDays(1), 'terms-of-use/version-2.vue'); | ||
| $hosting = $this->createPolicy('hosting-policy', $now->subDays(1), 'hosting-policy/version-1.vue'); | ||
|
|
||
| PolicyAcceptance::create([ | ||
| 'user_id' => $user->id, | ||
| 'policy_id' => $terms->id, | ||
| 'accepted_at' => $now, | ||
| ]); | ||
|
|
||
| PolicyAcceptance::create([ | ||
| 'user_id' => $user->id, | ||
| 'policy_id' => $hosting->id, | ||
| 'accepted_at' => $now, | ||
| ]); | ||
|
|
||
| $this->actingAs($user, 'api') | ||
| ->json('GET', $this->missingPoliciesRoute) | ||
| ->assertStatus(200) | ||
| ->assertJson(['items' => []]); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we have this block of code twice here, I think it could be nice to have a private function for this query and call the it both in
getCurrentPoliciesandgetMissingPolicies.