feat(events): implementa endpoint de GPS check-in HE4-48#226
Open
GustavoSimao wants to merge 1 commit into
Open
feat(events): implementa endpoint de GPS check-in HE4-48#226GustavoSimao wants to merge 1 commit into
GustavoSimao wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis pull request adds a complete GPS-based event check-in system. It introduces database schema migrations to extend Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app-modules/events/src/Models/EventModel.php (1)
266-277: ⚡ Quick winCast
gps_radiusandxp_baseexplicitly to integers.Those fields are numeric business values; explicit casts prevent type drift in strict runtime paths.
Suggested patch
protected function casts(): array { return [ 'active' => 'boolean', 'event_at' => 'datetime', 'start_at' => 'datetime', 'end_at' => 'datetime', 'event_type' => EventTypeEnum::class, 'status' => EventStatusEnum::class, 'location_lat' => 'float', 'location_lng' => 'float', + 'gps_radius' => 'integer', + 'xp_base' => 'integer', ]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/events/src/Models/EventModel.php` around lines 266 - 277, The casts() method currently returns type casts for event fields but misses explicit integer casts for the numeric business fields; update the array returned by casts() (in the EventModel::casts method) to include 'gps_radius' => 'integer' and 'xp_base' => 'integer' so those attributes are always cast to ints at runtime.tests/Feature/GpsCheckinTest.php (1)
189-234: ⚡ Quick winAssert persistence side effects in the success scenario.
This test currently validates only response payload. Add assertions for persisted pivot values and character XP to protect the critical contract.
Proposed test hardening
it('retorna 200 quando dentro do raio e janela válida', function (): void { @@ $response->assertJsonStructure([ 'state', 'verification_method', 'verified_at', 'xp_awarded', 'streak_multiplier', 'streak_current', ]); + + $pivot = $event->attendees()->where('user_id', $user->id)->first()->pivot; + expect($pivot->state)->toBe(CheckinStatusEnum::Verified); + expect($pivot->xp_awarded)->toBe(100); + expect($pivot->verification_method)->toBe('gps'); + expect($pivot->verified_at)->not->toBeNull(); + + $user->character->refresh(); + expect($user->character->experience)->toBe(100); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Feature/GpsCheckinTest.php` around lines 189 - 234, Add assertions that verify persisted side effects after the successful checkin: assert the event-user pivot (via the EventModel->attendees() pivot for $event and $user) was updated to the verified state and contains verification_method 'gps' and xp_awarded 100 (or the fields your pivot uses), and assert the Character for $user had its XP increased by xp_base (e.g., 100) and any streak fields (streak_current/streak_multiplier) were updated accordingly; use existing symbols EventModel, attendees(), CheckinStatusEnum, Character and the $event/$user fixtures to locate where to insert these DB/persisted assertions after the $response assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app-modules/events/database/migrations/2026_05_13_112452_add_checkin_fields_to_events_table.php`:
- Around line 11-21: The migration defines new columns in the up() method but
lacks a down() method to reverse those changes; add a down() implementation that
uses Schema::table('events', function (Blueprint $table) { ... }) to drop the
added columns (status, location_lat, location_lng, gps_radius, xp_base) so the
migration is reversible—implement the down() method in the same migration class
and call $table->dropColumn([...]) (or individual dropColumn calls) to remove
those fields.
In
`@app-modules/events/database/migrations/2026_05_13_112653_add_checkin_fields_to_events_attendees_table.php`:
- Around line 11-21: The migration adds five nullable columns in up() on the
events_attendees table but lacks a down() to reverse it; add a public function
down(): void that calls Schema::table('events_attendees', function (Blueprint
$table) {
$table->dropColumn(['state','verified_at','verification_method','xp_awarded','streak_multiplier']);
}) so the migration is reversible; reference the existing up() method and the
column names to ensure the down() drops exactly those columns.
In `@app-modules/events/src/Actions/GpsCheckinAction.php`:
- Around line 36-40: GpsCheckinAction assumes event schedule and GPS fields
exist (eventModel->start_at, end_at, radius, latitude, longitude) causing 500s
for nullable schema; before performing time and radius checks in
GpsCheckinAction, guard these nullable fields by validating/presence-checking
eventModel->start_at and eventModel->end_at and the GPS fields (radius,
latitude, longitude) and return a controlled domain error (e.g.,
response()->json(['error'=>'event_incomplete'], 400)) if any required field is
missing; update the conditional blocks that reference start_at/end_at and the
radius/distance calculation to first check for null and short-circuit with the
domain error so downstream ->copy(), ->subMinutes(), and geo math never run on
null.
- Around line 44-50: Wrap the verification/state-check and the subsequent writes
into a database transaction and lock the pivot row to prevent TOCTOU races: in
GpsCheckinAction (where $pivot is checked at the state check around the existing
pivot->state === CheckinStatusEnum::Verified and later updated at the write
block around lines 69-72), start a DB transaction, reload the pivot with a
"select ... for update" (or use the ORM's lockForUpdate on the pivot record),
re-check $pivot->state inside the transaction, and only then perform the state
change, set verified_at and xp_awarded and persist the changes (and commit);
ensure any XP-award side-effects (balance updates, notifications) occur inside
the same transaction or are idempotent/compensated to avoid double-awarding on
concurrent requests or failures.
In `@app-modules/events/src/Http/Requests/GpsCheckinRequest.php`:
- Around line 20-23: The validation currently returns only 'numeric' for lat/lng
in the rules() method of GpsCheckinRequest, which permits impossible
coordinates; update the rules to constrain latitude to -90..90 and longitude to
-180..180 (e.g., add 'between:-90,90' for 'lat' and 'between:-180,180' for
'lng', or use 'min'/'max' equivalents) so invalid GPS values are rejected.
---
Nitpick comments:
In `@app-modules/events/src/Models/EventModel.php`:
- Around line 266-277: The casts() method currently returns type casts for event
fields but misses explicit integer casts for the numeric business fields; update
the array returned by casts() (in the EventModel::casts method) to include
'gps_radius' => 'integer' and 'xp_base' => 'integer' so those attributes are
always cast to ints at runtime.
In `@tests/Feature/GpsCheckinTest.php`:
- Around line 189-234: Add assertions that verify persisted side effects after
the successful checkin: assert the event-user pivot (via the
EventModel->attendees() pivot for $event and $user) was updated to the verified
state and contains verification_method 'gps' and xp_awarded 100 (or the fields
your pivot uses), and assert the Character for $user had its XP increased by
xp_base (e.g., 100) and any streak fields (streak_current/streak_multiplier)
were updated accordingly; use existing symbols EventModel, attendees(),
CheckinStatusEnum, Character and the $event/$user fixtures to locate where to
insert these DB/persisted assertions after the $response assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 0a9d63fe-c9c0-4ee5-8ff9-dcb68de9acd4
📒 Files selected for processing (15)
app-modules/events/database/migrations/2026_05_13_112452_add_checkin_fields_to_events_table.phpapp-modules/events/database/migrations/2026_05_13_112653_add_checkin_fields_to_events_attendees_table.phpapp-modules/events/routes/checkin-routes.phpapp-modules/events/src/Actions/GpsCheckinAction.phpapp-modules/events/src/Actions/HaversineAction.phpapp-modules/events/src/Actions/UpdateCheckinAction.phpapp-modules/events/src/Actions/XpCalculatorAction.phpapp-modules/events/src/Enums/CheckinStatusEnum.phpapp-modules/events/src/Enums/EventStatusEnum.phpapp-modules/events/src/EventsServiceProvider.phpapp-modules/events/src/Http/Controllers/GpsCheckinController.phpapp-modules/events/src/Http/Requests/GpsCheckinRequest.phpapp-modules/events/src/Models/EventModel.phpapp-modules/events/src/Models/Pivot/EventAttend.phptests/Feature/GpsCheckinTest.php
Comment on lines
+11
to
+21
| public function up(): void | ||
| { | ||
| Schema::table('events', function (Blueprint $table): void { | ||
| $table->string('status')->nullable(); | ||
| $table->decimal('location_lat', 10, 7)->nullable(); | ||
| $table->decimal('location_lng', 10, 7)->nullable(); | ||
| $table->integer('gps_radius')->nullable(); | ||
| $table->integer('xp_base')->nullable(); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add a down() rollback for the new columns.
This migration is not reversible right now, which is risky for rollback scenarios.
Suggested patch
return new class extends Migration
{
public function up(): void
{
Schema::table('events', function (Blueprint $table): void {
$table->string('status')->nullable();
$table->decimal('location_lat', 10, 7)->nullable();
$table->decimal('location_lng', 10, 7)->nullable();
$table->integer('gps_radius')->nullable();
$table->integer('xp_base')->nullable();
});
}
+
+ public function down(): void
+ {
+ Schema::table('events', function (Blueprint $table): void {
+ $table->dropColumn([
+ 'status',
+ 'location_lat',
+ 'location_lng',
+ 'gps_radius',
+ 'xp_base',
+ ]);
+ });
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function up(): void | |
| { | |
| Schema::table('events', function (Blueprint $table): void { | |
| $table->string('status')->nullable(); | |
| $table->decimal('location_lat', 10, 7)->nullable(); | |
| $table->decimal('location_lng', 10, 7)->nullable(); | |
| $table->integer('gps_radius')->nullable(); | |
| $table->integer('xp_base')->nullable(); | |
| }); | |
| } | |
| }; | |
| public function up(): void | |
| { | |
| Schema::table('events', function (Blueprint $table): void { | |
| $table->string('status')->nullable(); | |
| $table->decimal('location_lat', 10, 7)->nullable(); | |
| $table->decimal('location_lng', 10, 7)->nullable(); | |
| $table->integer('gps_radius')->nullable(); | |
| $table->integer('xp_base')->nullable(); | |
| }); | |
| } | |
| public function down(): void | |
| { | |
| Schema::table('events', function (Blueprint $table): void { | |
| $table->dropColumn([ | |
| 'status', | |
| 'location_lat', | |
| 'location_lng', | |
| 'gps_radius', | |
| 'xp_base', | |
| ]); | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app-modules/events/database/migrations/2026_05_13_112452_add_checkin_fields_to_events_table.php`
around lines 11 - 21, The migration defines new columns in the up() method but
lacks a down() method to reverse those changes; add a down() implementation that
uses Schema::table('events', function (Blueprint $table) { ... }) to drop the
added columns (status, location_lat, location_lng, gps_radius, xp_base) so the
migration is reversible—implement the down() method in the same migration class
and call $table->dropColumn([...]) (or individual dropColumn calls) to remove
those fields.
Comment on lines
+11
to
+21
| public function up(): void | ||
| { | ||
| Schema::table('events_attendees', function (Blueprint $table): void { | ||
| $table->string('state')->nullable(); | ||
| $table->timestamp('verified_at')->nullable(); | ||
| $table->string('verification_method')->nullable(); | ||
| $table->integer('xp_awarded')->nullable(); | ||
| $table->float('streak_multiplier')->nullable(); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add a down() method to keep the migration reversible.
Without rollback logic, reverting a failed deploy is harder than necessary.
Suggested patch
return new class extends Migration
{
public function up(): void
{
Schema::table('events_attendees', function (Blueprint $table): void {
$table->string('state')->nullable();
$table->timestamp('verified_at')->nullable();
$table->string('verification_method')->nullable();
$table->integer('xp_awarded')->nullable();
$table->float('streak_multiplier')->nullable();
});
}
+
+ public function down(): void
+ {
+ Schema::table('events_attendees', function (Blueprint $table): void {
+ $table->dropColumn([
+ 'state',
+ 'verified_at',
+ 'verification_method',
+ 'xp_awarded',
+ 'streak_multiplier',
+ ]);
+ });
+ }
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app-modules/events/database/migrations/2026_05_13_112653_add_checkin_fields_to_events_attendees_table.php`
around lines 11 - 21, The migration adds five nullable columns in up() on the
events_attendees table but lacks a down() to reverse it; add a public function
down(): void that calls Schema::table('events_attendees', function (Blueprint
$table) {
$table->dropColumn(['state','verified_at','verification_method','xp_awarded','streak_multiplier']);
}) so the migration is reversible; reference the existing up() method and the
column names to ensure the down() drops exactly those columns.
Comment on lines
+36
to
+40
| if (now()->lt($eventModel->start_at->copy()->subMinutes(30))) { | ||
| return response()->json(['error' => 'event_not_started'], 400); | ||
| } | ||
|
|
||
| if (now()->gt($eventModel->end_at->copy()->addMinutes(30))) { |
There was a problem hiding this comment.
Guard nullable event fields before time/radius checks.
Line 36, Line 40, and Lines 55-59 assume event schedule/GPS fields are always present. With nullable schema fields, incomplete event setup can cause 500s instead of a controlled domain error.
Proposed fix
public function execute(EventModel $eventModel, float $latitude, float $longitude): JsonResponse
{
+ if (
+ !$eventModel->start_at
+ || !$eventModel->end_at
+ || $eventModel->location_lat === null
+ || $eventModel->location_lng === null
+ || $eventModel->gps_radius === null
+ ) {
+ return response()->json(['error' => 'event_not_configured_for_checkin'], 400);
+ }
+
$pivot = $eventModel->attendees()->where('user_id', auth()->user()->id)->first()?->pivot;Also applies to: 52-59
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/events/src/Actions/GpsCheckinAction.php` around lines 36 - 40,
GpsCheckinAction assumes event schedule and GPS fields exist
(eventModel->start_at, end_at, radius, latitude, longitude) causing 500s for
nullable schema; before performing time and radius checks in GpsCheckinAction,
guard these nullable fields by validating/presence-checking eventModel->start_at
and eventModel->end_at and the GPS fields (radius, latitude, longitude) and
return a controlled domain error (e.g.,
response()->json(['error'=>'event_incomplete'], 400)) if any required field is
missing; update the conditional blocks that reference start_at/end_at and the
radius/distance calculation to first check for null and short-circuit with the
domain error so downstream ->copy(), ->subMinutes(), and geo math never run on
null.
Comment on lines
+44
to
+50
| if ($pivot->state === CheckinStatusEnum::Verified) { | ||
| return response()->json([ | ||
| 'state' => $pivot->state, | ||
| 'verified_at' => $pivot->verified_at, | ||
| 'xp_awarded' => $pivot->xp_awarded, | ||
| ], 409); | ||
| } |
There was a problem hiding this comment.
Make verification and XP awarding atomic/race-safe.
There is a TOCTOU gap between Line 44 (state check) and Lines 69-72 (writes). Concurrent requests can both pass and award XP twice; failures after pivot update can leave inconsistent data.
Proposed fix
+use Illuminate\Support\Facades\DB;
+
public function execute(EventModel $eventModel, float $latitude, float $longitude): JsonResponse
{
- $pivot = $eventModel->attendees()->where('user_id', auth()->user()->id)->first()?->pivot;
+ $user = auth()->user();
+
+ return DB::transaction(function () use ($eventModel, $latitude, $longitude, $user): JsonResponse {
+ $pivot = $eventModel->attendees()
+ ->where('user_id', $user->id)
+ ->lockForUpdate()
+ ->first()?->pivot;
- if (!$pivot) {
- return response()->json(['error' => 'not_registered'], 403);
- }
+ if (!$pivot) {
+ return response()->json(['error' => 'not_registered'], 403);
+ }
- if ($pivot->state === CheckinStatusEnum::Verified) {
- return response()->json([
- 'state' => $pivot->state,
- 'verified_at' => $pivot->verified_at,
- 'xp_awarded' => $pivot->xp_awarded,
- ], 409);
- }
+ if ($pivot->state === CheckinStatusEnum::Verified) {
+ return response()->json([
+ 'state' => $pivot->state,
+ 'verified_at' => $pivot->verified_at,
+ 'xp_awarded' => $pivot->xp_awarded,
+ ], 409);
+ }
- $xpAwarded = $this->xpCalculatorAction->execute($eventModel);
- $this->updateCheckinAction->execute($pivot, $xpAwarded);
- auth()->user()->character->increment('experience', $xpAwarded);
+ $xpAwarded = $this->xpCalculatorAction->execute($eventModel);
+ $this->updateCheckinAction->execute($pivot, $xpAwarded);
+
+ if (!$user->character) {
+ return response()->json(['error' => 'character_not_found'], 409);
+ }
+ $user->character->increment('experience', $xpAwarded);
- return response()->json([
- // ...
- ]);
+ return response()->json([
+ // ...
+ ]);
+ });
}Also applies to: 67-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/events/src/Actions/GpsCheckinAction.php` around lines 44 - 50,
Wrap the verification/state-check and the subsequent writes into a database
transaction and lock the pivot row to prevent TOCTOU races: in GpsCheckinAction
(where $pivot is checked at the state check around the existing pivot->state ===
CheckinStatusEnum::Verified and later updated at the write block around lines
69-72), start a DB transaction, reload the pivot with a "select ... for update"
(or use the ORM's lockForUpdate on the pivot record), re-check $pivot->state
inside the transaction, and only then perform the state change, set verified_at
and xp_awarded and persist the changes (and commit); ensure any XP-award
side-effects (balance updates, notifications) occur inside the same transaction
or are idempotent/compensated to avoid double-awarding on concurrent requests or
failures.
Comment on lines
+20
to
+23
| return [ | ||
| 'lat' => ['required', 'numeric'], | ||
| 'lng' => ['required', 'numeric'], | ||
| ]; |
There was a problem hiding this comment.
Enforce geographic bounds for latitude/longitude.
numeric allows invalid coordinates; add range checks to reject impossible GPS values.
Suggested patch
public function rules(): array
{
return [
- 'lat' => ['required', 'numeric'],
- 'lng' => ['required', 'numeric'],
+ 'lat' => ['required', 'numeric', 'between:-90,90'],
+ 'lng' => ['required', 'numeric', 'between:-180,180'],
];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return [ | |
| 'lat' => ['required', 'numeric'], | |
| 'lng' => ['required', 'numeric'], | |
| ]; | |
| return [ | |
| 'lat' => ['required', 'numeric', 'between:-90,90'], | |
| 'lng' => ['required', 'numeric', 'between:-180,180'], | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-modules/events/src/Http/Requests/GpsCheckinRequest.php` around lines 20 -
23, The validation currently returns only 'numeric' for lat/lng in the rules()
method of GpsCheckinRequest, which permits impossible coordinates; update the
rules to constrain latitude to -90..90 and longitude to -180..180 (e.g., add
'between:-90,90' for 'lat' and 'between:-180,180' for 'lng', or use 'min'/'max'
equivalents) so invalid GPS values are rejected.
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.
Description
Implements a GPS-based check-in endpoint for event attendance verification with distance validation using the Haversine formula and XP award integration. The feature validates check-in timing windows (30 minutes around event time), prevents duplicate verifications, calculates rewards based on event configuration, and tracks verification metadata per attendee.
References
Dependencies & Requirements
No new external dependencies added. The implementation uses existing Laravel framework components. Requirements:
status,location_lat,location_lng,gps_radius, andxp_basefieldsContributor Summary
Changes Summary
Scheduledevent statusVerifiedcheck-in statusPOST /api/events/{event}/checkinwith auth:sanctum middlewareNotes: Streak multiplier functionality (HE4-47) is pending; related values are fixed at 1 and 0 respectively, marked with TODO comments in the codebase.