feat: refactor player and album relationship to many-to-many#127
feat: refactor player and album relationship to many-to-many#127flaviolpgjr wants to merge 1 commit intoclicksign:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the domain relationship between Player and Album from 1:N to N:N by introducing a join model/table (player_albums), migrating existing associations, and updating models/tests/docs to reflect the new many-to-many structure.
Changes:
- Added
player_albumsjoin table +PlayerAlbummodel with uniqueness constraint. - Backfilled existing
albums.player_iddata intoplayer_albumsand removedplayer_idfromalbums. - Updated model associations, fixtures, and Minitest coverage; expanded README and adjusted
mimemagicdependency.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/models/player.rb | Replaces direct has_many :albums with has_many :albums, through: :player_albums. |
| app/models/album.rb | Replaces belongs_to :player with has_many :players, through: :player_albums. |
| app/models/player_album.rb | Introduces join model and uniqueness validation. |
| db/migrate/20260414125603_create_player_albums.rb | Creates player_albums table and unique composite index. |
| db/migrate/20260414125631_migrate_album_player_to_player_albums.rb | Migrates existing album-player links into the join table. |
| db/migrate/20260414131544_remove_player_id_from_albums.rb | Removes the old player_id reference from albums. |
| db/schema.rb | Updates schema to reflect the new join table and removed column. |
| test/models/player_test.rb | Adds coverage asserting a player can have many albums. |
| test/models/album_test.rb | Updates validity expectations and asserts an album can have many players. |
| test/models/player_album_test.rb | Adds join model validation tests (presence + uniqueness). |
| test/fixtures/players.yml | Adds madonna fixture. |
| test/fixtures/albums.yml | Removes per-album player association and adds a collab album. |
| test/fixtures/player_albums.yml | Adds join fixtures linking players and albums. |
| README.md | Documents the refactor approach, rationale, and how to run/tests. |
| Gemfile | Pins mimemagic to restore installability. |
| Gemfile.lock | Updates mimemagic resolution and dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def up | ||
| Album.reset_column_information | ||
|
|
||
| Album.where.not(player_id: nil).find_each do |album| | ||
| PlayerAlbum.find_or_create_by!( |
There was a problem hiding this comment.
This data migration references the application models (Album/PlayerAlbum) directly. Migrations should avoid depending on app models because future refactors (renames, validations/callback changes, default scopes) can cause db:migrate to fail on fresh setups. Consider defining lightweight migration-local models (e.g., Class.new(ActiveRecord::Base) with self.table_name = ...) or using SQL/insert_all to backfill the join table.
| def up | |
| Album.reset_column_information | |
| Album.where.not(player_id: nil).find_each do |album| | |
| PlayerAlbum.find_or_create_by!( | |
| class MigrationAlbum < ActiveRecord::Base | |
| self.table_name = "albums" | |
| end | |
| class MigrationPlayerAlbum < ActiveRecord::Base | |
| self.table_name = "player_albums" | |
| end | |
| def up | |
| MigrationAlbum.reset_column_information | |
| MigrationPlayerAlbum.reset_column_information | |
| MigrationAlbum.where.not(player_id: nil).find_each do |album| | |
| MigrationPlayerAlbum.find_or_create_by!( |
|
|
||
| # Use ActiveStorage variant | ||
| # gem 'mini_magick', '~> 4.8' | ||
| gem "mimemagic", "~> 0.3.10" |
There was a problem hiding this comment.
The Gemfile consistently uses single-quoted strings, but this new gem declaration uses double quotes. For consistency with the existing style in this file, switch this line to single quotes.
| gem "mimemagic", "~> 0.3.10" | |
| gem 'mimemagic', '~> 0.3.10' |
Importante
Caso precise isolar o pr em uma release separada, favor incluir no titulo do PR a informação [rls_isolada]
ex: [ARCH-123][rls_isolada]Atualização de Excalibur
Descrição
Este PR realiza o refactoring do relacionamento entre Player e Album, alterando de 1:N para N:N.
Principais mudanças:
A mudança foi feita em etapas para garantir consistência dos dados e compatibilidade durante a transição.
Issue tracker
[Link para o card no Jira.]
Code Review
Como fazer o Code Review:
CONTRIBUTING.md
Screenshots (para mudanças de UI, se houver)
[Adicione alguns screenshots das mudanças de UI inclusas no seu PR, se houver. Isso ajuda o revisor a entender como o usuário utilizaria o código do PR.]
Links e observações
[Links úteis que podem contextualizar e ajudar o revisor, por exemplo para a página de uma dependência que escolheu adicionar, ou um código que se inspirou, ou documentação externa (docs de uma API, do Vue, do Rails, etc).]
Checklist para poder mergear