chore: events module enhancements#140
Conversation
|
Warning Rate limit exceeded@danielhe4rt has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds polymorphic scheduling: new EventAgenda, EventSubmission (renamed from Talk), EventSegment, and EventSubmissionSpeaker pivot; updates migrations, factories, seeders, models, Filament resources, Blade components, tests, and a Sushi dependency to support agenda-driven event scheduling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas to focus on:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app-modules/events/src/Models/EventModel.php (1)
83-101: Consider transaction wrapping for race condition safety.The
attend()method increments counters, refreshes the model, then attaches the user withattend_ordercalculated from the refreshed counts. Between the increment (lines 89-93) and the attach (lines 97-100), another concurrent request could modify the counts, leading to duplicateattend_ordervalues or incorrect sequencing.Wrap the entire operation in a database transaction:
public function attend(mixed $userId, AttendingStatusEnum $status = AttendingStatusEnum::Attending): void { if ($this->isParticipating($userId)) { return; } + DB::transaction(function () use ($userId, $status) { match ($status) { AttendingStatusEnum::Attending => $this->increment('attendees_count'), AttendingStatusEnum::Waitlist => $this->increment('waitlist_count'), default => throw new Exception('Unexpected match value'), }; $this->refresh(); $this->attendees()->attach($userId, [ 'status' => $status, 'attend_order' => $this->attendees_count + $this->waitlist_count, ]); + }); }Also add the import at the top of the file:
+use Illuminate\Support\Facades\DB;app-modules/he4rt/resources/views/components/schedule-card.blade.php (1)
13-29: Unreachable match arm:$startsAt->isFuture()will never execute.In PHP
match, the first matching arm wins. Sincedefault(lines 19-23) matches everything and appears before$startsAt->isFuture()(lines 24-28), the "Em breve" state is unreachable. The status will show "Em andamento" instead of "Em breve" for future events.Reorder the match arms so
defaultis last:$config = match (true) { $endsAt->isPast() => [ 'color' => 'text-green-300', 'text' => 'Finalizado', 'icon' => 'heroicon-s-check-circle', ], - default => [ - 'color' => 'text-orange-300', - 'text' => 'Em andamento', - 'icon' => 'heroicon-s-clock', - ], $startsAt->isFuture() => [ 'color' => 'text-blue-300', 'text' => 'Em breve', 'icon' => 'heroicon-o-calendar', ], + default => [ + 'color' => 'text-orange-300', + 'text' => 'Em andamento', + 'icon' => 'heroicon-s-clock', + ], };
🧹 Nitpick comments (12)
app-modules/events/database/migrations/2025_11_27_132714_add_attend_order_to_events_attendees_table.php (1)
14-21: Consider adding a down() method for rollback support.While the migration correctly adds the
attend_ordercolumn, it lacks adown()method to remove the column during rollback. Although this is consistent with other migrations in the project, adding rollback support improves migration safety during development.Add this method to enable rollback:
/** * Reverse the migrations. */ public function down(): void { Schema::table('events_attendees', function (Blueprint $table): void { $table->dropColumn('attend_order'); }); }app-modules/events/src/Models/Pivot/EventAttend.php (1)
19-24: Consider adding an explicit cast for attend_order.While Laravel infers integer types from the database schema, explicitly casting
attend_orderto'integer'in thecasts()method improves type safety and makes the model's contract clearer.protected function casts(): array { return [ 'status' => AttendingStatusEnum::class, + 'attend_order' => 'integer', ]; }app-modules/events/src/Enums/SchedulableTypeEnum.php (1)
19-25: Consider a more specific return type for getIcon().While
BackedEnumis technically correct, using a more specific return type likeHeroiconwould improve type safety and make the API clearer.-public function getIcon(): BackedEnum +public function getIcon(): Heroicon { return match ($this) { self::Talk => Heroicon::Microphone, self::Segment => Heroicon::Bars2, }; }app-modules/he4rt/resources/views/components/ticket.blade.php (1)
5-5: Consider removing the unused githubUsername prop.The
githubUsernameprop is still declared but is no longer used in the template after Line 31 was changed to use$name. If this prop isn't needed for backward compatibility, consider removing it to keep the component interface clean.@props([ 'userImg' => '', 'username', 'name', - 'githubUsername', 'ticketNumber', 'githubLink' => '#',app-modules/events/src/Filament/Events/ParticipantDashboard.php (2)
56-60: Consider adding authentication checks.The
eventAttend()method assumesauth()->id()is never null, but if called when unauthenticated (despite navigation guards), it could cause issues. Consider adding an early guard or type-checking.Apply this diff to add a guard:
public function eventAttend(): void { - + abort_unless(auth()->check(), 401); + $this->event->attend(auth()->id(), AttendingStatusEnum::Attending); }
62-68: Potential optimization for participant retrieval.The current query on Line 66 fetches the participant by filtering the attendees relation and calling
first(). Since the relationship is already loaded on$this->event, you could optimize by using the relationship methods directly or leveraging eager loading to avoid additional queries.Consider this alternative:
protected function getViewData(): array { return [ 'event' => $this->event, - 'participant' => $this->event->attendees()->where('user_id', auth()->id())->first(), + 'participant' => $this->event->attendees->firstWhere('id', auth()->id()), ]; }app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/CreateEventAgenda.php (1)
14-19: Remove unnecessary blank line and consider simplifying.The empty array on lines 16-18 contains an unnecessary blank line (Line 17). Additionally, if the default behavior of
CreateRecordis to have no header actions, this entire method override may be unnecessary.Apply this diff to remove the blank line:
protected function getHeaderActions(): array { return [ - ]; }Or remove the entire method if the default is already an empty array:
class CreateEventAgenda extends CreateRecord { protected static string $resource = EventAgendaResource::class; - - protected function getHeaderActions(): array - { - return [ - - ]; - } }app-modules/events/database/factories/EventAgendaFactory.php (1)
34-48: Add return type hints for factory state methods.Both
forSegment()andforTalk()return$this->state(...)but lack explicit return types.-public function forSegment(?EventSegment $segment = null) +public function forSegment(?EventSegment $segment = null): static { return $this->state(fn () => [ 'schedulable_type' => EventSegment::class, 'schedulable_id' => $segment ?? EventSegment::query()->inRandomOrder()->first()->getKey(), ]); } -public function forTalk(?Talk $talk = null) +public function forTalk(?Talk $talk = null): static { return $this->state(fn () => [ 'schedulable_type' => Talk::class, 'schedulable_id' => $talk ?? Talk::factory(), ]); }app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php (1)
100-101: Inconsistent prop binding style.Line 100 uses Blade interpolation
{{...}}inside quotes while line 101 uses the:propsyntax. Both work, but consistency is preferred.- githubLink="https://github.com/{{auth()->user()->username}}" + :githubLink="'https://github.com/' . auth()->user()->username"app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (3)
46-87: Checkform()signature vs your base Resource and tighten schedule validationTwo things to double‑check here:
Method signature:
public static function form(Schema $schema): Schemadiffers from the canonical FilamentResource::form(Form $form): Form. If you’re still extending the stockFilament\Resources\Resource, make sure your local base class actually expects thisSchemasignature, otherwise PHP will complain about incompatible method declarations and Filament won’t be able to call the form properly.Time fields and ordering: If
starting_at/ending_atare stored asdatetimecolumns (as suggested in the summary), you may want to:
- use a
DateTimePickeror otherwise ensure the stored value includes a date, and- add a rule to guarantee chronological order, e.g.:
TimePicker::make('ending_at') ->native(false) ->required() ->rule('after:starting_at');This avoids accidentally saving an agenda item whose end time is before the start.
89-125: Improve schedulable & time columns for readabilityFunctionally the table works, but UX can be improved:
schedulable_typeandschedulable_idwill render as a fully‑qualified class name and raw id, which isn’t very meaningful to admins. Since bothTalkandEventSegmentexpose atitle, consider something like:TextColumn::make('schedulable.title') ->label('Item') ->searchable();and, if you need the type, either:
a formatted type column (e.g. via your
SchedulableTypeEnum), or
->formatStateUsing(fn (string $state) => class_basename($state))onschedulable_type.For
starting_at/ending_at, adding explicit formatting (e.g.->time('H:i')or->dateTime('H:i')) will keep the list compact and consistent instead of dumping raw DB strings.These are all presentation‑only tweaks but significantly help readability when agendas grow.
144-167: Global search configuration is solid; consider including the schedulable titleEager‑loading
tenantandevent, restricting searchable attributes totenant.nameandevent.title, and returning both ingetGlobalSearchResultDetails()is well‑structured.If you want search to also hit the actual agenda item, you could add
schedulable.title(and optionally a formatted schedulable type) both togetGloballySearchableAttributes()and the details array, subject to how yourSchedulableTypeEnumand morph relation are set up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
app-modules/events/database/factories/EventAgendaFactory.php(1 hunks)app-modules/events/database/factories/TalkFactory.php(0 hunks)app-modules/events/database/migrations/2025_11_05_192756_create_events_attendees_table.php(0 hunks)app-modules/events/database/migrations/2025_11_27_132714_add_attend_order_to_events_attendees_table.php(1 hunks)app-modules/events/database/migrations/2025_11_27_145728_create_events_agenda_table.php(1 hunks)app-modules/events/database/migrations/2025_11_27_153537_remove_starts_at_ends_at_columns_to_events_talks_table.php(1 hunks)app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php(3 hunks)app-modules/events/resources/views/components/themes/3pontos/components/sections/schedule.blade.php(1 hunks)app-modules/events/resources/views/components/themes/3pontos/participant-dashboard.blade.php(1 hunks)app-modules/events/src/AdminEventPanelPlugin.php(2 hunks)app-modules/events/src/Enums/SchedulableTypeEnum.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/CreateEventAgenda.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/EditEventAgenda.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/ListEventAgendas.php(1 hunks)app-modules/events/src/Filament/Events/ParticipantDashboard.php(3 hunks)app-modules/events/src/Models/EventAgenda.php(1 hunks)app-modules/events/src/Models/EventModel.php(3 hunks)app-modules/events/src/Models/EventSegment.php(1 hunks)app-modules/events/src/Models/Pivot/EventAttend.php(1 hunks)app-modules/events/src/Models/Talk.php(0 hunks)app-modules/he4rt/resources/views/components/schedule-card.blade.php(3 hunks)app-modules/he4rt/resources/views/components/ticket.blade.php(2 hunks)app-modules/user/src/Models/User.php(2 hunks)composer.json(1 hunks)database/seeders/ThreeDotsSeeder.php(3 hunks)
💤 Files with no reviewable changes (3)
- app-modules/events/database/migrations/2025_11_05_192756_create_events_attendees_table.php
- app-modules/events/database/factories/TalkFactory.php
- app-modules/events/src/Models/Talk.php
🧰 Additional context used
🧬 Code graph analysis (8)
app-modules/events/database/migrations/2025_11_27_145728_create_events_agenda_table.php (2)
app-modules/tenant/src/Models/Tenant.php (1)
Tenant(23-104)app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (1)
table(89-125)
app-modules/events/src/AdminEventPanelPlugin.php (1)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (1)
EventAgendaResource(38-168)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/ListEventAgendas.php (3)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (1)
EventAgendaResource(38-168)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/CreateEventAgenda.php (1)
getHeaderActions(14-19)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/EditEventAgenda.php (1)
getHeaderActions(17-24)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/CreateEventAgenda.php (3)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (1)
EventAgendaResource(38-168)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/EditEventAgenda.php (1)
getHeaderActions(17-24)app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/ListEventAgendas.php (1)
getHeaderActions(15-20)
app-modules/events/database/migrations/2025_11_27_132714_add_attend_order_to_events_attendees_table.php (1)
app-modules/events/database/migrations/2025_11_05_192756_create_events_attendees_table.php (1)
up(14-22)
app-modules/events/src/Filament/Events/ParticipantDashboard.php (4)
app-modules/tenant/src/Models/Tenant.php (2)
Tenant(23-104)events(72-75)app-modules/events/src/Models/EventModel.php (3)
tenant(145-148)attend(83-101)attendees(69-81)app-modules/user/src/Models/User.php (1)
events(72-84)app-modules/events/src/Filament/Events/EventLandingPage.php (1)
getViewData(47-52)
app-modules/events/database/factories/EventAgendaFactory.php (3)
app-modules/events/src/Models/EventSegment.php (1)
EventSegment(10-28)app-modules/tenant/src/Models/Tenant.php (1)
Tenant(23-104)app-modules/shared/src/Factory.php (1)
Factory(9-12)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (3)
app-modules/events/src/Models/EventSegment.php (1)
EventSegment(10-28)app-modules/events/src/Models/EventAgenda.php (2)
tenant(37-40)event(45-48)app-modules/events/src/Models/EventModel.php (1)
tenant(145-148)
🪛 GitHub Check: Perform Phpstan Check / Run
app-modules/user/src/Models/User.php
[failure] 146-146:
Access to an undefined property He4rt\User\Models\User::$name.
app-modules/events/src/Models/EventAgenda.php
[failure] 57-57:
Access to an undefined property He4rt\Events\Models\EventAgenda::$schedulable_type.
🪛 PHPMD (2.15.0)
database/seeders/ThreeDotsSeeder.php
90-90: Avoid unused local variables such as '$segments'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (17)
app-modules/events/database/migrations/2025_11_27_153537_remove_starts_at_ends_at_columns_to_events_talks_table.php (2)
14-20: Up migration correctly removes talk scheduling columnsDropping
starts_atandends_atfromevents_talksis consistent with moving scheduling to the new agenda structure and the migration filename. No issues from a schema-change perspective.
25-31: I'll help you verify the review comment about this migration file. Let me start by examining the complete migration file and related schema information.
<function_calls>
Read the full migration file to see both up() and down() methods
#!/bin/bashRead the migration file
cat -n "app-modules/events/database/migrations/2025_11_27_153537_remove_starts_at_ends_at_columns_to_events_talks_table.php"
</function_calls>
<function_calls>
Search for other migrations related to events_talks table to understand the schema
#!/bin/bashFind all migration files in the events module
find . -path "/events/database/migrations/.php" -type f | head -20
</function_calls>
<function_calls>
Search for migrations that created or modified events_talks table
#!/bin/bashSearch for migrations mentioning events_talks
rg -l "events_talks" "app-modules/events/database/migrations/" 2>/dev/null | head -10
</function_calls>app-modules/events/src/Models/Pivot/EventAttend.php (1)
14-17: LGTM! attend_order added to fillable.The addition of
attend_orderto the fillable array correctly enables mass assignment for the new pivot field, aligning with the database migration.app-modules/events/src/AdminEventPanelPlugin.php (1)
10-10: LGTM! EventAgendaResource properly registered.The new resource is correctly imported and registered in the panel's resources array, enabling Filament admin management for EventAgenda models.
Also applies to: 26-26
app-modules/events/src/Enums/SchedulableTypeEnum.php (1)
13-26: Well-structured enum with proper icon mapping.The enum correctly maps schedulable types to their model classes and provides appropriate icons for each case. The implementation of
HasIconis correct.app-modules/he4rt/resources/views/components/ticket.blade.php (1)
4-4: Good improvement using the name prop.Replacing
githubUsernamewithnamein the display provides a more appropriate user-friendly name instead of the GitHub handle.Also applies to: 31-31
app-modules/events/resources/views/components/themes/3pontos/participant-dashboard.blade.php (1)
2-2: Variables $participant and $event are correctly provided to the view.The
ParticipantDashboardFilament Page class explicitly passes both variables via thegetViewData()method (lines 62-68):
$eventis set duringmount()from the tenant's active event$participantis retrieved as the current user's attendee record from the eventBoth are available in the view context as expected.
composer.json (1)
12-12: Sushi package version 2.5.3 is verified to exist and is free from known vulnerabilities.Version 2.5.3 is a stable release published on February 13, 2025, with the latest commit adding Laravel 12 support. The package is actively maintained (2,976 stars, 185 forks) with no known security vulnerabilities in Packagist, GitHub Security Advisories, or Composer's advisory database. The
^2.5.3version constraint is appropriate for this well-maintained dependency.app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/EditEventAgenda.php (1)
1-25: LGTM!The EditEventAgenda page follows standard Filament patterns and includes appropriate header actions for managing EventAgenda records.
app-modules/events/src/Models/EventModel.php (1)
166-173: LGTM!The
agenda()relation is correctly defined with appropriate ordering bystarting_at. This provides a clean way to retrieve scheduled agenda items for events.app-modules/events/src/Filament/Admin/Resources/EventAgenda/Pages/ListEventAgendas.php (1)
1-21: LGTM!The ListEventAgendas page follows standard Filament patterns with appropriate header actions for creating new EventAgenda records.
app-modules/events/database/migrations/2025_11_27_145728_create_events_agenda_table.php (1)
19-22: I'll verify the column type choices in the migration file by examining the actual code and related models.
<function_calls>
#!/bin/bashFind and read the migration file
fd -t f -n "2025_11_27_145728_create_events_agenda_table.php" | head -1 | xargs -I {} cat -n "{}"
</function_calls>Now let me check the EventAgenda model to verify the casts:
<function_calls>
#!/bin/bashFind EventAgenda model
fd -t f -name "EventAgenda.php" | xargs -I {} cat -n "{}"
</function_calls>Let me search for EventAgenda model and related usage patterns:
<function_calls>
#!/bin/bashSearch for EventAgenda class definition
rg -n "class EventAgenda" --type php -A 50 | head -80
</function_calls>Let me also check if the project uses UUIDs or standard integer IDs:
<function_calls>
#!/bin/bashCheck for UUID usage patterns
rg -n "uuid|UUID" --type php -B 2 -A 2 | head -40
</function_calls>Let me search more broadly for the events_agenda table structure and any related code:
<function_calls>
#!/bin/bashSearch for events_agenda references
rg -n "events_agenda|schedulable_id|starting_at" --type php -B 2 -A 2 | head -100
</function_calls>Let me check what migrations exist in the events database folder:
<function_calls>
#!/bin/bashList migrations in events module
fd -t f -e php ".migrations." | grep -i events | sort
</function_calls><function_calls>
#!/bin/bashFind the exact migration path and read it
find . -name "2025_11_27_145728" -type f 2>/dev/null
</function_calls><function_calls>
#!/bin/bashSearch for the file in app-modules
find app-modules -name "create_events_agenda" -type f 2>/dev/null
</function_calls><function_calls>
#!/bin/bashList all files in the events database migrations directory
ls -la app-modules/events/database/migrations/ 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashRead the migration file directly
cat -n "app-modules/events/database/migrations/2025_11_27_145728_create_events_agenda_table.php"
</function_calls><function_calls>
#!/bin/bashFind EventAgenda model
find . -name "EventAgenda.php" -type f 2>/dev/null
</function_calls><function_calls>
#!/bin/bashSearch for EventAgenda definition
rg -n "class EventAgenda" --type php -A 100 2>/dev/null | head -120
</function_calls><function_calls>
#!/bin/bashCheck the structure of app-modules
ls -la app-modules/ 2>/dev/null
</function_calls><function_calls>
#!/bin/bashList all migration files
find . -path "/migrations/.php" -type f | sort | tail -20
</function_calls><function_calls>
#!/bin/bashGet repository structure
git ls-files | grep -E "(migrations|models)" | grep -i events | head -20
</function_calls><function_calls>
#!/bin/bashCheck if repo has been cloned and list root structure
pwd
ls -la | head -20
</function_calls>Based on my verification attempts, I need to gather more information about the codebase structure and content. Let me search for polymorphic relation patterns and datetime handling:
<function_calls>
#!/bin/bashSearch for polymorphic relationships and MorphsTo patterns
rg -n "morphsTo|morphMany|morphOne" --type php -B 3 -A 3 | head -60
</function_calls><function_calls>
#!/bin/bashSearch for datetime casting patterns in models
rg -n "protected $casts|'.'=>.datetime|'.'=>.'datetime" --type php | head -40
</function_calls><function_calls>
#!/bin/bashCheck Laravel conventions - search for standard polymorphic ID patterns
rg -n "unsignedBigInteger.*polymorphic|polymorphic.*unsignedBigInteger" --type php -B 2 -A 2 | head -40
</function_calls><function_calls>
#!/bin/bashSearch for dateTime vs string column type usage patterns in migrations
rg -n "->dateTime(|->string(" --type php | grep -E "(starting|ending|_at)" | head -30
</function_calls>Let me try a broader search for the specific migration and model definitions:
<function_calls>
#!/bin/bashSearch for all content matching the review snippet
rg -n "schedulable_type|schedulable_id" --type php -B 5 -A 5 | head -100
</function_calls>Let me check the git log for recent changes related to events_agenda:
<function_calls>
#!/bin/bashCheck git history for events_agenda
git log --oneline --all -- "events_agenda" 2>/dev/null | head -10
</function_calls>Let me examine the current branch and files:
<function_calls>
#!/bin/bashCheck current branch and recent changes
git status 2>/dev/null
echo "---"
git diff --stat HEAD 2>/dev/null | head -30
</function_calls><function_calls>
#!/bin/bashList all modified files in the PR/branch
git diff --name-only HEAD~1 2>/dev/null | grep -i events
</function_calls><function_calls>
#!/bin/bashShow the content of migration files in the diff
git show HEAD:app-modules/events/database/migrations/2025_11_27_145728_create_events_agenda_table.php 2>/dev/null || echo "File not found in HEAD"
</function_calls><function_calls>
#!/bin/bashTry to find any PHP file with the events_agenda schema
rg -l "events_agenda" --type php 2>/dev/null
</function_calls><function_calls>
#!/bin/bashDisplay all files in the working directory that might relate to events and agenda
find . -type f -name "*.php" 2>/dev/null | grep -E "(event|agenda)" | head -30
</function_calls>Based on my extensive verification attempts, I cannot access the actual migration and model files in the codebase. The repository structure or files may not be available in the sandbox environment. However, I can provide verification based on Laravel best practices and the concerns raised in the original review:
The original review raises valid concerns about non-standard column types. Let me verify these concerns against Laravel conventions by checking if similar patterns exist elsewhere:
<function_calls>
#!/bin/bashCheck general Laravel migration patterns in the codebase
rg -n "Schema::create" --type php -A 15 | head -80
</function_calls>Based on my verification efforts, I cannot conclusively determine the intent behind the column type choices because the actual file contents are not accessible in the sandbox environment. However, the original review's concerns are valid according to Laravel conventions:
schedulable_idasstring: In standard Laravel polymorphic relations, the ID column isunsignedBigInteger. Usingstringis non-standard unless the project specifically uses UUIDs or other string-based identifiers.
starting_atandending_atasstring: For timestamp/datetime data, Laravel conventions usedateTime,timestamp, ortimecolumn types, notstring. String columns cannot benefit from database-level sorting, comparison, and timezone handling.The review comment's observations appear sound based on Laravel best practices. Without access to confirm whether this project uses UUIDs, has special casts for string dates, or has other special requirements, the verification cannot be completed with absolute certainty.
app-modules/events/src/Models/EventSegment.php (1)
10-28: LGTM!Using Sushi for static segment data is a clean approach for this lookup-style model. The implementation is straightforward and integrates well with the polymorphic schedulable pattern.
app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php (1)
67-86: Duplicate "Área do Participante" sections with same messaging.When a participant exists, both the new conditional block (lines 27-42) and this section (lines 68-85) display "Parabéns! Sua vaga está garantida!" with identical section titles. This creates redundant UI.
Is this duplication intentional? Consider consolidating these sections or differentiating their purposes.
app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (3)
7-27: Verify namespaces for Filament table actionsYou’re importing all actions from
Filament\Actions\*, but table record/bulk actions are typically underFilament\Tables\Actions\*(or accessed viaTables\Actions\...). If you see class‑not‑found or unexpected behavior on the table, double‑check that these are the correct classes for your Filament version and, if needed, switch to theFilament\Tables\Actionsvariants or aTables\Actionsalias.
127-134: Page registrations look consistentIndex, create, and edit routes are wired in the expected way to their page classes; nothing blocking here.
136-142: Soft‑delete query override matches Filament’s patternOverriding
getEloquentQuery()to removeSoftDeletingScopewhile pairing it withTrashedFilterin the table is the standard Filament approach for exposing trashed records. This looks good.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app-modules/he4rt/resources/views/components/schedule-card.blade.php (1)
10-29: I'm unable to clone the repository directly, but I can verify the PHP logic described in the review comment through language analysis. Let me assess the technical correctness of the issue.Based on the review comment provided, I can analyze the PHP
match(true)construct:PHP
match(true)behavior verification:In PHP,
match()is exhaustive and returns the value of the first matching arm. When usingmatch(true):
Current order (problematic):
- Checks
$endsAt->isPast()— matches if true- Falls through to
default— always matches (catches all remaining cases)- Never reaches
$startsAt->isFuture()checkProposed order (correct):
- Checks
$endsAt->isPast()— matches if true- Checks
$startsAt->isFuture()— matches if true- Falls through to
default— catches remaining cases (ongoing events)The logic is sound: An event that hasn't ended yet should be checked for whether it hasn't started (future) before being classified as ongoing.
Verification: The review comment's analysis is technically correct. The unreachable branch issue is real, and the proposed reordering would fix it.
match(true)arm ordering makes the "Em breve" state unreachableWith the current
match (true), thedefaultarm will always be selected whenever$endsAt->isPast()is false, so the$startsAt->isFuture()arm is never evaluated. Future events are incorrectly shown as "Em andamento" instead of "Em breve".Reorder the arms to evaluate conditions before the catch-all:
- $config = match (true) { - $endsAt->isPast() => [ - 'color' => 'text-green-300', - 'text' => 'Finalizado', - 'icon' => 'heroicon-s-check-circle', - ], - default => [ - 'color' => 'text-orange-300', - 'text' => 'Em andamento', - 'icon' => 'heroicon-s-clock', - ], - $startsAt->isFuture() => [ - 'color' => 'text-blue-300', - 'text' => 'Em breve', - 'icon' => 'heroicon-o-calendar', - ], - }; + $config = match (true) { + $endsAt->isPast() => [ + 'color' => 'text-green-300', + 'text' => 'Finalizado', + 'icon' => 'heroicon-s-check-circle', + ], + $startsAt->isFuture() => [ + 'color' => 'text-blue-300', + 'text' => 'Em breve', + 'icon' => 'heroicon-o-calendar', + ], + default => [ + 'color' => 'text-orange-300', + 'text' => 'Em andamento', + 'icon' => 'heroicon-s-clock', + ], + };
♻️ Duplicate comments (3)
app-modules/events/database/factories/EventAgendaFactory.php (1)
34-48: Use IDs (and guard against null) forschedulable_idinforSegment()/forTalk().Both helpers currently risk invalid values for
schedulable_id:
forSegment()can:
- Pass an
EventSegmentmodel instance directly intoschedulable_id($segment ?? ...), which should be an integer or Factory, not a Model.- Call
first()->getKey()without checking fornull, which will fatally error if noEventSegmentrows exist.
forTalk()similarly passes anEventSubmissionmodel when$talkis provided, instead of its key.A safer pattern:
public function forSegment(?EventSegment $segment = null) { - return $this->state(fn () => [ - 'schedulable_type' => EventSegment::class, - 'schedulable_id' => $segment ?? EventSegment::query()->inRandomOrder()->first()->getKey(), - ]); + return $this->state(function () use ($segment) { + $schedulableId = $segment?->getKey() + ?? EventSegment::query()->inRandomOrder()->firstOrFail()->getKey(); + + return [ + 'schedulable_type' => EventSegment::class, + 'schedulable_id' => $schedulableId, + ]; + }); } public function forTalk(?EventSubmission $talk = null) { - return $this->state(fn () => [ - 'schedulable_type' => EventSubmission::class, - 'schedulable_id' => $talk ?? EventSubmission::factory(), - ]); + return $this->state(function () use ($talk) { + return [ + 'schedulable_type' => EventSubmission::class, + 'schedulable_id' => $talk?->getKey() ?? EventSubmission::factory(), + ]; + }); }app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php (1)
1-4: Fix swapped/mismatched@varannotations on props.The inline
@varcomments are reversed and reference the wrong variable names ($participanton theeventline and$eventon theparticipantline). This is confusing for static analysis and IDEs.Consider rewriting to match actual usage:
@props([ // $event->day, $event->start 'event'/** @var \He4rt\Events\Models\EventModel $event */, // $participant?->pivot->attend_order 'participant'/** @var mixed $participant */, {{-- e.g. User with EventAttend pivot --}} ])(Adjust the
participanttype to the concrete model you actually pass in.)app-modules/user/src/Models/User.php (1)
148-159: Address edge cases in the shortName accessor.The
shortName()accessor doesn't handle single-word or empty names properly, which was already flagged in previous review comments.Based on past review feedback, the implementation should handle:
- Single-word names (where
pop()returnsnull)- Empty names
- The static analysis warning about undefined
$namepropertyPlease refer to the previous review comments for the recommended solution.
🧹 Nitpick comments (10)
app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php (3)
60-66: Consider making the schedulable field required.The
MorphToSelectfor schedulable is not marked as required. If EventAgenda records must have a schedulable relationship, add->required()for data integrity.MorphToSelect::make('schedulable') ->types([ Type::make(EventSubmission::class) ->titleAttribute('title'), Type::make(EventSegment::class) ->titleAttribute('title'), - ]), + ]) + ->required(),
101-103: Improve schedulable type display with human-readable labels.The
schedulable_typecolumn displays the raw class name (e.g.,App\Models\EventSubmission). Consider formatting it to show a user-friendly label.- TextColumn::make('schedulable_type'), + TextColumn::make('schedulable_type') + ->formatStateUsing(fn (string $state): string => class_basename($state)),Or, if SchedulableTypeEnum provides labels:
- TextColumn::make('schedulable_type'), + TextColumn::make('schedulable_type') + ->formatStateUsing(function (?string $state): string { + if (!$state) return '-'; + return \He4rt\Events\Enums\SchedulableTypeEnum::fromClass($state)?->getLabel() ?? class_basename($state); + }),
105-107: Format time columns for better readability.The
starting_atandending_atcolumns display raw time values. Consider formatting them for consistency.- TextColumn::make('starting_at'), + TextColumn::make('starting_at') + ->time('H:i'), - TextColumn::make('ending_at'), + TextColumn::make('ending_at') + ->time('H:i'),app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php (1)
36-69: Minor copy tweak in participant headline (optional).The button label reads
Compartilhe na redes; in PT‑BR this should beCompartilhe nas redes(“nas” instead of “na”).- Compartilhe na redes + Compartilhe nas redesPurely cosmetic, but improves UX text quality.
app-modules/events/src/Models/EventModel.php (1)
66-81: Attendance pivot update looks good; you can simplifyattend()a bit.
- Adding
attend_ordertowithPivot()matches how$participant?->pivot->attend_orderis used in the ticket view.- Computing
attend_orderasattendees_count + waitlist_countafter incrementing is reasonable for a simple join‑order.You probably don’t need the extra
refresh()here, sinceincrement()updates the in‑memory attributes already. You could write:match ($status) { AttendingStatusEnum::Attending => $this->increment('attendees_count'), AttendingStatusEnum::Waitlist => $this->increment('waitlist_count'), default => throw new Exception('Unexpected match value'), }; $this->attendees()->attach($userId, [ 'status' => $status, 'attend_order' => $this->attendees_count + $this->waitlist_count, ]);If you ever need strictly gap‑free, contention‑safe ordering, you’d want a transaction/lock around the counter + attach, but that’s likely overkill here.
Also applies to: 83-101
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/CreateEventSubmissionSpeaker.php (1)
14-19: Consider removing the unnecessary override.The
getHeaderActions()override that returns an empty array is likely redundant, asCreateRecordtypically doesn't include header actions by default.Apply this diff to remove the unnecessary override:
- protected function getHeaderActions(): array - { - return [ - - ]; - }app-modules/events/src/Models/EventSubmission.php (2)
64-75: PHPDoc return type references genericPivotinstead of the actual pivot model.The
@returnannotation usesPivotbut the relationship is configured to useEventSubmissionSpeaker. Consider updating the PHPDoc for better IDE support and type accuracy./** - * @return BelongsToMany<User, $this, Pivot> + * @return BelongsToMany<User, $this, EventSubmissionSpeaker> */ public function speakers(): BelongsToMany
7-7: Unused import if PHPDoc is corrected.The
Pivotimport is only used in the PHPDoc. If the return type annotation is updated to referenceEventSubmissionSpeaker, this import can be removed.app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (1)
7-30: Fix Pintordered_importswarningPint is flagging
ordered_importshere. Please run your formatter (php artisan pintor project-standard tooling) so this file’susestatements match the configured ordering and grouping.app-modules/he4rt/resources/views/components/schedule-card.blade.php (1)
2-6: Props now required; ensure all call sites pass valid valuesAll props (
startsAt,endsAt,title,icon,speakers) are now required and untyped at the template boundary. This is fine, but it makes it easy for a missing or non‑CarbonstartsAt/endsAtto blow up at runtime when->isPast()/->isFuture()or->format()are called.I’d recommend either:
- enforcing non‑nullable Carbon instances at the caller level (and double‑checking all usages of this component in the PR), or
- adding null‑safe guards + a sane fallback config here if a prop is accidentally omitted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
app-modules/events/database/factories/EventAgendaFactory.php(1 hunks)app-modules/events/database/factories/EventSubmissionFactory.php(1 hunks)app-modules/events/database/factories/EventSubmissionSpeakerFactory.php(1 hunks)app-modules/events/database/migrations/2025_11_27_171411_create_event_submission_speakers_table.php(1 hunks)app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php(3 hunks)app-modules/events/resources/views/components/themes/3pontos/components/sections/schedule.blade.php(1 hunks)app-modules/events/src/Enums/SchedulableTypeEnum.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/EventAgenda/EventAgendaResource.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/Talks/TalkResource.php(1 hunks)app-modules/events/src/Filament/App/Talks/TalkResource.php(1 hunks)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php(1 hunks)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/CreateEventSubmissionSpeaker.php(1 hunks)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/EditEventSubmissionSpeaker.php(1 hunks)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/ListEventSubmissionSpeakers.php(1 hunks)app-modules/events/src/Models/EventModel.php(3 hunks)app-modules/events/src/Models/EventSubmission.php(3 hunks)app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php(1 hunks)app-modules/events/tests/Feature/Filament/Admin/Event/TalkRelationManagerTest.php(5 hunks)app-modules/events/tests/Feature/Filament/Admin/Talk/CreateTalk.php(2 hunks)app-modules/events/tests/Feature/Filament/Admin/Talk/EditTalk.php(1 hunks)app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php(5 hunks)app-modules/events/tests/Feature/Filament/App/Talks/ListTalkTest.php(4 hunks)app-modules/he4rt/resources/views/components/schedule-card.blade.php(3 hunks)app-modules/user/src/Models/User.php(3 hunks)database/seeders/BaseSeeder.php(1 hunks)database/seeders/ThreeDotsSeeder.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app-modules/events/src/Enums/SchedulableTypeEnum.php
- app-modules/events/resources/views/components/themes/3pontos/components/sections/schedule.blade.php
🧰 Additional context used
🧬 Code graph analysis (16)
app-modules/events/database/factories/EventSubmissionSpeakerFactory.php (2)
app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php (1)
EventSubmissionSpeaker(13-37)app-modules/events/database/factories/EventSubmissionFactory.php (1)
definition(22-35)
database/seeders/BaseSeeder.php (2)
app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php (1)
user(33-36)app-modules/events/src/Models/EventModel.php (1)
tenant(145-148)
app-modules/events/src/Filament/Admin/Resources/Talks/TalkResource.php (2)
app-modules/events/src/Filament/App/Talks/TalkResource.php (1)
TalkResource(18-45)app-modules/provider/src/Models/Provider.php (1)
model(51-54)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/CreateEventSubmissionSpeaker.php (3)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (1)
EventSubmissionSpeakerResource(32-135)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/EditEventSubmissionSpeaker.php (1)
getHeaderActions(17-24)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/ListEventSubmissionSpeakers.php (1)
getHeaderActions(15-20)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/ListEventSubmissionSpeakers.php (3)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (1)
EventSubmissionSpeakerResource(32-135)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/CreateEventSubmissionSpeaker.php (1)
getHeaderActions(14-19)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/EditEventSubmissionSpeaker.php (1)
getHeaderActions(17-24)
app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php (1)
app-modules/events/src/Models/EventSubmission.php (1)
event(59-62)
app-modules/events/database/migrations/2025_11_27_171411_create_event_submission_speakers_table.php (1)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (1)
table(64-92)
app-modules/events/database/factories/EventSubmissionFactory.php (1)
app-modules/shared/src/Factory.php (1)
Factory(9-12)
app-modules/events/tests/Feature/Filament/App/Talks/ListTalkTest.php (5)
app-modules/events/src/Models/EventModel.php (2)
talks(153-156)tenant(145-148)app-modules/user/src/Models/User.php (1)
talks(105-108)app-modules/events/src/Models/EventSubmission.php (1)
tenant(80-83)app-modules/character/src/Models/Character.php (1)
tenant(60-63)app-modules/season/src/Models/Season.php (1)
tenant(68-71)
app-modules/events/tests/Feature/Filament/Admin/Event/TalkRelationManagerTest.php (3)
app-modules/events/src/Models/EventModel.php (2)
talks(153-156)tenant(145-148)app-modules/user/src/Models/User.php (1)
talks(105-108)app-modules/events/src/Models/EventSubmission.php (2)
event(59-62)tenant(80-83)
app-modules/events/src/Models/EventSubmission.php (2)
app-modules/events/database/factories/EventSubmissionFactory.php (1)
EventSubmissionFactory(18-36)app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php (1)
EventSubmissionSpeaker(13-37)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/EditEventSubmissionSpeaker.php (3)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (1)
EventSubmissionSpeakerResource(32-135)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/CreateEventSubmissionSpeaker.php (1)
getHeaderActions(14-19)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/ListEventSubmissionSpeakers.php (1)
getHeaderActions(15-20)
app-modules/events/src/Filament/App/Talks/TalkResource.php (1)
app-modules/events/src/Filament/Admin/Resources/Talks/TalkResource.php (1)
TalkResource(20-55)
app-modules/events/src/Models/EventModel.php (2)
app-modules/user/src/Models/User.php (1)
talks(105-108)app-modules/events/src/Models/EventSubmission.php (1)
speakers(67-75)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (6)
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/ListEventSubmissionSpeakers.php (1)
ListEventSubmissionSpeakers(11-21)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/CreateEventSubmissionSpeaker.php (1)
CreateEventSubmissionSpeaker(10-20)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/EditEventSubmissionSpeaker.php (1)
EditEventSubmissionSpeaker(13-25)app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php (3)
EventSubmissionSpeaker(13-37)event(25-28)user(33-36)app-modules/provider/src/Models/Provider.php (1)
model(51-54)app-modules/events/src/Models/EventSubmission.php (2)
event(59-62)user(51-54)
app-modules/user/src/Models/User.php (3)
app-modules/events/src/Models/Pivot/EventAttend.php (1)
EventAttend(10-25)app-modules/user/src/Observers/UserObserver.php (1)
UserObserver(9-17)app-modules/events/src/Models/EventModel.php (1)
talks(153-156)
🪛 GitHub Check: Perform Phpstan Check / Run
app-modules/user/src/Models/User.php
[failure] 151-151:
Access to an undefined property He4rt\User\Models\User::$name.
🪛 GitHub Check: Perform Pint format / Run
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php
[warning] 1-1:
Found violation(s) of type: ordered_imports
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Perform Pest Tests / Run
🔇 Additional comments (22)
app-modules/events/resources/views/components/themes/3pontos/components/sections/event-ticket.blade.php (1)
79-89: Ensure this component is only rendered for authenticated users.All bindings here assume
auth()->user()is non‑null. If this Blade component can ever be rendered for guests (or before auth is established), these calls will throw.If it’s guaranteed to live behind auth‑only routes, you’re fine; otherwise consider:
- Passing the user in as a prop, or
- Guarding with
@auth/optional(auth()->user()).app-modules/events/src/Models/EventModel.php (1)
150-173: Relations migrated toEventSubmission/EventAgendalook consistent.
talks()now targetsEventSubmission::classwithevent_idFK.speakers()useshasManyThrough(User::class, EventSubmission::class, 'event_id', 'id', 'id', 'user_id'), matching the submission owner relationship.- New
agenda()relation toEventAgendaordered bystarting_ataligns with the new agenda model.This all lines up with the broader Talk → EventSubmission refactor and new agenda support.
app-modules/events/tests/Feature/Filament/Admin/Talk/CreateTalk.php (1)
10-11: Test updates toEventSubmissionlook correct.The test now asserts against
EventSubmission::classinstead ofTalk, with the same field expectations. This matches the model rename and should keep coverage intact.Also applies to: 45-54
database/seeders/BaseSeeder.php (1)
68-81: Reduced seed counts are fine and help keep environments lighter.Dropping
Provider,Meeting, andMessagefactories tocount(2)each should speed up seeding without changing behavior materially.app-modules/events/tests/Feature/Filament/Admin/Talk/EditTalk.php (1)
7-7: EditTalk test now correctly usesEventSubmission.Using
EventSubmission::factory()->create()and passing its key intoEditTalkkeeps the test aligned with the new model name without changing the scenario.Also applies to: 12-12
app-modules/events/src/Filament/App/Talks/TalkResource.php (1)
16-20: Resource model swap toEventSubmissionlooks consistent.Binding
TalkResourcetoEventSubmission::classmatches the rest of the refactor. Just ensureEventSubmissionexposes atenant()relationship, sincetenantOwnershipRelationshipNameis still'tenant'.app-modules/events/src/Filament/Admin/Resources/Talks/TalkResource.php (1)
17-17: LGTM! Clean refactoring from Talk to EventSubmission.The model reference has been correctly updated throughout the resource, and the changes align with the broader PR objective of renaming Talk to EventSubmission.
Also applies to: 22-22
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/ListEventSubmissionSpeakers.php (1)
11-20: LGTM! Standard Filament list page implementation.The page correctly implements the list page pattern with the appropriate
CreateActionin the header.app-modules/user/src/Models/User.php (1)
13-13: LGTM! Talks relation correctly updated.The import and relation have been properly updated from
TalktoEventSubmission, aligning with the broader refactoring in this PR.Also applies to: 103-108
app-modules/events/database/factories/EventSubmissionFactory.php (1)
9-9: LGTM! Clean factory refactoring.The factory has been properly renamed from
TalkFactorytoEventSubmissionFactory, with all references correctly updated to useEventSubmission::class.Also applies to: 16-20
app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php (1)
10-10: LGTM! Tests correctly updated for EventSubmission.All references to
Talkhave been properly updated toEventSubmissionthroughout the test file, including imports, factory calls, and database assertions.Also applies to: 46-46, 66-66, 78-78, 101-101
app-modules/events/tests/Feature/Filament/App/Talks/ListTalkTest.php (1)
20-24: LGTM!The model rename from
TalktoEventSubmissionis consistently applied across factory usage, type hints, and test assertions.Also applies to: 39-39, 48-48
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource/Pages/EditEventSubmissionSpeaker.php (1)
1-25: LGTM!Standard Filament edit page with appropriate header actions for a soft-deletable resource.
app-modules/events/tests/Feature/Filament/Admin/Event/TalkRelationManagerTest.php (1)
35-39: LGTM!The model rename from
TalktoEventSubmissionis consistently applied across all factory usages, queries, and test data throughout the test file.Also applies to: 107-108, 116-116, 138-146, 158-169
app-modules/events/src/Models/EventSubmission.php (1)
31-46: LGTM on the model rename and fillable changes.The class rename to
EventSubmission, factory binding update, and removal ofstarts_at/ends_atfrom fillable (scheduling now handled byEventAgenda) are correctly implemented.database/seeders/ThreeDotsSeeder.php (3)
60-138: LGTM on the agenda-based seeding structure.The new
$agendaarray cleanly defines event schedule with types, times, and speakers. This is a good improvement over the previous approach.
155-176: Schedulable linkage implemented correctly.The
EventSubmissionis properly created and linked to the agenda viaschedulable_typeandschedulable_id. This addresses the concern from previous reviews about missing schedulable linkage.
145-149: I'll verify the review comment by examining the User migration file and model to determine if thefirstOrCreateoperation will succeed with onlynameandusernameprovided.
<function_calls>
shell
#!/bin/bashFind the User migration file
fd -e php -p 'create_users_table|CreateUsersTable' database/migrations
If found, display its contents
echo "=== USER MIGRATION CONTENTS ==="
find database/migrations -name "users" -type f | head -1 | xargs cat 2>/dev/null
</function_calls>app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php (2)
94-101: Pages and global search wiring look consistentRouting to index/create/edit pages and the global search configuration (eager loading
event/user, searchable attributes, and result details) are coherent with theEventSubmissionSpeakerrelations and should integrate cleanly with Filament’s global search.Also applies to: 111-134
64-92: I'm unable to access the repository to verify the soft-delete mismatch claim. The repository clone operation failed, and the GitHub CLI API request returned a 404 error, which may indicate the repository is not publicly accessible or the path is incorrect.To complete the verification, I need:
- Confirmation of repository accessibility: Is the repository
he4rt/he4rt-bot-apipublic and correctly named?- Direct file access: Can you provide the contents of:
app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php- The migration file that creates the
event_submission_speakertable- The full resource file
app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php(including thegetEloquentQuery()method around lines 103-109)Alternatively, if you can share these file contents directly, I can complete the verification immediately.
app-modules/he4rt/resources/views/components/schedule-card.blade.php (2)
36-36: Explicit time formatting looks goodUsing
$startsAt->format('H:i')for the label is clear and matches the UX intent; no issues here.
47-57: Based on my verification of the codebase and Laravel 12 behavior, I can now provide the rewritten review comment.The primary concern in this review is based on incorrect understanding of Laravel Collection behavior
In Laravel 12 (which this codebase uses),
Illuminate\Support\CollectionimplementsCountable, making empty collections falsy in boolean contexts. Therefore,@if ($speakers)where$speakersis an empty Collection will correctly evaluate to false and prevent rendering the// \\wrapper. The current code is already safe.The suggestion to add
->isNotEmpty()is unnecessary but harmless if preferred for explicitness.The second suggestion to precompute speaker names in the controller/model is valid optional refactoring that would keep the Blade template thinner—consider this if you want cleaner separation of concerns.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php (1)
71-87: Remove or complete the incomplete test.This test creates an accepted
EventSubmissionand then creates another one, but doesn't assert any expected behavior. The test appears incomplete - it should either:
- Assert that a conflict error is raised (if time-based conflicts are still validated), or
- Be removed entirely if the
TalkTimeIsAvailablerule is obsolete with the new EventAgenda architecture.Given that schedule fields have been removed from the talk submission form, this test block is likely obsolete and should be removed:
-describe('TalkTimeIsAvailable Rule', function (): void { - - it('should should not be able to create a talk if there is already one at this time', function (): void { - - EventSubmission::factory()->recycle($this->event)->create([ - 'status' => TalkStatusEnum::Accepted, - ]); - livewire(CreateTalk::class) - ->assertOk() - ->fillForm([ - 'event_id' => $this->event->getKey(), - 'field_type' => 'whatever', - 'title' => 'title whatever', - 'description' => 'description whatever', - ]) - ->call('create'); - }); - it('should should be able to create a talk if there is already one but is not accepted yet', function (): void { - - EventSubmission::factory()->recycle($this->event)->create([ - 'status' => TalkStatusEnum::Pending, - ]); - livewire(CreateTalk::class) - ->assertOk() - ->fillForm([ - 'event_id' => $this->event->getKey(), - 'field_type' => 'whatever', - 'title' => 'title whatever', - 'description' => 'description whatever', - ]) - ->call('create') - ->assertHasNoFormErrors(); - })->skip(); -});app-modules/events/src/Models/EventSubmission.php (1)
38-46: Remove dead code and fix incomplete refactoring of EventSubmission timing fields.The migration removed
starts_atandends_atcolumns from theevents_talkstable, but the EventSubmission model still contains:
- Casts for these non-existent fields (lines 103-104)
- Accessors that reference them (lines 85-97:
start()andend()accessors)- @Property declarations (lines 28-29)
These now reference columns that don't exist in the database, making them dead code. The accessors will always return null since
$this->starts_atno longer exists. Blade templates using$event->startand$event->endwill display null values.Remove:
- Casts for
starts_atandends_at(lines 103-104)- Accessor methods
start()andend()(lines 85-97)- @Property declarations for these fields (lines 28-29)
If timing data is now stored in EventAgenda through the polymorphic relationship, access it through that relationship instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app-modules/events/src/Filament/Admin/Resources/Talks/Schemas/TalkForm.php(1 hunks)app-modules/events/src/Filament/Admin/Resources/Talks/Tables/TalksTable.php(0 hunks)app-modules/events/src/Filament/App/Talks/Schemas/TalkForm.php(1 hunks)app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php(1 hunks)app-modules/events/src/Models/EventSubmission.php(3 hunks)app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php(5 hunks)
💤 Files with no reviewable changes (1)
- app-modules/events/src/Filament/Admin/Resources/Talks/Tables/TalksTable.php
🚧 Files skipped from review as they are similar to previous changes (1)
- app-modules/events/src/Filament/Resources/EventSubmissionSpeakerResource.php
🧰 Additional context used
🧬 Code graph analysis (2)
app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php (1)
app-modules/events/src/Models/EventSubmission.php (1)
event(59-62)
app-modules/events/src/Models/EventSubmission.php (3)
app-modules/events/database/factories/EventSubmissionFactory.php (1)
EventSubmissionFactory(18-36)app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php (1)
EventSubmissionSpeaker(13-37)app-modules/events/src/Models/EventModel.php (2)
UseFactory(43-234)speakers(161-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Perform Pest Tests / Run
🔇 Additional comments (4)
app-modules/events/src/Filament/Admin/Resources/Talks/Schemas/TalkForm.php (1)
64-76: LGTM! Schedule section removal aligns with new architecture.The removal of the schedule section (StartEndFieldsSchema) is consistent with the broader refactoring that moves scheduling concerns to the EventAgenda model. The form now focuses solely on talk proposal details.
app-modules/events/src/Models/EventSubmission.php (1)
1-36: LGTM! Model refactoring is well-structured.The class has been cleanly renamed from Talk to EventSubmission with updated factory references. The table name
events_talksremains unchanged, which is fine for backwards compatibility.app-modules/events/src/Filament/App/Talks/Schemas/TalkForm.php (1)
54-81: LGTM! Consistent removal of schedule fields.Like the Admin version, this form now focuses on talk proposal details with schedule management delegated to EventAgenda. The change is consistent across both Filament panels.
app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php (1)
10-10: LGTM! Model references correctly updated.All references have been properly migrated from
TalktoEventSubmission, including imports, factory calls, and database assertions.Also applies to: 44-50, 64-68
4 participants
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.