Skip to content

feat: more migrations#188

Merged
danielhe4rt merged 1 commit into4.xfrom
feat/more-migrations
Mar 23, 2026
Merged

feat: more migrations#188
danielhe4rt merged 1 commit into4.xfrom
feat/more-migrations

Conversation

@danielhe4rt
Copy link
Contributor

@danielhe4rt danielhe4rt commented Mar 23, 2026

Summary

  • Adds more migrations for the project

Summary by CodeRabbit

  • Bug Fixes

    • Improved character experience handling during user account deduplication to prevent data loss
    • Normalized GitHub URLs and LinkedIn URLs in user profiles by fixing malformed links and removing invalid entries
  • Chores

    • Removed an internal auditing tool

@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

This pull request removes the AuditMorphColumns Artisan command that previously audited polymorphic type columns. It updates an external identities deduplication migration to fetch full character records and create new character entries when merging user data results in XP for users without existing characters. Two new migrations normalize social media URLs in the user_information table: one standardizes GitHub URLs to the canonical https://github.com/<handle> format, and another normalizes LinkedIn URLs to https://linkedin.com/in/<handle> format. Both migrations identify and null out junk or malformed entries, and their rollback methods are intentionally irreversible.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title "feat: more migrations" is vague and generic. While it's technically related to the changes (migrations are modified/added), it doesn't convey meaningful information about what the migrations actually accomplish—removing an audit command, deduplicating identities, or normalizing GitHub/LinkedIn URLs. Provide a more specific title that describes the primary change, such as "feat: add GitHub and LinkedIn URL normalization migrations and remove audit command" or focus on the main objective.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

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

Tip

CodeRabbit can use your project's PHP CodeSniffer (phpcs) configuration to improve the quality of PHP code reviews.

Add a phpcs.xml or phpcs.xml.dist file to your project to customize how CodeRabbit runs phpcs. See PHP CodeSniffer documentation for more details.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
database/migrations/2026_03_22_000001_normalize_linkedin_urls.php (1)

10-25: Junk detection constants are duplicated across migrations.

The JUNK_KEYWORDS and JUNK_EXACT arrays are nearly identical between this file and the GitHub URL normalization migration. Consider extracting these to a shared trait or helper class to reduce duplication and ensure consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@database/migrations/2026_03_22_000001_normalize_linkedin_urls.php` around
lines 10 - 25, The JUNK_KEYWORDS and JUNK_EXACT arrays are duplicated across
migrations (e.g., the constants in this migration and the GitHub URL
normalization migration); extract these constants into a single shared helper
(for example a MigrationHelpers or NormalizationHelpers trait/class) and replace
the duplicated private const arrays in both migration classes with references to
the shared symbol (e.g., use MigrationHelpers::JUNK_KEYWORDS or include the
trait that exposes JUNK_KEYWORDS and JUNK_EXACT). Update the migrations to
import/use that helper and remove the duplicated constant declarations so both
migrations read from the single source of truth.
database/migrations/2026_03_21_192332_deduplicate_external_identities.php (1)

32-75: Consider wrapping the migration loop in a transaction.

The migration performs multiple related updates per duplicate (messages, voice_messages, characters, external_identities). If the migration fails midway through processing a duplicate, data could be left in an inconsistent state where some records are updated but others are not.

♻️ Suggested transaction wrapping
         foreach ($duplicates as $dup) {
+            DB::transaction(function () use ($dup) {
             DB::table('messages')
                 ->where('external_identity_id', $dup->old_ei_id)
                 ->update(['external_identity_id' => $dup->new_ei_id]);
             // ... rest of the loop body ...
             DB::table('external_identities')
                 ->where('id', $dup->old_ei_id)
                 ->update(['deleted_at' => now()]);
+            });
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@database/migrations/2026_03_21_192332_deduplicate_external_identities.php`
around lines 32 - 75, Wrap the per-duplicate work in a database transaction to
ensure the updates to messages, voice_messages, characters and
external_identities are atomic: replace the body of the foreach ($duplicates as
$dup) loop with a DB::transaction(function() use ($dup) { ... }) closure so that
all operations that touch messages, voice_messages, characters (the logic that
reads $oldCharacter, increments/creates for $dup->new_user_id) and the
external_identities update are committed or rolled back together; use
DB::transaction to abort on exceptions and keep the outer loop intact.
database/migrations/2026_03_22_000000_normalize_github_urls.php (1)

36-63: Consider batching updates for performance on large datasets.

The migration performs one UPDATE query per row. For tables with many rows, this could be slow. If performance becomes an issue, you could batch updates using whereIn for rows with the same transformation, or use raw SQL CASE expressions.

This is acceptable for a one-time migration, so flagging as optional.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@database/migrations/2026_03_22_000000_normalize_github_urls.php` around lines
36 - 63, The migration's up() iterates rows and issues an UPDATE per row (see
up(), $rows, isJunk(), tryFixUrl()), which can be slow on large tables; modify
it to batch updates by collecting IDs per target value (e.g., null IDs, each
distinct $fixed value) and then perform a single
DB::table('user_information')->whereIn('id', $ids)->update([...]) per group, or
build a single raw UPDATE with a CASE expression mapping id to new github_url to
apply all changes in one query; ensure you still call isJunk() and tryFixUrl()
to determine each row's target and then execute grouped updates instead of
updating inside the foreach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@database/migrations/2026_03_21_192332_deduplicate_external_identities.php`:
- Around line 41-46: The code assumes $oldCharacter is an object and directly
reads $oldCharacter->experience, causing a null dereference when no character
exists for $dup->old_user_id; update the assignment of $oldXp to safely handle a
missing record by either checking if $oldCharacter is null before accessing its
property or using the PHP null-safe operator (e.g. $oldCharacter?->experience ??
0) so $oldXp becomes 0 when no old character is found; adjust the logic around
the $oldCharacter retrieval and the $oldXp assignment in the
deduplicate_external_identities migration accordingly.
- Around line 57-69: The insert can blow up or create a NULL tenant_id because
$oldCharacter may be null or its tenant_id may be null; before calling
DB::table('characters')->insert(...) when $oldXp > 0, validate $oldCharacter and
its tenant_id — if $oldCharacter is null or $oldCharacter->tenant_id is null,
resolve a safe tenant id (e.g. use $dup->tenant_id or lookup the user's tenant)
or skip and log a warning, then only call DB::table('characters')->insert with a
non-null tenant_id; update the block referencing $oldCharacter, $oldXp and
DB::table('characters')->insert accordingly.

In `@database/migrations/2026_03_22_000000_normalize_github_urls.php`:
- Line 45: The line setting $lower uses mb_trim(), which requires PHP 8.4;
replace it with a safe fallback to support older PHP: check if
function_exists('mb_trim') and call mb_trim($row->github_url) when available,
otherwise use trim($row->github_url); then pass the result into mb_strtolower as
currently done (i.e., $value = function_exists('mb_trim') ?
mb_trim($row->github_url) : trim($row->github_url); $lower =
mb_strtolower($value);). This mirrors the version-safe approach used in the
LinkedIn migration and ensures $lower, mb_strtolower, mb_trim and
$row->github_url are handled portably.

---

Nitpick comments:
In `@database/migrations/2026_03_21_192332_deduplicate_external_identities.php`:
- Around line 32-75: Wrap the per-duplicate work in a database transaction to
ensure the updates to messages, voice_messages, characters and
external_identities are atomic: replace the body of the foreach ($duplicates as
$dup) loop with a DB::transaction(function() use ($dup) { ... }) closure so that
all operations that touch messages, voice_messages, characters (the logic that
reads $oldCharacter, increments/creates for $dup->new_user_id) and the
external_identities update are committed or rolled back together; use
DB::transaction to abort on exceptions and keep the outer loop intact.

In `@database/migrations/2026_03_22_000000_normalize_github_urls.php`:
- Around line 36-63: The migration's up() iterates rows and issues an UPDATE per
row (see up(), $rows, isJunk(), tryFixUrl()), which can be slow on large tables;
modify it to batch updates by collecting IDs per target value (e.g., null IDs,
each distinct $fixed value) and then perform a single
DB::table('user_information')->whereIn('id', $ids)->update([...]) per group, or
build a single raw UPDATE with a CASE expression mapping id to new github_url to
apply all changes in one query; ensure you still call isJunk() and tryFixUrl()
to determine each row's target and then execute grouped updates instead of
updating inside the foreach.

In `@database/migrations/2026_03_22_000001_normalize_linkedin_urls.php`:
- Around line 10-25: The JUNK_KEYWORDS and JUNK_EXACT arrays are duplicated
across migrations (e.g., the constants in this migration and the GitHub URL
normalization migration); extract these constants into a single shared helper
(for example a MigrationHelpers or NormalizationHelpers trait/class) and replace
the duplicated private const arrays in both migration classes with references to
the shared symbol (e.g., use MigrationHelpers::JUNK_KEYWORDS or include the
trait that exposes JUNK_KEYWORDS and JUNK_EXACT). Update the migrations to
import/use that helper and remove the duplicated constant declarations so both
migrations read from the single source of truth.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2e90d517-9d67-4a45-8b9d-e59e6c50c71a

📥 Commits

Reviewing files that changed from the base of the PR and between ae8afda and ae1e60f.

📒 Files selected for processing (5)
  • .ai/mcp/mcp.json
  • app/Console/Commands/AuditMorphColumns.php
  • database/migrations/2026_03_21_192332_deduplicate_external_identities.php
  • database/migrations/2026_03_22_000000_normalize_github_urls.php
  • database/migrations/2026_03_22_000001_normalize_linkedin_urls.php
💤 Files with no reviewable changes (1)
  • app/Console/Commands/AuditMorphColumns.php

@danielhe4rt danielhe4rt requested a review from gvieira18 March 23, 2026 00:31
@danielhe4rt danielhe4rt merged commit acd26d4 into 4.x Mar 23, 2026
6 checks passed
@danielhe4rt danielhe4rt deleted the feat/more-migrations branch March 23, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants