-
Notifications
You must be signed in to change notification settings - Fork 3
Create endpoint to get polices that should be accepted #1198
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
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 |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
|
|
||
| namespace App\Http\Controllers; | ||
|
|
||
| use App\Http\Resources\PoliciesCollection; | ||
| use App\Policy; | ||
| use Carbon\CarbonImmutable; | ||
|
|
||
| class PoliciesController extends Controller { | ||
| public function getCurrentPolicies() { | ||
|
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. Consider adding a |
||
| $now = CarbonImmutable::now(); | ||
|
|
||
| // This works based on the assumption that the latest policy has the highest id given that id is AUTO_INCREMENT | ||
| $latestPolicyIds = Policy::where('active_from', '<', $now) | ||
|
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. As I understand, query 1 builds a list of IDs with group-by + max and then |
||
| ->selectRaw('MAX(id) as id') | ||
| ->groupBy('policy_type') | ||
| ->pluck('id'); | ||
|
|
||
| $currentPolicies = Policy::whereIn('id', $latestPolicyIds)->get(); | ||
|
|
||
| return new PoliciesCollection($currentPolicies); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| <?php | ||
|
|
||
| namespace App\Http\Resources; | ||
|
|
||
| use Illuminate\Http\Request; | ||
| use Illuminate\Http\Resources\Json\ResourceCollection; | ||
|
|
||
| class PoliciesCollection extends ResourceCollection { | ||
| /** | ||
| * Transform the resource collection into an array. | ||
| * | ||
| * @return array<int|string, mixed> | ||
| */ | ||
| public function toArray(Request $request): array { | ||
| return [ | ||
| 'items' => $this->collection, | ||
|
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. the key Not a functionality issue but coding style |
||
| ]; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?php | ||
|
|
||
| namespace Database\Factories; | ||
|
|
||
| use App\Policy; | ||
| use Illuminate\Database\Eloquent\Factories\Factory; | ||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| /** | ||
| * @extends Factory<Model> | ||
| */ | ||
| class PolicyFactory extends Factory { | ||
| protected $model = Policy::class; | ||
|
|
||
| /** | ||
| * Define the model's default state. | ||
| * | ||
| * @return array<string, mixed> | ||
| */ | ||
| public function definition(): array { | ||
| return [ | ||
| 'policy_type' => $this->faker->randomElement(['terms-of-use', 'hosting-policy']), | ||
| 'active_from' => now(), | ||
| 'content_vue_file' => fake()->slug() . '.vue', | ||
| ]; | ||
| } | ||
| } |
|
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. a few suggestions for extra test coverage:
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?php | ||
|
|
||
| namespace Tests\Http\Controllers; | ||
|
|
||
| use App\Policy; | ||
| use Illuminate\Foundation\Testing\DatabaseTransactions; | ||
| use Tests\TestCase; | ||
|
|
||
| class PoliciesControllerTest extends TestCase { | ||
| use DatabaseTransactions; | ||
|
|
||
| public function testGetCurrentPolicies(): void { | ||
| // Future policy | ||
| Policy::factory()->create([ | ||
| 'active_from' => now()->addDay(), | ||
|
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. real-time |
||
| ]); | ||
| // Active policy | ||
| Policy::factory()->create([ | ||
| 'active_from' => now()->subMonth(), | ||
| ]); | ||
|
|
||
| $response = $this->getJson('/policies/current'); | ||
|
|
||
| $response->assertOk(); | ||
| $response->assertJsonCount(1, 'data.items'); | ||
|
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. this test only asserts the count, so it can pass even if the endpoint returns the wrong policy. |
||
| } | ||
| } | ||
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.
consider adding PHPDoc for this controller