Conversation
- Move Message model, actions, DTOs, controllers, requests to Message/ subdomain - Move Voice model, actions, DTOs, requests to Voice/ subdomain - Update namespaces in ActivityServiceProvider - Update routes and MessageReceivedEvent imports - Update related tests
- Add Interaction model with polymorphic source, value tier, and status - Add ActivityType, ActivityStatus, ValueTier enums - Add TrackActivity, ClassifyActivity, CalculateReward actions - Add ApproveInteraction, RejectInteraction actions for admin review - Add ActivitySourceContract for provider implementations - Add HasInteractions trait for Character model - Add InteractionTracked, InteractionApproved events - Add config/activity-tracking.php with classification rules - Add migration for interactions table - Add unit tests for all Tracking actions
- Add DevToOAuthClient implementing OAuthClientContract - Add DevToOAuthAccessDTO and DevToOAuthUser - Add DevToApiClient for DevTo API wrapper - Add SyncDevToArticles artisan command for scheduled polling - Add IntegrationDevToServiceProvider - Add config/integration-devto.php with org_slug and polling settings - Add feature tests for OAuth and sync
- Add DevTo case to IdentityProvider enum with OAuth client binding - Add getClient(), getColor(), getIcon(), getDescription(), getScopes(), isEnabled() methods - Add devto config block to services.php - Add ExternalIdentity DevTo relationship support - Update tests for FindProfile and UpdateProfile
- Add HasInteractions trait for interactions relationship - Use trait in Character model for activity tracking integration
📝 WalkthroughWalkthroughThis PR implements a comprehensive activity tracking system for gamification. It introduces an Interaction model with a database schema, activity classification logic, reward calculation mechanics, and action handlers for approval and rejection workflows. The changes include new enums for activity types and statuses, Filament admin resources for managing interactions, and a Dev.to integration module that syncs articles as trackable activities. Additionally, Message and Voice-related classes are reorganized into dedicated sub-namespaces. The configuration file Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
Clintonrocha98
left a comment
There was a problem hiding this comment.
PR analisado com atenção e aprovado. LGTM (Looks Good To Me).
As mudanças estão alinhadas com o objetivo da tarefa, apresentam boa clareza na implementação e respeitam as convenções adotadas no projeto. A estrutura do código facilita o entendimento, e não foram encontrados pontos críticos ou inconsistências que impeçam a integração.
A solução demonstra cuidado com organização e manutenibilidade, sem indícios de regressões no comportamento esperado. Código validado e pronto para merge.
| { | ||
| protected static ?string $model = Interaction::class; | ||
|
|
||
| protected static string|UnitEnum|null $navigationGroup = 'Gamefication'; |
There was a problem hiding this comment.
Ponto mais semântico em si, aqui não deveria ser Gamification?
| public function getColor(): array | ||
| { | ||
| return match ($this) { | ||
| self::High => Color::Purple, |
There was a problem hiding this comment.
Será que talvez não seria uma boa usar cores simbolizando os tipos? Como geralmente o pessoal usa o vermelho para o High, amarelo para medium
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app-modules/activity/src/Message/Filament/Admin/Resources/Messages/Tables/MessagesTable.php (1)
39-42:⚠️ Potential issue | 🟡 MinorTypo in label: "Obteined" should be "Obtained".
✏️ Proposed fix
TextColumn::make('obtained_experience') - ->label('Obteined XP') + ->label('Obtained XP') ->numeric() ->sortable(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/src/Message/Filament/Admin/Resources/Messages/Tables/MessagesTable.php` around lines 39 - 42, Typo in the Filament column label: update the TextColumn declaration for obtained_experience (TextColumn::make('obtained_experience')->label(...)) in MessagesTable.php to change the label from "Obteined XP" to "Obtained XP" so the UI shows the correct spelling.app-modules/activity/src/Message/Filament/Admin/Resources/Messages/Schemas/MessageForm.php (1)
33-35:⚠️ Potential issue | 🟡 MinorTypo in label: "Chanel" should be "Channel".
✏️ Proposed fix
TextInput::make('channel_id') - ->label('Chanel') + ->label('Channel') ->nullable(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/src/Message/Filament/Admin/Resources/Messages/Schemas/MessageForm.php` around lines 33 - 35, The TextInput field defined by TextInput::make('channel_id') has a typo in its label (->label('Chanel')), so update the label text to the correct spelling "Channel" by changing the label call on the channel_id TextInput to ->label('Channel').app-modules/activity/src/Message/Filament/Admin/Resources/Messages/MessageResource.php (1)
24-24:⚠️ Potential issue | 🟡 MinorTypo: "Gamefication" should be "Gamification".
The navigation group label contains a spelling error.
📝 Proposed fix
- protected static string|UnitEnum|null $navigationGroup = 'Gamefication'; + protected static string|UnitEnum|null $navigationGroup = 'Gamification';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/src/Message/Filament/Admin/Resources/Messages/MessageResource.php` at line 24, Fix the typo in the MessageResource class by updating the protected static property navigationGroup (protected static string|UnitEnum|null $navigationGroup) value from 'Gamefication' to 'Gamification' so the navigation group label is spelled correctly.
🧹 Nitpick comments (11)
app-modules/activity/tests/Unit/Tracking/RejectInteractionTest.php (1)
12-21: Assert persisted state, not only returned instance.To ensure
RejectInteractionactually saves, reload the model and assert values from DB-backed state.Suggested test hardening
test('rejects interaction', function (): void { $interaction = Interaction::factory()->create([ 'status' => ActivityStatus::Pending, ]); $result = resolve(RejectInteraction::class)->handle($interaction); + $interaction->refresh(); expect($result->status)->toBe(ActivityStatus::Rejected) - ->and($result->reviewed_at)->not->toBeNull(); + ->and($result->reviewed_at)->not->toBeNull() + ->and($interaction->status)->toBe(ActivityStatus::Rejected) + ->and($interaction->reviewed_at)->not->toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/tests/Unit/Tracking/RejectInteractionTest.php` around lines 12 - 21, The test currently only asserts the returned instance from RejectInteraction::handle; update it to assert persisted DB state by reloading the Interaction model after calling resolve(RejectInteraction::class)->handle($interaction) (e.g., $interaction->refresh() or Interaction::find($interaction->id)) and then assert that the reloaded model's status is ActivityStatus::Rejected and reviewed_at is not null to ensure the change was saved to the database.app-modules/integration-devto/config/integration-devto.php (1)
8-8: Harden polling interval config against invalid env values.
DEVTO_POLLING_INTERVALshould be normalized to a positive integer to avoid scheduler misconfiguration.Suggested fix
- 'polling_interval_minutes' => env('DEVTO_POLLING_INTERVAL', 30), + 'polling_interval_minutes' => max(1, (int) env('DEVTO_POLLING_INTERVAL', 30)),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-devto/config/integration-devto.php` at line 8, The polling_interval_minutes config uses env('DEVTO_POLLING_INTERVAL', 30) without validation; normalize and enforce a positive integer by reading the env value, casting/parsing it to an integer and falling back to the default when missing/invalid or non-positive (e.g., use intval/ (int) cast and max(1, $value) or is_numeric check), then assign that sanitized value to 'polling_interval_minutes' instead of trusting the raw env call.app-modules/activity/src/Message/Http/Controllers/MessagesController.php (1)
26-34: Consider separating voice functionality into its own controller.The
MessagesControllerunder theMessagenamespace handles both text messages and voice messages. Since voice functionality has been moved to its ownVoicesubdomain (as evidenced by the imports fromHe4rt\Activity\Voice\...), consider movingpostVoiceMessageto a dedicatedVoiceMessagesControllerin the Voice namespace for better separation of concerns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/src/Message/Http/Controllers/MessagesController.php` around lines 26 - 34, The postVoiceMessage method (CreateVoiceMessageRequest, NewVoiceMessage) currently lives in MessagesController and mixes voice subdomain concerns; move this method into a new VoiceMessagesController inside the Voice namespace (e.g., He4rt\Activity\Voice\Http\Controllers) and update routing to point to VoiceMessagesController::postVoiceMessage, keeping the same signature and behavior (call NewVoiceMessage->persist($request->validated()) and return response()->noContent()); remove the method from MessagesController and adjust imports/usages accordingly.app-modules/activity/tests/Unit/Tracking/ApproveInteractionTest.php (1)
36-41: Consider documenting the expected reward calculation.The assertion
coins_awarded = 253is a magic number. Adding a comment explaining how this value is derived (e.g., frompeerReviewBase: 200, engagement metricsreactions: 42, bookmarks: 8, comments: 12, and coin range100-300) would improve test readability and maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/tests/Unit/Tracking/ApproveInteractionTest.php` around lines 36 - 41, The test uses a magic number 253 for coins_awarded/wallet balance without explanation; update the test near the assertion on coins_awarded (expect($result->coins_awarded)->toBe(253)) and the wallet balance check to include a concise inline comment that documents the reward calculation (e.g., base peerReviewBase = 200 plus engagement contributions from reactions/bookmarks/comments and applied coin range 100-300) so future readers can see how 253 was derived; reference the symbols ActivityStatus::Approved, $result->coins_awarded, and $character->fresh()->wallets()->first() when adding the comment.app-modules/integration-devto/src/OAuth/DevToOAuthUser.php (1)
17-19: Consider adding defensive checks for required payload fields.
payload['id']andpayload['username']are accessed directly without null checks. If the Dev.to API returns an unexpected response structure, this will throw an unclear error. Consider validating required fields or providing clearer error messages.🛡️ Proposed defensive validation
public static function make(OAuthAccessDTO $credentials, array $payload): OAuthUserDTO { + if (!isset($payload['id'], $payload['username'])) { + throw new \InvalidArgumentException('DevTo user payload missing required fields: id, username'); + } + return new self( credentials: $credentials, providerId: (string) $payload['id'],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-devto/src/OAuth/DevToOAuthUser.php` around lines 17 - 19, The mapping in DevToOAuthUser that assigns providerId and username directly from $payload risks undefined index errors; update the code in DevToOAuthUser to defensively validate that $payload['id'] and $payload['username'] exist and are non-empty before using them (e.g., check isset/empty or use null-coalescing), and if missing throw or return a clear, descriptive exception/error (include which field is missing and reference IdentityProvider::DevTo in the message) so callers can handle malformed Dev.to responses safely.app-modules/identity/src/ExternalIdentity/Models/ExternalIdentity.php (1)
47-49: Potential N+1 query issue withmessages_countin$appends.The
messages_countaccessor executes$this->messages()->count()on every model serialization. When loading multipleExternalIdentityrecords, this triggers a separate COUNT query per record.Consider using
withCount('messages')when querying, and conditionally appending the attribute only when the count has been loaded:♻️ Proposed refactor to avoid N+1 queries
protected $appends = [ - 'messages_count', ];Then load the count explicitly where needed:
ExternalIdentity::withCount('messages')->get();Or make the accessor conditional:
protected function getMessagesCountAttribute(): int { + if ($this->relationLoaded('messages')) { + return $this->messages->count(); + } + if (array_key_exists('messages_count', $this->attributes)) { + return $this->attributes['messages_count']; + } return $this->messages()->count(); }Also applies to: 92-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/identity/src/ExternalIdentity/Models/ExternalIdentity.php` around lines 47 - 49, The messages_count accessor added via $appends causes an N+1 because getMessagesCountAttribute calls $this->messages()->count() for every model; change the accessor (getMessagesCountAttribute) to first return $this->attributes['messages_count'] if present (this is set by ExternalIdentity::withCount('messages')), then fall back to $this->relationLoaded('messages') ? $this->messages->count() : null (or only then call $this->messages()->count() if you really need a DB hit), and remove unconditional appends usage (or only append 'messages_count' when you explicitly loaded it) so callers should load counts with withCount('messages') when retrieving multiple ExternalIdentity records.app-modules/activity/config/activity-tracking.php (1)
6-22: Consider using enum values in config keys to reduce drift risk.Using raw strings for activity/tier names can silently diverge from enums over time. Prefer enum-backed keys/values where possible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/config/activity-tracking.php` around lines 6 - 22, Replace raw string activity and tier names in the 'classification' array and the 'auto_approve_tiers' list with the corresponding enum values to prevent drift; specifically, use your ActivityType enum members (e.g., ActivityType::ARTICLE, ActivityType::PR_MERGED, etc.) as the keys for the 'classification' map and use ActivityTier enum members (e.g., ActivityTier::HIGH, ActivityTier::MEDIUM, ActivityTier::LOW) for the 'tier' fields and entries in 'auto_approve_tiers' (use ->value or ::value depending on your PHP enum implementation) so the config references the enums ActivityType and ActivityTier instead of raw strings.app-modules/integration-devto/composer.json (1)
8-13: Move test and database namespaces toautoload-dev.
He4rt\\IntegrationDevTo\\Tests\\,He4rt\\IntegrationDevTo\\Database\\Factories\\, andHe4rt\\IntegrationDevTo\\Database\\Seeders\\are currently inautoloadbut should be inautoload-dev. This keeps the production autoloader lean and prevents these namespaces from being loaded when the package is used as a dependency in non-dev environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-devto/composer.json` around lines 8 - 13, The composer.json currently lists test and database namespaces under "autoload" which should be moved to "autoload-dev" to avoid shipping dev-only classes in production; relocate the PSR-4 entries "He4rt\\IntegrationDevTo\\Tests\\", "He4rt\\IntegrationDevTo\\Database\\Factories\\", and "He4rt\\IntegrationDevTo\\Database\\Seeders\\" from the "autoload" -> "psr-4" block into a new or existing "autoload-dev" -> "psr-4" block, ensuring JSON syntax remains valid (commas, braces) after the change.app-modules/activity/src/Tracking/Actions/TrackActivity.php (1)
36-50: Consider adding the interaction creation to the same transaction.If the goal is to ensure atomicity, the
Interaction::query()->create()call should also be within the transaction scope. Currently, if the auto-approval logic fails after creation, a pending-like interaction would remain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/activity/src/Tracking/Actions/TrackActivity.php` around lines 36 - 50, The interaction is created outside the transaction so failures later (e.g., auto-approval) can leave a partial state; move the Interaction::query()->create(...) into the same DB transaction scope (or pass the active transaction/connection to the create call) used for the subsequent logic in TrackActivity.php so creation and auto-approval are atomic; locate the transaction block and replace the external Interaction::query()->create with a create executed inside that transaction (or use the transaction's query builder/connection when calling Interaction::query()).app-modules/integration-devto/src/Polling/SyncDevToArticles.php (2)
93-103: Consider wrapping API call in try-catch to prevent single article failure from stopping sync.If
getArticle()fails for one article, the entire sync command will crash. Catching the exception and logging would allow processing to continue for other articles.♻️ Proposed improvement
if ($existingInteraction !== null) { - $articleDetails = $this->apiClient->getArticle($article['id']); + try { + $articleDetails = $this->apiClient->getArticle($article['id']); + } catch (\Throwable $e) { + Log::warning('DevTo sync: failed to fetch article details for update', [ + 'article_id' => $article['id'], + 'error' => $e->getMessage(), + ]); + return 'skipped'; + } $existingInteraction->update([Apply similar error handling to line 119.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-devto/src/Polling/SyncDevToArticles.php` around lines 93 - 103, In SyncDevToArticles, wrap the call to $this->apiClient->getArticle($article['id']) and the subsequent $existingInteraction->update(...) in a try-catch that catches exceptions from getArticle() (and from update), logs the error with context (article id and exception message) and continues to the next article so a single failure doesn't stop the whole sync; apply the same try-catch pattern to the other API call around line 119 as well so both getArticle-related failures are handled gracefully.
41-55: Extract page size as a constant and add resilience for API failures.The magic number
30should be a named constant for clarity. Additionally, the pagination loop lacks error handling - ifgetArticlesByOrg()throws, the command fails without partial progress being saved.♻️ Proposed improvement
+ private const PAGE_SIZE = 30; + public function handle(): int { $orgSlug = config('integration-devto.org_slug'); $page = 1; $totalCreated = 0; $totalUpdated = 0; $totalSkipped = 0; $this->info('Syncing articles from DevTo org: '.$orgSlug); do { - $articles = $this->apiClient->getArticlesByOrg($orgSlug, $page); + try { + $articles = $this->apiClient->getArticlesByOrg($orgSlug, $page); + } catch (\Throwable $e) { + Log::error('DevTo sync: failed to fetch articles', [ + 'page' => $page, + 'error' => $e->getMessage(), + ]); + $this->error('Failed to fetch articles from DevTo API: '.$e->getMessage()); + break; + } foreach ($articles as $article) { $result = $this->processArticle($article); match ($result) { 'created' => $totalCreated++, 'updated' => $totalUpdated++, 'skipped' => $totalSkipped++, }; } $page++; - } while (count($articles) === 30); + } while (count($articles) === self::PAGE_SIZE);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-modules/integration-devto/src/Polling/SyncDevToArticles.php` around lines 41 - 55, Extract the magic number 30 into a named constant (e.g., private const PAGE_SIZE = 30) and use that constant in the pagination condition instead of the literal; wrap the API call to $this->apiClient->getArticlesByOrg($orgSlug, $page) in a try/catch so failures don't abort the whole run—on exception, log the error via the class logger, stop the loop (or set $articles = [] and break) so partial progress (the $totalCreated/$totalUpdated/$totalSkipped counts from processArticle) is preserved, and only increment $page after a successful fetch; keep references to SyncDevToArticles, getArticlesByOrg, and processArticle to locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app-modules/activity/database/migrations/2026_03_18_000000_create_interactions_table.php`:
- Line 26: The migration currently makes external_ref globally unique which can
cause cross-tenant/provider collisions; update the migration in
create_interactions_table (remove ->unique() from
$table->string('external_ref')->unique()->nullable()) and instead add a
composite unique index such as
$table->unique(['tenant_id','provider','external_ref']) (or whatever
tenant/provider column names are used) so uniqueness is enforced per tenant and
provider.
In `@app-modules/activity/src/Tracking/Actions/ApproveInteraction.php`:
- Around line 20-44: Wrap the entire approval flow in a DB transaction and make
it single-use by reloading and locking the Interaction row for update, verifying
its status is ActivityStatus::Pending before proceeding; then run
calculateReward->handle, credit the wallet (Credit::class with CreditDTO),
increment character experience, and set status/reviewed_at with
interaction->update inside the transaction, and dispatch InteractionApproved
only after the transaction commits (use DB::transaction with DB::afterCommit or
equivalent) and return the fresh interaction; if the status is not Pending,
abort/throw to prevent double-approve.
In `@app-modules/activity/src/Tracking/Actions/CalculateReward.php`:
- Around line 22-45: The final computed reward ($coinsAwarded) is only clamped
to the upper bound; ensure it is clamped into the full allowed range [coins_min,
coins_max] before it's used (including the branch where $engagementSnapshot is
null and when $peerReviewBase is used). Update the logic in CalculateReward
(affecting $coinsAwarded computation) to apply min(max($value,
$interaction->coins_min), $interaction->coins_max) (or equivalent) for both the
engagementSnapshot branch and the else branch so negative or too-small
peerReviewBase values cannot fall below $interaction->coins_min before being
passed to Credit.
In `@app-modules/activity/src/Tracking/Actions/RejectInteraction.php`:
- Around line 12-19: Restrict the state change in handle(Interaction
$interaction) to only update rows currently in ActivityStatus::Pending and fail
if none were affected: perform a conditional update via Interaction::where('id',
$interaction->id)->where('status', ActivityStatus::Pending)->update([ 'status'
=> ActivityStatus::Rejected, 'reviewed_at' => now() ]) and check the returned
affected-rows count; if zero, throw an exception (or return a clear failure) to
signal an invalid transition/race, otherwise return the fresh interaction
record.
In `@app-modules/activity/src/Tracking/Actions/TrackActivity.php`:
- Around line 52-67: Wrap the auto-approval block that handles reward
calculation, wallet credit and XP increment in a database transaction to ensure
atomicity: when $classification['status'] === ActivityStatus::AutoApproved,
perform $this->calculateReward->handle($interaction), find the Character
(Character::query()->findOrFail($dto->characterId)), call
resolve(Credit::class)->handle(new CreditDTO(...)) and
$character->increment('experience', ...) inside a DB::transaction (or equivalent
transactional helper) so failures roll back; consider extending the transaction
to include interaction creation by wrapping the entire handle() method if full
atomicity is desired.
In
`@app-modules/activity/src/Tracking/Filament/Admin/Resources/Interactions/InteractionResource.php`:
- Line 21: The navigation group string on InteractionResource (protected static
string|UnitEnum|null $navigationGroup) is misspelled as 'Gamefication'; update
that value to the correct spelling 'Gamification' so the admin navigation
displays the proper group name.
- Line 25: The resource currently sets protected static ?string
$recordTitleAttribute = 'type' but 'type' is cast to the backed string enum
ActivityType, which can cause unreliable rendering; fix by either adding a
string accessor on the model (e.g. getTypeLabelAttribute or
getTypeDisplayAttribute that returns (string) $this->type or uses
ActivityType::from(...)->label) and point recordTitleAttribute at that accessor
(e.g. 'type_label'), or override the resource title formatter in
InteractionResource (implement a getTitle() / recordTitleUsing() style hook to
return a string from $record->type->value or $record->type->label); reference
InteractionResource::$recordTitleAttribute and the model's type attribute /
ActivityType enum when making the change.
In `@app-modules/activity/src/Voice/Http/Requests/CreateVoiceMessageRequest.php`:
- Around line 19-21: The provider validation in CreateVoiceMessageRequest.php
incorrectly includes "devto" for voice messages; update the 'provider' rule (in
the validation array inside CreateVoiceMessageRequest) to remove "devto" so it
only allows the actual voice providers (e.g., 'twitch,discord'), and keep the
existing 'provider_id' and 'state' rules unchanged; if devto was intentionally
added, instead adjust the allowed 'state' values or add conditional validation
logic in CreateVoiceMessageRequest to handle non-voice providers appropriately.
In `@app-modules/activity/tests/Unit/Actions/NewMessageTest.php`:
- Around line 5-7: The test constructs NewMessage and calls persist using the
old signature and is skipped; update the test to instantiate NewMessage with its
current constructor signature, pass a NewMessageDTO instance (not an array) to
NewMessage->persist(), and update the PersistMessage mock to expect and return
values for a NewMessageDTO parameter; remove or disable the skip so the
assertions run. Specifically, replace the old NewMessage(...) construction with
the new constructor usage, create a NewMessageDTO (using the same test payload
fields), have the PersistMessage mock expect persist(NewMessageDTO $dto) and
return the expected result, then call $newMessage->persist($dto) and assert
outcomes.
In `@app-modules/activity/tests/Unit/Tracking/CalculateRewardTest.php`:
- Around line 43-48: The test titled "uses coins_min when no engagement and auto
approved" doesn't mark the Interaction as auto-approved, so update the
Interaction fixture created via Interaction::factory() to explicitly set the
status to the auto-approved value (e.g., 'approved' or the relevant constant) by
adding a 'status' => 'approved' (or Interaction::STATUS_APPROVED) entry to the
factory payload for this test; alternatively, if you intend to test the pending
case, rename the test to reflect that behavior so the test name matches the
fixture.
In `@app-modules/gamification/src/Character/Models/Character.php`:
- Line 37: Character::wallet() currently defines a HasOne Eloquent relation and
thereby overrides HasWallet::wallet(Currency $currency = Currency::Coin):
?Wallet causing the trait's currency-filtering API to be lost; fix by renaming
the relation method (e.g., to walletRelation() or walletsRelation()) and
keeping/adding a compatibility method matching HasWallet::wallet(Currency
$currency = Currency::Coin): ?Wallet that delegates to the relation (e.g., call
$this->wallets()->where('currency', $currency)->first()) or explicitly calls the
trait implementation via HasWallet::wallet($currency); update any internal call
sites to use the new relation name.
In `@app-modules/integration-devto/src/OAuth/DevToOAuthAccessDTO.php`:
- Around line 11-16: Validate the incoming OAuth payload in
DevToOAuthAccessDTO::make before constructing the DTO: ensure 'access_token'
exists and is a non-empty string (throw an InvalidArgumentException with a clear
message if missing), coerce/validate 'expires_in' to an int or null (reject
non-numeric values), and ensure 'refresh_token' is a string (use empty string
default only after validation); then pass the validated/typed values into the
DevToOAuthAccessDTO constructor so malformed provider responses produce a
controlled exception rather than causing downstream errors.
In `@app-modules/integration-devto/src/OAuth/DevToOAuthClient.php`:
- Around line 39-45: The getAuthenticatedUser method currently calls the Dev.to
API without checking for HTTP errors; update getAuthenticatedUser(OAuthAccessDTO
$credentials) to verify the $response status (e.g., using
$response->successful() or checking $response->failed()) before passing data to
DevToOAuthUser::make, and when the call fails throw or return a meaningful
exception including status code and response body (or log the details) so
callers receive clear error information instead of malformed user data.
- Around line 26-37: The auth method in DevToOAuthClient.php currently posts to
Dev.to and immediately calls ->json() without handling failures; update
auth(string $code) to handle HTTP/network errors by either using the HTTP
client's ->throw() before ->json() or by checking $response->successful() /
$response->ok() and throwing a meaningful exception when the request failed, and
only then pass the validated response data into DevToOAuthAccessDTO::make;
ensure you catch network exceptions (e.g., RequestException) if using try/catch
and include the error context in the thrown exception or log.
In `@app-modules/integration-devto/src/Polling/DevToApiClient.php`:
- Around line 15-31: The Dev.to API calls in getArticles and getArticle silently
return an empty array on failures; update both methods to use Laravel's Http
facade with timeout, retry, and throw so failures surface and are
observable—call Http::timeout(…)->retry(…, …)->throw()->get(...) (or equivalent)
when building the request, and remove the silent fallback that masks errors so
the methods propagate exceptions instead of returning [] on non-2xx responses;
locate these changes in the getArticles (or similar list-fetching method) and
getArticle functions shown in DevToApiClient.
---
Outside diff comments:
In
`@app-modules/activity/src/Message/Filament/Admin/Resources/Messages/MessageResource.php`:
- Line 24: Fix the typo in the MessageResource class by updating the protected
static property navigationGroup (protected static string|UnitEnum|null
$navigationGroup) value from 'Gamefication' to 'Gamification' so the navigation
group label is spelled correctly.
In
`@app-modules/activity/src/Message/Filament/Admin/Resources/Messages/Schemas/MessageForm.php`:
- Around line 33-35: The TextInput field defined by
TextInput::make('channel_id') has a typo in its label (->label('Chanel')), so
update the label text to the correct spelling "Channel" by changing the label
call on the channel_id TextInput to ->label('Channel').
In
`@app-modules/activity/src/Message/Filament/Admin/Resources/Messages/Tables/MessagesTable.php`:
- Around line 39-42: Typo in the Filament column label: update the TextColumn
declaration for obtained_experience
(TextColumn::make('obtained_experience')->label(...)) in MessagesTable.php to
change the label from "Obteined XP" to "Obtained XP" so the UI shows the correct
spelling.
---
Nitpick comments:
In `@app-modules/activity/config/activity-tracking.php`:
- Around line 6-22: Replace raw string activity and tier names in the
'classification' array and the 'auto_approve_tiers' list with the corresponding
enum values to prevent drift; specifically, use your ActivityType enum members
(e.g., ActivityType::ARTICLE, ActivityType::PR_MERGED, etc.) as the keys for the
'classification' map and use ActivityTier enum members (e.g.,
ActivityTier::HIGH, ActivityTier::MEDIUM, ActivityTier::LOW) for the 'tier'
fields and entries in 'auto_approve_tiers' (use ->value or ::value depending on
your PHP enum implementation) so the config references the enums ActivityType
and ActivityTier instead of raw strings.
In `@app-modules/activity/src/Message/Http/Controllers/MessagesController.php`:
- Around line 26-34: The postVoiceMessage method (CreateVoiceMessageRequest,
NewVoiceMessage) currently lives in MessagesController and mixes voice subdomain
concerns; move this method into a new VoiceMessagesController inside the Voice
namespace (e.g., He4rt\Activity\Voice\Http\Controllers) and update routing to
point to VoiceMessagesController::postVoiceMessage, keeping the same signature
and behavior (call NewVoiceMessage->persist($request->validated()) and return
response()->noContent()); remove the method from MessagesController and adjust
imports/usages accordingly.
In `@app-modules/activity/src/Tracking/Actions/TrackActivity.php`:
- Around line 36-50: The interaction is created outside the transaction so
failures later (e.g., auto-approval) can leave a partial state; move the
Interaction::query()->create(...) into the same DB transaction scope (or pass
the active transaction/connection to the create call) used for the subsequent
logic in TrackActivity.php so creation and auto-approval are atomic; locate the
transaction block and replace the external Interaction::query()->create with a
create executed inside that transaction (or use the transaction's query
builder/connection when calling Interaction::query()).
In `@app-modules/activity/tests/Unit/Tracking/ApproveInteractionTest.php`:
- Around line 36-41: The test uses a magic number 253 for coins_awarded/wallet
balance without explanation; update the test near the assertion on coins_awarded
(expect($result->coins_awarded)->toBe(253)) and the wallet balance check to
include a concise inline comment that documents the reward calculation (e.g.,
base peerReviewBase = 200 plus engagement contributions from
reactions/bookmarks/comments and applied coin range 100-300) so future readers
can see how 253 was derived; reference the symbols ActivityStatus::Approved,
$result->coins_awarded, and $character->fresh()->wallets()->first() when adding
the comment.
In `@app-modules/activity/tests/Unit/Tracking/RejectInteractionTest.php`:
- Around line 12-21: The test currently only asserts the returned instance from
RejectInteraction::handle; update it to assert persisted DB state by reloading
the Interaction model after calling
resolve(RejectInteraction::class)->handle($interaction) (e.g.,
$interaction->refresh() or Interaction::find($interaction->id)) and then assert
that the reloaded model's status is ActivityStatus::Rejected and reviewed_at is
not null to ensure the change was saved to the database.
In `@app-modules/identity/src/ExternalIdentity/Models/ExternalIdentity.php`:
- Around line 47-49: The messages_count accessor added via $appends causes an
N+1 because getMessagesCountAttribute calls $this->messages()->count() for every
model; change the accessor (getMessagesCountAttribute) to first return
$this->attributes['messages_count'] if present (this is set by
ExternalIdentity::withCount('messages')), then fall back to
$this->relationLoaded('messages') ? $this->messages->count() : null (or only
then call $this->messages()->count() if you really need a DB hit), and remove
unconditional appends usage (or only append 'messages_count' when you explicitly
loaded it) so callers should load counts with withCount('messages') when
retrieving multiple ExternalIdentity records.
In `@app-modules/integration-devto/composer.json`:
- Around line 8-13: The composer.json currently lists test and database
namespaces under "autoload" which should be moved to "autoload-dev" to avoid
shipping dev-only classes in production; relocate the PSR-4 entries
"He4rt\\IntegrationDevTo\\Tests\\",
"He4rt\\IntegrationDevTo\\Database\\Factories\\", and
"He4rt\\IntegrationDevTo\\Database\\Seeders\\" from the "autoload" -> "psr-4"
block into a new or existing "autoload-dev" -> "psr-4" block, ensuring JSON
syntax remains valid (commas, braces) after the change.
In `@app-modules/integration-devto/config/integration-devto.php`:
- Line 8: The polling_interval_minutes config uses env('DEVTO_POLLING_INTERVAL',
30) without validation; normalize and enforce a positive integer by reading the
env value, casting/parsing it to an integer and falling back to the default when
missing/invalid or non-positive (e.g., use intval/ (int) cast and max(1, $value)
or is_numeric check), then assign that sanitized value to
'polling_interval_minutes' instead of trusting the raw env call.
In `@app-modules/integration-devto/src/OAuth/DevToOAuthUser.php`:
- Around line 17-19: The mapping in DevToOAuthUser that assigns providerId and
username directly from $payload risks undefined index errors; update the code in
DevToOAuthUser to defensively validate that $payload['id'] and
$payload['username'] exist and are non-empty before using them (e.g., check
isset/empty or use null-coalescing), and if missing throw or return a clear,
descriptive exception/error (include which field is missing and reference
IdentityProvider::DevTo in the message) so callers can handle malformed Dev.to
responses safely.
In `@app-modules/integration-devto/src/Polling/SyncDevToArticles.php`:
- Around line 93-103: In SyncDevToArticles, wrap the call to
$this->apiClient->getArticle($article['id']) and the subsequent
$existingInteraction->update(...) in a try-catch that catches exceptions from
getArticle() (and from update), logs the error with context (article id and
exception message) and continues to the next article so a single failure doesn't
stop the whole sync; apply the same try-catch pattern to the other API call
around line 119 as well so both getArticle-related failures are handled
gracefully.
- Around line 41-55: Extract the magic number 30 into a named constant (e.g.,
private const PAGE_SIZE = 30) and use that constant in the pagination condition
instead of the literal; wrap the API call to
$this->apiClient->getArticlesByOrg($orgSlug, $page) in a try/catch so failures
don't abort the whole run—on exception, log the error via the class logger, stop
the loop (or set $articles = [] and break) so partial progress (the
$totalCreated/$totalUpdated/$totalSkipped counts from processArticle) is
preserved, and only increment $page after a successful fetch; keep references to
SyncDevToArticles, getArticlesByOrg, and processArticle to locate the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c21423f-48e8-48a9-bd8c-a0262f536ea1
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (71)
app-modules/activity/config/activity-tracking.phpapp-modules/activity/database/factories/InteractionFactory.phpapp-modules/activity/database/factories/MessageFactory.phpapp-modules/activity/database/migrations/2026_03_18_000000_create_interactions_table.phpapp-modules/activity/routes/message-routes.phpapp-modules/activity/src/Message/Actions/NewMessage.phpapp-modules/activity/src/Message/Actions/PersistMessage.phpapp-modules/activity/src/Message/DTOs/NewMessageDTO.phpapp-modules/activity/src/Message/Filament/Admin/Resources/Messages/MessageResource.phpapp-modules/activity/src/Message/Filament/Admin/Resources/Messages/Pages/CreateMessage.phpapp-modules/activity/src/Message/Filament/Admin/Resources/Messages/Pages/EditMessage.phpapp-modules/activity/src/Message/Filament/Admin/Resources/Messages/Pages/ListMessages.phpapp-modules/activity/src/Message/Filament/Admin/Resources/Messages/Schemas/MessageForm.phpapp-modules/activity/src/Message/Filament/Admin/Resources/Messages/Tables/MessagesTable.phpapp-modules/activity/src/Message/Http/Controllers/MessagesController.phpapp-modules/activity/src/Message/Http/Requests/CreateMessageRequest.phpapp-modules/activity/src/Message/Models/Message.phpapp-modules/activity/src/Providers/ActivityServiceProvider.phpapp-modules/activity/src/Tracking/Actions/ApproveInteraction.phpapp-modules/activity/src/Tracking/Actions/CalculateReward.phpapp-modules/activity/src/Tracking/Actions/ClassifyActivity.phpapp-modules/activity/src/Tracking/Actions/RejectInteraction.phpapp-modules/activity/src/Tracking/Actions/TrackActivity.phpapp-modules/activity/src/Tracking/Concerns/HasInteractions.phpapp-modules/activity/src/Tracking/Contracts/ActivitySourceContract.phpapp-modules/activity/src/Tracking/DTOs/TrackActivityDTO.phpapp-modules/activity/src/Tracking/Enums/ActivityStatus.phpapp-modules/activity/src/Tracking/Enums/ActivityType.phpapp-modules/activity/src/Tracking/Enums/ValueTier.phpapp-modules/activity/src/Tracking/Events/InteractionApproved.phpapp-modules/activity/src/Tracking/Events/InteractionTracked.phpapp-modules/activity/src/Tracking/Filament/Admin/Resources/Interactions/InteractionResource.phpapp-modules/activity/src/Tracking/Filament/Admin/Resources/Interactions/Pages/ListInteractions.phpapp-modules/activity/src/Tracking/Filament/Admin/Resources/Interactions/Pages/ViewInteraction.phpapp-modules/activity/src/Tracking/Filament/Admin/Resources/Interactions/Tables/InteractionsTable.phpapp-modules/activity/src/Tracking/Models/Interaction.phpapp-modules/activity/src/Voice/Actions/NewVoiceMessage.phpapp-modules/activity/src/Voice/DTOs/NewVoiceMessageDTO.phpapp-modules/activity/src/Voice/Http/Requests/CreateVoiceMessageRequest.phpapp-modules/activity/src/Voice/Models/Voice.phpapp-modules/activity/tests/Feature/Filament/Admin/Message/CreateMessageTest.phpapp-modules/activity/tests/Feature/Filament/Admin/Message/DeleteMessageTest.phpapp-modules/activity/tests/Feature/Filament/Admin/Message/EditMessageTest.phpapp-modules/activity/tests/Feature/Filament/Admin/Message/ListMessageTest.phpTest.phpapp-modules/activity/tests/Feature/Filament/Admin/Message/MessageResourceTest.phpapp-modules/activity/tests/Unit/Actions/NewMessageTest.phpapp-modules/activity/tests/Unit/Tracking/ApproveInteractionTest.phpapp-modules/activity/tests/Unit/Tracking/CalculateRewardTest.phpapp-modules/activity/tests/Unit/Tracking/ClassifyActivityTest.phpapp-modules/activity/tests/Unit/Tracking/RejectInteractionTest.phpapp-modules/activity/tests/Unit/Tracking/TrackActivityTest.phpapp-modules/bot-discord/src/Events/MessageReceivedEvent.phpapp-modules/gamification/src/Character/Models/Character.phpapp-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.phpapp-modules/identity/src/ExternalIdentity/Models/ExternalIdentity.phpapp-modules/identity/src/Tenant/Models/Tenant.phpapp-modules/identity/tests/Feature/FindProfileTest.phpapp-modules/identity/tests/Feature/UpdateProfileTest.phpapp-modules/integration-devto/composer.jsonapp-modules/integration-devto/config/integration-devto.phpapp-modules/integration-devto/src/OAuth/DevToOAuthAccessDTO.phpapp-modules/integration-devto/src/OAuth/DevToOAuthClient.phpapp-modules/integration-devto/src/OAuth/DevToOAuthUser.phpapp-modules/integration-devto/src/Polling/DevToApiClient.phpapp-modules/integration-devto/src/Polling/SyncDevToArticles.phpapp-modules/integration-devto/src/Providers/IntegrationDevToServiceProvider.phpapp-modules/integration-devto/tests/Feature/DevToOAuthTest.phpapp-modules/integration-devto/tests/Feature/SyncDevToArticlesTest.phpcomposer.jsonconfig/services.phpdatabase/seeders/BaseSeeder.php
| $table->integer('xp_awarded')->nullable(); | ||
| $table->string('status')->default('pending'); | ||
| $table->nullableUuidMorphs('source'); | ||
| $table->string('external_ref')->unique()->nullable(); |
There was a problem hiding this comment.
Scope external_ref uniqueness to tenant/provider to avoid cross-tenant collisions.
A global unique constraint on external_ref can reject legitimate interactions from another tenant/provider using the same upstream reference.
Suggested migration adjustment
- $table->string('external_ref')->unique()->nullable();
+ $table->string('external_ref')->nullable();
@@
$table->index(['character_id', 'type', 'created_at'], 'idx_interactions_character_type');
$table->index(['status', 'value_tier'], 'idx_interactions_status_tier');
$table->index(['tenant_id', 'occurred_at'], 'idx_interactions_tenant');
+ $table->unique(
+ ['tenant_id', 'provider', 'external_ref'],
+ 'uniq_interactions_tenant_provider_external_ref'
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app-modules/activity/database/migrations/2026_03_18_000000_create_interactions_table.php`
at line 26, The migration currently makes external_ref globally unique which can
cause cross-tenant/provider collisions; update the migration in
create_interactions_table (remove ->unique() from
$table->string('external_ref')->unique()->nullable()) and instead add a
composite unique index such as
$table->unique(['tenant_id','provider','external_ref']) (or whatever
tenant/provider column names are used) so uniqueness is enforced per tenant and
provider.
| public function handle(Interaction $interaction, ?int $peerReviewBase = null): Interaction | ||
| { | ||
| $reward = $this->calculateReward->handle($interaction, $peerReviewBase); | ||
|
|
||
| $character = Character::query()->findOrFail($interaction->character_id); | ||
| $wallet = $character->getOrCreateWallet(); | ||
|
|
||
| resolve(Credit::class)->handle(new CreditDTO( | ||
| walletId: $wallet->id, | ||
| amount: $reward['coins_awarded'], | ||
| referenceType: Interaction::class, | ||
| referenceId: $interaction->id, | ||
| description: 'Reward: '.$interaction->type->value, | ||
| )); | ||
|
|
||
| $character->increment('experience', $reward['xp_awarded']); | ||
|
|
||
| $interaction->update([ | ||
| 'status' => ActivityStatus::Approved, | ||
| 'reviewed_at' => now(), | ||
| ]); | ||
|
|
||
| event(new InteractionApproved($interaction->fresh())); | ||
|
|
||
| return $interaction->fresh(); |
There was a problem hiding this comment.
Make approval atomic and single-use.
This method has no server-side pending check and no transaction around reward calculation, wallet credit, XP increment, and status update. Two concurrent approvals can double-credit coins/XP, and any failure in the middle leaves the interaction partially applied. Wrap the flow in a DB transaction, lock the interaction row, enforce Pending, and dispatch InteractionApproved only after commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/activity/src/Tracking/Actions/ApproveInteraction.php` around
lines 20 - 44, Wrap the entire approval flow in a DB transaction and make it
single-use by reloading and locking the Interaction row for update, verifying
its status is ActivityStatus::Pending before proceeding; then run
calculateReward->handle, credit the wallet (Credit::class with CreditDTO),
increment character experience, and set status/reviewed_at with
interaction->update inside the transaction, and dispatch InteractionApproved
only after the transaction commits (use DB::transaction with DB::afterCommit or
equivalent) and return the fresh interaction; if the status is not Pending,
abort/throw to prevent double-approve.
| if ($engagementSnapshot !== null) { | ||
| $base = $peerReviewBase ?? (int) (($interaction->coins_min + $interaction->coins_max) / 2); | ||
|
|
||
| $reactionsBonus = min( | ||
| ($engagementSnapshot['reactions'] ?? 0) * $engagementFormula['reactions_multiplier'], | ||
| $engagementFormula['reactions_cap'] | ||
| ); | ||
|
|
||
| $bookmarksBonus = min( | ||
| ($engagementSnapshot['bookmarks'] ?? 0) * $engagementFormula['bookmarks_multiplier'], | ||
| $engagementFormula['bookmarks_cap'] | ||
| ); | ||
|
|
||
| $commentsBonus = min( | ||
| ($engagementSnapshot['comments'] ?? 0) * $engagementFormula['comments_multiplier'], | ||
| $engagementFormula['comments_cap'] | ||
| ); | ||
|
|
||
| $engagementBonus = (int) ($reactionsBonus + $bookmarksBonus + $commentsBonus); | ||
| $coinsAwarded = min($base + $engagementBonus, $interaction->coins_max); | ||
| } else { | ||
| $coinsAwarded = $peerReviewBase !== null | ||
| ? min($peerReviewBase, $interaction->coins_max) | ||
| : $interaction->coins_min; |
There was a problem hiding this comment.
Clamp the final reward into [coins_min, coins_max].
The current logic only applies the upper bound. A low or negative peerReviewBase can award less than coins_min, and that value is later passed straight to Credit.
Suggested fix
- $coinsAwarded = min($base + $engagementBonus, $interaction->coins_max);
+ $coinsAwarded = max(
+ $interaction->coins_min,
+ min($base + $engagementBonus, $interaction->coins_max),
+ );
} else {
$coinsAwarded = $peerReviewBase !== null
- ? min($peerReviewBase, $interaction->coins_max)
+ ? max($interaction->coins_min, min($peerReviewBase, $interaction->coins_max))
: $interaction->coins_min;
}📝 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.
| if ($engagementSnapshot !== null) { | |
| $base = $peerReviewBase ?? (int) (($interaction->coins_min + $interaction->coins_max) / 2); | |
| $reactionsBonus = min( | |
| ($engagementSnapshot['reactions'] ?? 0) * $engagementFormula['reactions_multiplier'], | |
| $engagementFormula['reactions_cap'] | |
| ); | |
| $bookmarksBonus = min( | |
| ($engagementSnapshot['bookmarks'] ?? 0) * $engagementFormula['bookmarks_multiplier'], | |
| $engagementFormula['bookmarks_cap'] | |
| ); | |
| $commentsBonus = min( | |
| ($engagementSnapshot['comments'] ?? 0) * $engagementFormula['comments_multiplier'], | |
| $engagementFormula['comments_cap'] | |
| ); | |
| $engagementBonus = (int) ($reactionsBonus + $bookmarksBonus + $commentsBonus); | |
| $coinsAwarded = min($base + $engagementBonus, $interaction->coins_max); | |
| } else { | |
| $coinsAwarded = $peerReviewBase !== null | |
| ? min($peerReviewBase, $interaction->coins_max) | |
| : $interaction->coins_min; | |
| if ($engagementSnapshot !== null) { | |
| $base = $peerReviewBase ?? (int) (($interaction->coins_min + $interaction->coins_max) / 2); | |
| $reactionsBonus = min( | |
| ($engagementSnapshot['reactions'] ?? 0) * $engagementFormula['reactions_multiplier'], | |
| $engagementFormula['reactions_cap'] | |
| ); | |
| $bookmarksBonus = min( | |
| ($engagementSnapshot['bookmarks'] ?? 0) * $engagementFormula['bookmarks_multiplier'], | |
| $engagementFormula['bookmarks_cap'] | |
| ); | |
| $commentsBonus = min( | |
| ($engagementSnapshot['comments'] ?? 0) * $engagementFormula['comments_multiplier'], | |
| $engagementFormula['comments_cap'] | |
| ); | |
| $engagementBonus = (int) ($reactionsBonus + $bookmarksBonus + $commentsBonus); | |
| $coinsAwarded = max( | |
| $interaction->coins_min, | |
| min($base + $engagementBonus, $interaction->coins_max), | |
| ); | |
| } else { | |
| $coinsAwarded = $peerReviewBase !== null | |
| ? max($interaction->coins_min, min($peerReviewBase, $interaction->coins_max)) | |
| : $interaction->coins_min; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/activity/src/Tracking/Actions/CalculateReward.php` around lines
22 - 45, The final computed reward ($coinsAwarded) is only clamped to the upper
bound; ensure it is clamped into the full allowed range [coins_min, coins_max]
before it's used (including the branch where $engagementSnapshot is null and
when $peerReviewBase is used). Update the logic in CalculateReward (affecting
$coinsAwarded computation) to apply min(max($value, $interaction->coins_min),
$interaction->coins_max) (or equivalent) for both the engagementSnapshot branch
and the else branch so negative or too-small peerReviewBase values cannot fall
below $interaction->coins_min before being passed to Credit.
| public function handle(Interaction $interaction): Interaction | ||
| { | ||
| $interaction->update([ | ||
| 'status' => ActivityStatus::Rejected, | ||
| 'reviewed_at' => now(), | ||
| ]); | ||
|
|
||
| return $interaction->fresh(); |
There was a problem hiding this comment.
Enforce pending→rejected transition atomically.
Line 14 updates any interaction state unconditionally. This allows invalid transitions and last-write-wins races against approval. Restrict the update to Pending and fail when no row is updated.
Suggested fix
final class RejectInteraction
{
public function handle(Interaction $interaction): Interaction
{
- $interaction->update([
- 'status' => ActivityStatus::Rejected,
- 'reviewed_at' => now(),
- ]);
+ $updated = Interaction::query()
+ ->whereKey($interaction->getKey())
+ ->where('status', ActivityStatus::Pending)
+ ->update([
+ 'status' => ActivityStatus::Rejected,
+ 'reviewed_at' => now(),
+ ]);
+
+ if ($updated === 0) {
+ throw new \DomainException('Only pending interactions can be rejected.');
+ }
- return $interaction->fresh();
+ return $interaction->refresh();
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/activity/src/Tracking/Actions/RejectInteraction.php` around lines
12 - 19, Restrict the state change in handle(Interaction $interaction) to only
update rows currently in ActivityStatus::Pending and fail if none were affected:
perform a conditional update via Interaction::where('id',
$interaction->id)->where('status', ActivityStatus::Pending)->update([ 'status'
=> ActivityStatus::Rejected, 'reviewed_at' => now() ]) and check the returned
affected-rows count; if zero, throw an exception (or return a clear failure) to
signal an invalid transition/race, otherwise return the fresh interaction
record.
| if ($classification['status'] === ActivityStatus::AutoApproved) { | ||
| $reward = $this->calculateReward->handle($interaction); | ||
|
|
||
| $character = Character::query()->findOrFail($dto->characterId); | ||
| $wallet = $character->getOrCreateWallet(); | ||
|
|
||
| resolve(Credit::class)->handle(new CreditDTO( | ||
| walletId: $wallet->id, | ||
| amount: $reward['coins_awarded'], | ||
| referenceType: Interaction::class, | ||
| referenceId: $interaction->id, | ||
| description: 'Reward: '.$dto->type->value, | ||
| )); | ||
|
|
||
| $character->increment('experience', $reward['xp_awarded']); | ||
| } |
There was a problem hiding this comment.
Wrap auto-approval logic in a database transaction for atomicity.
The auto-approval flow creates an Interaction, credits coins to the wallet, and increments XP. If any step fails after interaction creation (e.g., wallet credit fails), the database will be in an inconsistent state with an interaction marked as auto-approved but no reward credited.
🔒 Proposed fix using DB transaction
+use Illuminate\Support\Facades\DB;
// ... in handle() method:
- if ($classification['status'] === ActivityStatus::AutoApproved) {
- $reward = $this->calculateReward->handle($interaction);
-
- $character = Character::query()->findOrFail($dto->characterId);
- $wallet = $character->getOrCreateWallet();
-
- resolve(Credit::class)->handle(new CreditDTO(
- walletId: $wallet->id,
- amount: $reward['coins_awarded'],
- referenceType: Interaction::class,
- referenceId: $interaction->id,
- description: 'Reward: '.$dto->type->value,
- ));
-
- $character->increment('experience', $reward['xp_awarded']);
- }
+ if ($classification['status'] === ActivityStatus::AutoApproved) {
+ DB::transaction(function () use ($interaction, $dto) {
+ $reward = $this->calculateReward->handle($interaction);
+
+ $character = Character::query()->findOrFail($dto->characterId);
+ $wallet = $character->getOrCreateWallet();
+
+ resolve(Credit::class)->handle(new CreditDTO(
+ walletId: $wallet->id,
+ amount: $reward['coins_awarded'],
+ referenceType: Interaction::class,
+ referenceId: $interaction->id,
+ description: 'Reward: '.$dto->type->value,
+ ));
+
+ $character->increment('experience', $reward['xp_awarded']);
+ });
+ }Note: Consider whether the entire handle() method (including interaction creation) should be wrapped in the transaction for full atomicity.
📝 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.
| if ($classification['status'] === ActivityStatus::AutoApproved) { | |
| $reward = $this->calculateReward->handle($interaction); | |
| $character = Character::query()->findOrFail($dto->characterId); | |
| $wallet = $character->getOrCreateWallet(); | |
| resolve(Credit::class)->handle(new CreditDTO( | |
| walletId: $wallet->id, | |
| amount: $reward['coins_awarded'], | |
| referenceType: Interaction::class, | |
| referenceId: $interaction->id, | |
| description: 'Reward: '.$dto->type->value, | |
| )); | |
| $character->increment('experience', $reward['xp_awarded']); | |
| } | |
| if ($classification['status'] === ActivityStatus::AutoApproved) { | |
| DB::transaction(function () use ($interaction, $dto) { | |
| $reward = $this->calculateReward->handle($interaction); | |
| $character = Character::query()->findOrFail($dto->characterId); | |
| $wallet = $character->getOrCreateWallet(); | |
| resolve(Credit::class)->handle(new CreditDTO( | |
| walletId: $wallet->id, | |
| amount: $reward['coins_awarded'], | |
| referenceType: Interaction::class, | |
| referenceId: $interaction->id, | |
| description: 'Reward: '.$dto->type->value, | |
| )); | |
| $character->increment('experience', $reward['xp_awarded']); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/activity/src/Tracking/Actions/TrackActivity.php` around lines 52
- 67, Wrap the auto-approval block that handles reward calculation, wallet
credit and XP increment in a database transaction to ensure atomicity: when
$classification['status'] === ActivityStatus::AutoApproved, perform
$this->calculateReward->handle($interaction), find the Character
(Character::query()->findOrFail($dto->characterId)), call
resolve(Credit::class)->handle(new CreditDTO(...)) and
$character->increment('experience', ...) inside a DB::transaction (or equivalent
transactional helper) so failures roll back; consider extending the transaction
to include interaction creation by wrapping the entire handle() method if full
atomicity is desired.
| use HasFactory; | ||
| use HasInteractions; | ||
| use HasUuids; | ||
| use HasWallet; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if HasWallet trait defines a wallet() method
ast-grep --pattern $'trait HasWallet {
$$$
function wallet($$$) {
$$$
}
$$$
}'Repository: he4rt/heartdevs.com
Length of output: 45
🏁 Script executed:
# Find the HasWallet trait definition
fd -type f -name "*.php" | xargs rg "trait HasWallet" -lRepository: he4rt/heartdevs.com
Length of output: 297
🏁 Script executed:
# Search for HasWallet more broadly
rg -i "hasWallet" --type php -lRepository: he4rt/heartdevs.com
Length of output: 223
🏁 Script executed:
# View the Character model to see the wallet() method at lines 94-97
cat -n app-modules/gamification/src/Character/Models/Character.php | sed -n '30,100p'Repository: he4rt/heartdevs.com
Length of output: 2544
🏁 Script executed:
# View the HasWallet trait implementation
cat -n app-modules/economy/src/Concerns/HasWallet.phpRepository: he4rt/heartdevs.com
Length of output: 1020
🏁 Script executed:
# Check if Character model or tests call wallet() with Currency parameter
rg "wallet\(" app-modules/gamification/src/Character/Models/Character.phpRepository: he4rt/heartdevs.com
Length of output: 99
🏁 Script executed:
# Check test files for Character and wallet usage patterns
rg "wallet\(" app-modules/gamification/tests/ -A 2 -B 2Repository: he4rt/heartdevs.com
Length of output: 45
Critical method conflict: Character::wallet() incompletely overrides HasWallet::wallet(Currency).
The Character model's explicit wallet(): HasOne relation method (lines 94-97) completely overrides the HasWallet trait's wallet(Currency $currency = Currency::Coin): ?Wallet method (app-modules/economy/src/Concerns/HasWallet.php lines 22-25). These methods have incompatible signatures and return types:
- Trait method: Filters a specific wallet by currency, returns
?Wallet - Character method: Defines a relation, returns
HasOne
The Character class loses access to the trait's wallet filtering functionality. If Character needs to retrieve wallets by currency, it cannot use wallet(Currency) and must resort to calling wallets()->where('currency', $currency)->first() directly, losing the trait's encapsulation.
Either refactor Character::wallet() to delegate to the trait's implementation, or rename one method to avoid the conflict.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/gamification/src/Character/Models/Character.php` at line 37,
Character::wallet() currently defines a HasOne Eloquent relation and thereby
overrides HasWallet::wallet(Currency $currency = Currency::Coin): ?Wallet
causing the trait's currency-filtering API to be lost; fix by renaming the
relation method (e.g., to walletRelation() or walletsRelation()) and
keeping/adding a compatibility method matching HasWallet::wallet(Currency
$currency = Currency::Coin): ?Wallet that delegates to the relation (e.g., call
$this->wallets()->where('currency', $currency)->first()) or explicitly calls the
trait implementation via HasWallet::wallet($currency); update any internal call
sites to use the new relation name.
| public static function make(array $payload): self | ||
| { | ||
| return new self( | ||
| accessToken: $payload['access_token'], | ||
| refreshToken: $payload['refresh_token'] ?? '', | ||
| expiresIn: $payload['expires_in'] ?? null, |
There was a problem hiding this comment.
Validate OAuth payload before DTO construction.
Line 14 assumes access_token always exists, and Line 16 accepts untyped expires_in. This can fail with malformed provider responses instead of producing a controlled error path.
Proposed hardening
+use InvalidArgumentException;
+
final class DevToOAuthAccessDTO extends OAuthAccessDTO
{
public static function make(array $payload): self
{
+ if (
+ ! isset($payload['access_token']) ||
+ ! is_string($payload['access_token']) ||
+ $payload['access_token'] === ''
+ ) {
+ throw new InvalidArgumentException('Dev.to OAuth response missing access_token.');
+ }
+
return new self(
accessToken: $payload['access_token'],
- refreshToken: $payload['refresh_token'] ?? '',
- expiresIn: $payload['expires_in'] ?? null,
+ refreshToken: isset($payload['refresh_token']) && is_string($payload['refresh_token'])
+ ? $payload['refresh_token']
+ : '',
+ expiresIn: isset($payload['expires_in']) ? (int) $payload['expires_in'] : null,
);
}
}📝 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.
| public static function make(array $payload): self | |
| { | |
| return new self( | |
| accessToken: $payload['access_token'], | |
| refreshToken: $payload['refresh_token'] ?? '', | |
| expiresIn: $payload['expires_in'] ?? null, | |
| use InvalidArgumentException; | |
| final class DevToOAuthAccessDTO extends OAuthAccessDTO | |
| { | |
| public static function make(array $payload): self | |
| { | |
| if ( | |
| ! isset($payload['access_token']) || | |
| ! is_string($payload['access_token']) || | |
| $payload['access_token'] === '' | |
| ) { | |
| throw new InvalidArgumentException('Dev.to OAuth response missing access_token.'); | |
| } | |
| return new self( | |
| accessToken: $payload['access_token'], | |
| refreshToken: isset($payload['refresh_token']) && is_string($payload['refresh_token']) | |
| ? $payload['refresh_token'] | |
| : '', | |
| expiresIn: isset($payload['expires_in']) ? (int) $payload['expires_in'] : null, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/integration-devto/src/OAuth/DevToOAuthAccessDTO.php` around lines
11 - 16, Validate the incoming OAuth payload in DevToOAuthAccessDTO::make before
constructing the DTO: ensure 'access_token' exists and is a non-empty string
(throw an InvalidArgumentException with a clear message if missing),
coerce/validate 'expires_in' to an int or null (reject non-numeric values), and
ensure 'refresh_token' is a string (use empty string default only after
validation); then pass the validated/typed values into the DevToOAuthAccessDTO
constructor so malformed provider responses produce a controlled exception
rather than causing downstream errors.
| public function auth(string $code): OAuthAccessDTO | ||
| { | ||
| $response = Http::asForm()->post('https://dev.to/oauth/token', [ | ||
| 'grant_type' => 'authorization_code', | ||
| 'code' => $code, | ||
| 'redirect_uri' => config('services.devto.redirect_uri'), | ||
| 'client_id' => config('services.devto.client_id'), | ||
| 'client_secret' => config('services.devto.client_secret'), | ||
| ]); | ||
|
|
||
| return DevToOAuthAccessDTO::make($response->json()); | ||
| } |
There was a problem hiding this comment.
Add error handling for OAuth token exchange.
The HTTP request to Dev.to's token endpoint lacks error handling. If the request fails (network error, 4xx/5xx response), calling ->json() on an unsuccessful response may return unexpected data or throw an exception.
🛡️ Proposed fix to add error handling
public function auth(string $code): OAuthAccessDTO
{
$response = Http::asForm()->post('https://dev.to/oauth/token', [
'grant_type' => 'authorization_code',
'code' => $code,
'redirect_uri' => config('services.devto.redirect_uri'),
'client_id' => config('services.devto.client_id'),
'client_secret' => config('services.devto.client_secret'),
]);
+ $response->throw();
+
return DevToOAuthAccessDTO::make($response->json());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/integration-devto/src/OAuth/DevToOAuthClient.php` around lines 26
- 37, The auth method in DevToOAuthClient.php currently posts to Dev.to and
immediately calls ->json() without handling failures; update auth(string $code)
to handle HTTP/network errors by either using the HTTP client's ->throw() before
->json() or by checking $response->successful() / $response->ok() and throwing a
meaningful exception when the request failed, and only then pass the validated
response data into DevToOAuthAccessDTO::make; ensure you catch network
exceptions (e.g., RequestException) if using try/catch and include the error
context in the thrown exception or log.
| public function getAuthenticatedUser(OAuthAccessDTO $credentials): OAuthUserDTO | ||
| { | ||
| $response = Http::withToken($credentials->accessToken) | ||
| ->get('https://dev.to/api/users/me'); | ||
|
|
||
| return DevToOAuthUser::make($credentials, $response->json()); | ||
| } |
There was a problem hiding this comment.
Add error handling for user info request.
Same issue as the token exchange - the request to /api/users/me lacks error handling for failed responses.
🛡️ Proposed fix
public function getAuthenticatedUser(OAuthAccessDTO $credentials): OAuthUserDTO
{
$response = Http::withToken($credentials->accessToken)
->get('https://dev.to/api/users/me');
+ $response->throw();
+
return DevToOAuthUser::make($credentials, $response->json());
}📝 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.
| public function getAuthenticatedUser(OAuthAccessDTO $credentials): OAuthUserDTO | |
| { | |
| $response = Http::withToken($credentials->accessToken) | |
| ->get('https://dev.to/api/users/me'); | |
| return DevToOAuthUser::make($credentials, $response->json()); | |
| } | |
| public function getAuthenticatedUser(OAuthAccessDTO $credentials): OAuthUserDTO | |
| { | |
| $response = Http::withToken($credentials->accessToken) | |
| ->get('https://dev.to/api/users/me'); | |
| $response->throw(); | |
| return DevToOAuthUser::make($credentials, $response->json()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/integration-devto/src/OAuth/DevToOAuthClient.php` around lines 39
- 45, The getAuthenticatedUser method currently calls the Dev.to API without
checking for HTTP errors; update getAuthenticatedUser(OAuthAccessDTO
$credentials) to verify the $response status (e.g., using
$response->successful() or checking $response->failed()) before passing data to
DevToOAuthUser::make, and when the call fails throw or return a meaningful
exception including status code and response body (or log the details) so
callers receive clear error information instead of malformed user data.
| $response = Http::get($baseUrl.'/articles', [ | ||
| 'username' => $orgSlug, | ||
| 'per_page' => $perPage, | ||
| 'page' => $page, | ||
| ]); | ||
|
|
||
| return $response->json() ?? []; | ||
| } | ||
|
|
||
| public function getArticle(int $articleId): array | ||
| { | ||
| $baseUrl = config('integration-devto.api_base_url'); | ||
|
|
||
| $response = Http::get(sprintf('%s/articles/%d', $baseUrl, $articleId)); | ||
|
|
||
| return $response->json() ?? []; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect this client for explicit timeout/retry/throw usage around outbound HTTP calls.
fd DevToApiClient.php | xargs -r rg -n -C3 'Http::|->get\(|->timeout\(|->retry\(|->throw\('Repository: he4rt/heartdevs.com
Length of output: 525
🏁 Script executed:
#!/bin/bash
# Check Laravel version and HTTP client availability
if [ -f composer.json ]; then
echo "=== composer.json (Laravel requirement) ==="
cat composer.json | head -20
fi
if [ -f composer.lock ]; then
echo "=== HTTP client package info ==="
cat composer.lock | jq '.packages[] | select(.name == "illuminate/http" or .name | contains("http")) | {name, version}' 2>/dev/null | head -30
fiRepository: he4rt/heartdevs.com
Length of output: 715
🏁 Script executed:
#!/bin/bash
# Search for all HTTP calls in the codebase to understand the scope
echo "=== All Http:: calls in codebase ==="
rg 'Http::' --type php -nRepository: he4rt/heartdevs.com
Length of output: 2616
🌐 Web query:
Laravel Illuminate Http Client timeout retry throw methods documentation
💡 Result:
Laravel's Illuminate HTTP Client (available via the Http facade) provides methods for handling timeouts, retries, and throwing exceptions. These are documented in the official Laravel documentation, with consistent behavior across recent versions (e.g., 11.x, 12.x). ### Timeout - timeout(int|float $seconds): Sets the maximum seconds to wait for a response (default: 30s). Throws Illuminate\Http\Client\ConnectionException if exceeded. Example: $response = Http::timeout(3)->get('...'); - connectTimeout(int|float $seconds): Sets connection timeout (default: 10s). Throws ConnectionException if exceeded. Example: $response = Http::connectTimeout(3)->get('...'); ### Retry - retry(int|array $times, int|Closure $sleepMilliseconds = 0, callable|null $when = null, bool $throw = true): Retries on client/server errors (4xx/5xx). - $times: Number of retries (or array of delays). - $sleepMilliseconds: Fixed delay (ms), or Closure (int $attempt, Exception $exception): int. - $when: Optional callable (Throwable $exception, PendingRequest $request): bool to decide if retry. - $throw: If true (default), throws RequestException after all retries fail; if false, returns last response. Example (basic): $response = Http::retry(3, 100)->post('...'); Example (custom delay): $response = Http::retry(3, fn(int $attempt, Exception $exception) => $attempt * 100)->post('...'); Example (no throw): $response = Http::retry(3, 100, throw: false)->post('...'); Note: Connection exceptions (e.g., timeouts) throw ConnectionException regardless of throw: false. ### Throw Methods (on Response) By default, no exceptions on 4xx/5xx. Use these to throw Illuminate\Http\Client\RequestException (with $response property): - throw: Throws if client/server error (4xx/5xx). - throwIf($condition) or throwIf(Closure $callback): Throws if error AND condition true. - throwUnless($condition) or throwUnless(Closure $callback): Throws if error AND condition false. Example: $response = Http::post('...'); $response->throw; // Throws on 4xx/5xx $response->throwIf(fn(Response $response) => $response->json('error.code') === 'INVALID'); Chainable: Http::post('...')->throw->json('user.id'); Sources: Official docs confirm these across Laravel 11.x/12.x (e.g., retry throw param added ~Laravel 9, throwIf/throwUnless refined later).
Citations:
- 1: https://laravel.com/docs/12.x/http-client
- 2: https://laravel.com/docs/master/http-client
- 3: https://laravel.com/docs/11.x/http-client
- 4: https://laravel.com/docs/8.x/http-client
- 5: https://v10.x.laravel.wiki/en/http-client
- 6: [9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions laravel/framework#40079
- 7: https://api.laravel.com/docs/11.x/Illuminate/Http/Client/PendingRequest.html
Add timeout, retry, and throw to Dev.to API calls for resilience and observability.
Current calls silently degrade into [] on failed responses, masking API errors and making sync reliability opaque. The suggested methods are available in Laravel's Http facade and should be applied consistently.
Hardening implementation
public function getArticlesByOrg(string $orgSlug, int $page = 1, int $perPage = 30): array
{
$baseUrl = config('integration-devto.api_base_url');
- $response = Http::get($baseUrl.'/articles', [
+ $response = Http::acceptJson()
+ ->timeout(10)
+ ->retry(3, 200)
+ ->get($baseUrl.'/articles', [
'username' => $orgSlug,
'per_page' => $perPage,
'page' => $page,
- ]);
+ ])->throw();
return $response->json() ?? [];
}
public function getArticle(int $articleId): array
{
$baseUrl = config('integration-devto.api_base_url');
- $response = Http::get(sprintf('%s/articles/%d', $baseUrl, $articleId));
+ $response = Http::acceptJson()
+ ->timeout(10)
+ ->retry(3, 200)
+ ->get(sprintf('%s/articles/%d', $baseUrl, $articleId))
+ ->throw();
return $response->json() ?? [];
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-modules/integration-devto/src/Polling/DevToApiClient.php` around lines 15
- 31, The Dev.to API calls in getArticles and getArticle silently return an
empty array on failures; update both methods to use Laravel's Http facade with
timeout, retry, and throw so failures surface and are observable—call
Http::timeout(…)->retry(…, …)->throw()->get(...) (or equivalent) when building
the request, and remove the silent fallback that masks errors so the methods
propagate exceptions instead of returning [] on non-2xx responses; locate these
changes in the getArticles (or similar list-fetching method) and getArticle
functions shown in DevToApiClient.
Summary
Trackingsubdomain withInteractionmodel for message/voice activity trackingHasInteractionstrait toCharactermodelDevToidentity provider and DevTo OAuth integration with article syncCommits
89f2385feat(gamification): add HasInteractions trait to Character modele1215c1feat(identity): add DevTo to IdentityProvider enum32e815bfeat(integration-devto): add new module for DevTo OAuth and article sync644964bfeat(activity): add Tracking subdomain with Interaction modelc49427erefactor(activity): restructure module into Message/Voice subdomainsSummary by CodeRabbit
Release Notes