fix: new message problem

[Clintonrocha98]

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved tenant isolation for user character resolution and profile updates

• Tests

  • Added comprehensive unit tests for character resolution scenarios, including new user creation and existing user resolution

• Refactor

  • Simplified character resolution model by removing unnecessary dependencies

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The pull request refactors user character resolution to decouple the User dependency from the ResolvedUserCharacter DTO. The UserCharacterResolver is updated to obtain user information through the provider entity instead. Tenant_id filtering is added to provider lookups across multiple locations, and comprehensive unit tests validate the new behavior.

Changes

Cohort / File(s)	Summary
DTO Refactoring app-modules/bot-discord/src/DTO/ResolvedUserCharacter.php	Removed User promoted property from constructor; eliminates direct User dependency. Constructor now accepts only Provider, Character, and isNewUser parameters.
Resolver Logic Updates app-modules/bot-discord/src/Actions/UserCharacterResolver.php	Added tenant_id and model_type filters to provider lookup. Updated new-user flow to refresh character before provider association. Changed existing-user flow to derive user_id from providerEntity->user->id instead of passed user object. Removed User object from provider creation response.
Provider Query Updates app-modules/user/src/Actions/UpdateProfile.php	Added tenant_id filter to Provider query to narrow results by tenant from UpdateProfileDTO.
Test Coverage app-modules/bot-discord/tests/Feature/Actions/UserCharacterResolverTest.php	Added two feature tests: new user provisioning scenario (validates isNewUser=true, entity creation, tenant associations, counts) and existing user resolution scenario (validates isNewUser=false, entity retrieval, count stability).

Sequence Diagram

sequenceDiagram
    participant Client
    participant Resolver as UserCharacterResolver
    participant Provider as Provider<br/>(Entity)
    participant Character as Character<br/>(Entity)
    
    Client->>Resolver: resolve(provider, providerId, username, tenantId)
    
    alt Existing Provider Found
        Resolver->>Provider: Query by tenant_id, model_type,<br/>provider, provider_id
        Provider-->>Resolver: providerEntity
        Resolver->>Character: Fetch character from<br/>providerEntity->character
        Character-->>Resolver: character
        Note over Resolver: isNewUser = false
        Resolver-->>Client: ResolvedUserCharacter(provider,<br/>character, false)
    else New Provider Required
        Resolver->>Provider: Query returns empty
        Resolver->>Provider: Create new provider
        Resolver->>Character: Create new character<br/>for tenant
        Character-->>Resolver: character (experience=0)
        Resolver->>Character: Refresh character
        Resolver->>Provider: Associate character<br/>to provider
        Note over Resolver: isNewUser = true
        Resolver-->>Client: ResolvedUserCharacter(provider,<br/>character, true)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring attention:
  • Verify that removing the User dependency from ResolvedUserCharacter doesn't break downstream consumers; check that providerEntity->user->id reliably provides user context
  • Confirm tenant_id filtering in provider lookups is consistently applied across UpdateProfile and UserCharacterResolver
  • Validate test assertions for entity counts and tenant associations reflect actual database state

Suggested reviewers

• RichardGL11

Poem

🐰 A rabbit's delight in your refactoring spree,
User untangled from each DTO tree,
Provider now holds what the user once knew,
Tests blooming bright to keep all things true! 🌿✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'fix: new message problem' is vague and does not clearly convey the specific technical changes made in the pull request.	Replace the generic title with a specific description of the changes, such as 'fix: remove User dependency from ResolvedUserCharacter and add tenant_id filtering' to better reflect the actual modifications.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/new-message-problem

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

app-modules/bot-discord/src/Actions/UserCharacterResolver.php (1)

22-27: Provider query now correctly scoped by tenant and model type

Adding tenant_id and model_type = User::class to the provider query makes the resolver tenant-safe and ensures only user-backed providers are considered. This directly addresses cross-tenant collisions for the same providerId.

If provider lookups on (tenant_id, model_type, provider, provider_id) are frequent, consider a composite DB index on those columns for better performance.

app-modules/bot-discord/tests/Feature/Actions/UserCharacterResolverTest.php (1)

42-78: Solid existing-user test; consider adding a cross-tenant regression case

The existing-user scenario is well covered: you assert that the resolver reuses the existing provider and character and doesn’t create extra records, which matches the intended behavior.

Given the bug this PR addresses, it may be worth adding a third test where:

• A provider exists for (provider, providerId) in a different tenant, and
• resolve() is called with a new tenantId.

That test should assert that a new user/character/provider is created for the second tenant, guarding the tenant-scoping behavior against future regressions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98b33f5 and 467ee49.

📒 Files selected for processing (4)

• app-modules/bot-discord/src/Actions/UserCharacterResolver.php (2 hunks)
• app-modules/bot-discord/src/DTO/ResolvedUserCharacter.php (0 hunks)
• app-modules/bot-discord/tests/Feature/Actions/UserCharacterResolverTest.php (1 hunks)
• app-modules/user/src/Actions/UpdateProfile.php (1 hunks)

💤 Files with no reviewable changes (1)

• app-modules/bot-discord/src/DTO/ResolvedUserCharacter.php

🧰 Additional context used

🧬 Code graph analysis (2)

app-modules/bot-discord/src/Actions/UserCharacterResolver.php (3)

app-modules/provider/src/Models/Provider.php (1)

• user (59-62)

app-modules/bot-discord/src/DTO/ResolvedUserCharacter.php (1)

• ResolvedUserCharacter (10-17)

app-modules/character/src/Models/Character.php (1)

• Character (28-128)

app-modules/bot-discord/tests/Feature/Actions/UserCharacterResolverTest.php (4)

app-modules/bot-discord/src/Actions/UserCharacterResolver.php (2)

• UserCharacterResolver (14-70)
• resolve (16-69)

app-modules/character/src/Models/Character.php (1)

• Character (28-128)

app-modules/provider/src/Models/Provider.php (1)

• Provider (28-96)

app-modules/tenant/src/Models/Tenant.php (1)

• Tenant (23-104)

⏰ 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). (2)

• GitHub Check: Perform Pest Tests / Run
• GitHub Check: Perform Rector Check / Run

🔇 Additional comments (4)

app-modules/user/src/Actions/UpdateProfile.php (1)

17-21: Tenant scoping for provider lookup looks correct

Adding where('tenant_id', $profileDTO->tenantId) aligns this profile update with tenant-aware provider resolution and prevents cross-tenant profile updates. No further issues spotted here.

app-modules/bot-discord/src/Actions/UserCharacterResolver.php (2)

40-50: Refreshing character after create ensures DB defaults are loaded

Calling $character->refresh() after creating the character is a good fix: it guarantees attributes with DB defaults (like experience = 0) are present on the in-memory model, matching what the tests assert and what downstream code likely expects.

---

59-62: Assumption that providerEntity->user always exists

The existing-user path now derives the character via:

->where('user_id', $providerEntity->user->id)

This is correct as long as every provider with model_type = User::class has a valid related user. If there’s any chance users can be deleted while providers remain, this line would throw on a null relation before firstOrFail() can run.

You may want to either (a) ensure FK + cascade rules prevent orphan providers, or (b) defensively guard for a missing user here (e.g., skip to “new user” behavior or fail with a clearer error) depending on business rules.

app-modules/bot-discord/tests/Feature/Actions/UserCharacterResolverTest.php (1)

12-40: Good coverage for the new-user creation flow

This test thoroughly validates the “no existing provider” path: it checks the resolver flags a new user, wires up provider and character correctly, and asserts counts/tenant IDs. This should catch regressions around character defaults and provider scoping.