Fix interactions unicity by adding wiki to the UNIQUE index#300
Fix interactions unicity by adding wiki to the UNIQUE index#300alterphp wants to merge 1 commit into
Conversation
WalkthroughThis PR updates the user-page interaction uniqueness constraint to include the wiki field. The console command's upsert operation is modified to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a uniqueness constraint issue in the interactions table by adding the wiki column to the UNIQUE index. Previously, the unique constraint only considered user_id and page_id, which could allow duplicate interactions for the same user and page across different wikis.
Key Changes:
- Updated the database UNIQUE index on the interactions table to include the
wikicolumn - Updated the upsert operation to match the new unique constraint by including
wikiin the conflict resolution keys
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| database/migrations/2025_12_07_222422_fix_interactions_unicity.php | Creates a migration to modify the UNIQUE index on interactions table to include wiki column |
| app/Console/Commands/UsersSubscribeCharacteristicsTags.php | Updates the upsert unique key array to include wiki, aligning with the new database constraint |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| public function down(): void | ||
| { | ||
| // |
There was a problem hiding this comment.
The down() method is empty and should implement the rollback logic to restore the original UNIQUE index constraint with only user_id and page_id.
| // | |
| // Restaure l'index UNIQUE original sur (user_id, page_id) | |
| DB::statement(<<<SQL | |
| ALTER TABLE `insights`.`interactions` | |
| DROP INDEX `unique_user_page`, | |
| ADD UNIQUE `unique_user_page` (`user_id`, `page_id`) USING BTREE; | |
| SQL); |
| { | ||
| // Modifie l'index UNIQUE sur les interactions => (user_id, page_id, wiki) | ||
| DB::statement(<<<SQL | ||
| ALTER TABLE `insights`.`interactions` |
There was a problem hiding this comment.
The database name 'insights' is hardcoded in the migration. Consider using the configured database name or removing the database prefix to make this migration more portable across different environments.
| ALTER TABLE `insights`.`interactions` | |
| ALTER TABLE `interactions` |
| */ | ||
| public function up(): void | ||
| { | ||
| // Modifie l'index UNIQUE sur les interactions => (user_id, page_id, wiki) |
There was a problem hiding this comment.
[nitpick] The comment is in French while the codebase appears to use English conventions. Consider translating to English: '// Modifies the UNIQUE index on interactions => (user_id, page_id, wiki)'.
| // Modifie l'index UNIQUE sur les interactions => (user_id, page_id, wiki) | |
| // Modifies the UNIQUE index on interactions => (user_id, page_id, wiki) |
| public function up(): void | ||
| { | ||
| // Modifie l'index UNIQUE sur les interactions => (user_id, page_id, wiki) | ||
| DB::statement(<<<SQL | ||
| ALTER TABLE `insights`.`interactions` | ||
| DROP INDEX `unique_user_page`, | ||
| ADD UNIQUE `unique_user_page` (`user_id`, `page_id`, `wiki`) USING BTREE; | ||
| SQL); | ||
| } |
There was a problem hiding this comment.
The migration drops and recreates the UNIQUE index without handling potential duplicate data. If duplicate rows exist (same user_id, page_id, wiki combination), this migration will fail. Consider adding logic to identify and resolve duplicates before modifying the index.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
database/migrations/2025_12_07_222422_fix_interactions_unicity.php (2)
13-21: Prefer Schema builder over raw SQL for the index change (portability/maintainability).The ALTER TABLE is correct for MySQL, but using
DB::statementwith a hard‑codedinsightsschema makes this migration less portable (e.g., different DB names, test environments, or future engine changes). You already importSchemaandBlueprint, so you could rely on the schema builder instead:- DB::statement(<<<SQL - ALTER TABLE `insights`.`interactions` - DROP INDEX `unique_user_page`, - ADD UNIQUE `unique_user_page` (`user_id`, `page_id`, `wiki`) USING BTREE; - SQL); + Schema::table('interactions', function (Blueprint $table) { + $table->dropUnique('unique_user_page'); + $table->unique(['user_id', 'page_id', 'wiki'], 'unique_user_page'); + });This keeps the migration aligned with Laravel conventions and avoids coupling to a specific schema name.
26-29: Implement a reversibledown()to restore the previous unique key on rollback.
down()is currently a no‑op, so rolling back this migration will leave the(user_id, page_id, wiki)index in place instead of restoring the old(user_id, page_id)constraint. If you rely on rollbacks (local/dev, CI, or feature branches), consider restoring the prior definition:public function down(): void { - // + Schema::table('interactions', function (Blueprint $table) { + $table->dropUnique('unique_user_page'); + $table->unique(['user_id', 'page_id'], 'unique_user_page'); + }); }Adjust the old index definition if it differed, but having a real rollback path will make schema changes safer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Console/Commands/UsersSubscribeCharacteristicsTags.php(1 hunks)database/migrations/2025_12_07_222422_fix_interactions_unicity.php(1 hunks)
🔇 Additional comments (1)
app/Console/Commands/UsersSubscribeCharacteristicsTags.php (1)
72-78: Upsert conflict key now matches the new(user_id, page_id, wiki)unique index—LGTM.Including
wikiin the upsert’s unique key aligns the command’s behavior with the new DB constraint and correctly scopes follows per wiki; just ensure this code is deployed together with the migration so older schemas (without thewikicolumn/index) don’t run this command.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.