From 668780ce94630748df267ebbb71b8eacb11a55f6 Mon Sep 17 00:00:00 2001 From: Flavio Junior Date: Tue, 14 Apr 2026 10:37:07 -0300 Subject: [PATCH] feat: refactor player and album relationship to many-to-many --- Gemfile | 1 + Gemfile.lock | 5 +- README.md | 226 +++++++++++++++++- app/models/album.rb | 5 +- app/models/player.rb | 5 +- app/models/player_album.rb | 6 + .../20260414125603_create_player_albums.rb | 12 + ...1_migrate_album_player_to_player_albums.rb | 16 ++ ...0414131544_remove_player_id_from_albums.rb | 5 + db/schema.rb | 14 +- test/fixtures/albums.yml | 10 +- test/fixtures/player_albums.yml | 19 ++ test/fixtures/players.yml | 3 + test/models/album_test.rb | 13 +- test/models/player_album_test.rb | 26 ++ test/models/player_test.rb | 11 +- 16 files changed, 354 insertions(+), 23 deletions(-) create mode 100644 app/models/player_album.rb create mode 100644 db/migrate/20260414125603_create_player_albums.rb create mode 100644 db/migrate/20260414125631_migrate_album_player_to_player_albums.rb create mode 100644 db/migrate/20260414131544_remove_player_id_from_albums.rb create mode 100644 test/fixtures/player_albums.yml create mode 100644 test/models/player_album_test.rb diff --git a/Gemfile b/Gemfile index 5d14a5c..6ae0e3d 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,7 @@ gem 'jbuilder', '~> 2.5' # Use ActiveStorage variant # gem 'mini_magick', '~> 4.8' +gem "mimemagic", "~> 0.3.10" # Use Capistrano for deployment # gem 'capistrano-rails', group: :development diff --git a/Gemfile.lock b/Gemfile.lock index f8bce13..f9481fa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,7 +96,9 @@ GEM marcel (0.3.2) mimemagic (~> 0.3.2) method_source (0.9.0) - mimemagic (0.3.2) + mimemagic (0.3.10) + nokogiri (~> 1) + rake mini_mime (1.0.0) mini_portile2 (2.3.0) minitest (5.11.3) @@ -199,6 +201,7 @@ DEPENDENCIES coffee-rails (~> 4.2) jbuilder (~> 2.5) listen (>= 3.0.5, < 3.2) + mimemagic (~> 0.3.10) puma (~> 3.12) rails (~> 5.2.0) sass-rails (~> 5.0) diff --git a/README.md b/README.md index 3bd6738..3819bbb 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,225 @@ -# Refactoring de relacionamento +# Ruby Dev Test 2 - Refactoring de Relacionamento -> A Madonna resolveu lançar um album em parceria com a Shakira! E agora?! +## Objetivo -Nosso PO jamais iria esperar que um album pudesse ter mais de um artista. Transforme a relacão 1 para N entre Player e Album em uma relação N para N. Precisamos de testes senão o chato do agilista vai brigar conosco! +Este projeto implementa o refactoring da relação entre `Player` e `Album`, alterando o relacionamento original de **1 para N** para **N para N**. + +O cenário proposto considera que um álbum pode possuir mais de um artista, como no exemplo de uma colaboração entre Madonna e Shakira. + +--- + +## Resumo da Mudança + +Antes do refactoring, a modelagem era: + +- `Player has_many :albums` +- `Album belongs_to :player` + +Isso implicava a existência de uma coluna `player_id` na tabela `albums`, permitindo apenas um artista por álbum. + +Após o refactoring, a modelagem passou a ser: + +- `Player has_many :player_albums` +- `Player has_many :albums, through: :player_albums` +- `Album has_many :player_albums` +- `Album has_many :players, through: :player_albums` + +Foi criada a tabela de junção `player_albums`, permitindo que um álbum esteja associado a múltiplos artistas. + +--- + +## Principais Mudanças + +### 1. Criação da tabela de junção + +Foi criada a tabela `player_albums` para representar o relacionamento N para N entre artistas e álbuns. + +Essa tabela contém: + +- `player_id` +- `album_id` + +Também foi adicionado um índice único para impedir duplicidade do mesmo relacionamento entre artista e álbum. + +--- + +### 2. Migração dos dados existentes + +Como o projeto original já possuía uma relação `albums.player_id`, foi criada uma migration de dados para preservar os vínculos existentes. + +Essa migration copia os dados da relação antiga para a nova tabela `player_albums`. + +### Por que usar migration para isso? + +A migração de dados foi implementada em uma migration, e não em uma rake task, porque a transformação faz parte da evolução versionada do schema e precisa acontecer na ordem correta junto com as demais alterações estruturais. + +Com isso, garantimos que: + +- a criação da join table acontece antes do backfill +- os dados antigos são migrados antes da remoção da foreign key antiga +- qualquer ambiente que rode `db:migrate` chegue ao mesmo estado final + +Uma rake task seria manual e não garantiria execução nem ordem correta. + +--- + +### 3. Remoção da foreign key antiga + +Após a migração dos dados, a coluna `player_id` foi removida da tabela `albums`. + +--- + +### 4. Ajuste dos models + +Os models foram atualizados para refletir o novo relacionamento N para N: + +- `Player` +- `Album` +- `PlayerAlbum` + +Também foi adicionada validação de unicidade na join table para impedir duplicidade do mesmo par `player` + `album`. + +--- + +### 5. Atualização dos testes + +Os testes foram adaptados para validar o novo comportamento: + +- `Album` deixa de exigir `player` +- `Player` pode ter vários álbuns +- `Album` pode ter vários artistas +- `PlayerAlbum` garante presença e unicidade do relacionamento + +Também foram ajustadas as fixtures para refletir o novo modelo. + +--- + +## Decisões de Projeto + +### Manter a stack original + +O projeto foi mantido na stack original: + +- Ruby 2.4.1 +- Rails 5.2 +- SQLite +- Minitest + +A decisão foi preservar o escopo do exercício e evitar misturar o refactoring de domínio com um upgrade de stack. + +### Dependência `mimemagic` + +Foi necessário ajustar a dependência transitiva `mimemagic`, pois a versão originalmente referenciada no lockfile não está mais disponível no RubyGems. + +A correção foi feita de forma mínima, apenas para restaurar a execução do projeto, sem promover upgrades amplos de dependências. + +### Runtime JavaScript + +Foi necessário ter um runtime JavaScript disponível no ambiente para o `uglifier` funcionar corretamente. A solução adotada foi instalar Node.js no ambiente local. + +--- + +## Estrutura Final do Relacionamento + +### Antes + +```ruby +class Album < ApplicationRecord + belongs_to :player +end + +class Player < ApplicationRecord + has_many :albums +end +``` + +### Depois + +```ruby +class Album < ApplicationRecord + has_many :player_albums, dependent: :destroy + has_many :players, through: :player_albums +end + +class Player < ApplicationRecord + has_many :player_albums, dependent: :destroy + has_many :albums, through: :player_albums +end +``` + +--- + +## Como rodar o projeto + +## Pré-requisitos + +- Ruby 2.4.1 +- Bundler 1.16.1 +- SQLite3 +- Node.js + +--- + +## Instalação + +### 1. Clonar o repositório + +```bash +git clone git@github.com:flaviolpgjr/ruby-dev-test-2.git +cd ruby-dev-test-2 +``` + +### 2. Instalar dependências Ruby + +```bash +bundle _1.16.1_ install +``` + +### 3. Preparar o banco + +```bash +bundle _1.16.1_ exec rails db:create +bundle _1.16.1_ exec rails db:migrate +``` + +--- + +## Rodar os testes + +```bash +bundle _1.16.1_ exec rails test +``` + +--- + +## O que os testes validam + +### `AlbumTest` +- álbum válido com nome +- presença de nome +- relacionamento com múltiplos artistas + +### `PlayerTest` +- player válido com nome +- presença de nome +- relacionamento com múltiplos álbuns + +### `PlayerAlbumTest` +- relacionamento válido +- presença de `player` +- presença de `album` +- unicidade do par `player` + `album` + +--- + +## Considerações Finais + +A solução foi construída com foco em: + +- preservação dos dados existentes +- simplicidade +- clareza do refactoring +- cobertura de testes +- respeito ao escopo do exercício + +O refactoring foi implementado em etapas para reduzir risco e garantir consistência durante a transição do relacionamento 1 para N para um relacionamento N para N. diff --git a/app/models/album.rb b/app/models/album.rb index cd2fddd..9cab22b 100644 --- a/app/models/album.rb +++ b/app/models/album.rb @@ -1,5 +1,6 @@ class Album < ApplicationRecord - belongs_to :player + has_many :player_albums, dependent: :destroy + has_many :players, through: :player_albums validates_presence_of :name -end +end \ No newline at end of file diff --git a/app/models/player.rb b/app/models/player.rb index 9bd25b2..6373ff7 100644 --- a/app/models/player.rb +++ b/app/models/player.rb @@ -1,5 +1,6 @@ class Player < ApplicationRecord - has_many :albums + has_many :player_albums, dependent: :destroy + has_many :albums, through: :player_albums validates_presence_of :name -end +end \ No newline at end of file diff --git a/app/models/player_album.rb b/app/models/player_album.rb new file mode 100644 index 0000000..78f77f0 --- /dev/null +++ b/app/models/player_album.rb @@ -0,0 +1,6 @@ +class PlayerAlbum < ApplicationRecord + belongs_to :player + belongs_to :album + + validates :player_id, uniqueness: { scope: :album_id } +end \ No newline at end of file diff --git a/db/migrate/20260414125603_create_player_albums.rb b/db/migrate/20260414125603_create_player_albums.rb new file mode 100644 index 0000000..3cfa4cb --- /dev/null +++ b/db/migrate/20260414125603_create_player_albums.rb @@ -0,0 +1,12 @@ +class CreatePlayerAlbums < ActiveRecord::Migration[5.2] + def change + create_table :player_albums do |t| + t.references :player, foreign_key: true, null: false + t.references :album, foreign_key: true, null: false + + t.timestamps + end + + add_index :player_albums, [:player_id, :album_id], unique: true + end +end \ No newline at end of file diff --git a/db/migrate/20260414125631_migrate_album_player_to_player_albums.rb b/db/migrate/20260414125631_migrate_album_player_to_player_albums.rb new file mode 100644 index 0000000..63b4773 --- /dev/null +++ b/db/migrate/20260414125631_migrate_album_player_to_player_albums.rb @@ -0,0 +1,16 @@ +class MigrateAlbumPlayerToPlayerAlbums < ActiveRecord::Migration[5.2] + def up + Album.reset_column_information + + Album.where.not(player_id: nil).find_each do |album| + PlayerAlbum.find_or_create_by!( + player_id: album.player_id, + album_id: album.id + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end \ No newline at end of file diff --git a/db/migrate/20260414131544_remove_player_id_from_albums.rb b/db/migrate/20260414131544_remove_player_id_from_albums.rb new file mode 100644 index 0000000..dfd3654 --- /dev/null +++ b/db/migrate/20260414131544_remove_player_id_from_albums.rb @@ -0,0 +1,5 @@ +class RemovePlayerIdFromAlbums < ActiveRecord::Migration[5.2] + def change + remove_reference :albums, :player, foreign_key: true + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index 9900836..7f9fd64 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,14 +10,22 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_08_02_203647) do +ActiveRecord::Schema.define(version: 2026_04_14_131544) do create_table "albums", force: :cascade do |t| t.string "name" - t.integer "player_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["player_id"], name: "index_albums_on_player_id" + end + + create_table "player_albums", force: :cascade do |t| + t.integer "player_id", null: false + t.integer "album_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["album_id"], name: "index_player_albums_on_album_id" + t.index ["player_id", "album_id"], name: "index_player_albums_on_player_id_and_album_id", unique: true + t.index ["player_id"], name: "index_player_albums_on_player_id" end create_table "players", force: :cascade do |t| diff --git a/test/fixtures/albums.yml b/test/fixtures/albums.yml index d154cc2..eaab0b4 100644 --- a/test/fixtures/albums.yml +++ b/test/fixtures/albums.yml @@ -1,11 +1,11 @@ fijacion: name: Fijación Oral, Vol. 1 - player: shakira -fixation: +oral_fixation: name: Oral Fixation, Vol. 2 - player: shakira -fixation: +she_wolf: name: She Wolf - player: shakira + +collab_album: + name: Madonna & Shakira \ No newline at end of file diff --git a/test/fixtures/player_albums.yml b/test/fixtures/player_albums.yml new file mode 100644 index 0000000..f739b07 --- /dev/null +++ b/test/fixtures/player_albums.yml @@ -0,0 +1,19 @@ +shakira_fijacion: + player: shakira + album: fijacion + +shakira_oral_fixation: + player: shakira + album: oral_fixation + +shakira_she_wolf: + player: shakira + album: she_wolf + +madonna_collab: + player: madonna + album: collab_album + +shakira_collab: + player: shakira + album: collab_album \ No newline at end of file diff --git a/test/fixtures/players.yml b/test/fixtures/players.yml index f7c00fe..05d0e8f 100644 --- a/test/fixtures/players.yml +++ b/test/fixtures/players.yml @@ -3,3 +3,6 @@ beyonce: shakira: name: Shakira + +madonna: + name: Madonna \ No newline at end of file diff --git a/test/models/album_test.rb b/test/models/album_test.rb index 4494e84..0388b84 100644 --- a/test/models/album_test.rb +++ b/test/models/album_test.rb @@ -2,7 +2,7 @@ class AlbumTest < ActiveSupport::TestCase test "valid album" do - album = Album.new(name: 'Peligro', player: players(:shakira)) + album = Album.new(name: 'Peligro') assert album.valid? end @@ -12,9 +12,10 @@ class AlbumTest < ActiveSupport::TestCase assert_not_empty album.errors[:name] end - test "presence of player" do - album = Album.new - assert_not album.valid? - assert_not_empty album.errors[:player] + test "can have many players" do + album = albums(:collab_album) + + assert_includes album.players, players(:madonna) + assert_includes album.players, players(:shakira) end -end +end \ No newline at end of file diff --git a/test/models/player_album_test.rb b/test/models/player_album_test.rb new file mode 100644 index 0000000..0ffc231 --- /dev/null +++ b/test/models/player_album_test.rb @@ -0,0 +1,26 @@ +require 'test_helper' + +class PlayerAlbumTest < ActiveSupport::TestCase + test "valid player_album" do + player_album = PlayerAlbum.new(player: players(:beyonce), album: albums(:fijacion)) + assert player_album.valid? + end + + test "presence of player" do + player_album = PlayerAlbum.new(album: albums(:fijacion)) + assert_not player_album.valid? + assert_not_empty player_album.errors[:player] + end + + test "presence of album" do + player_album = PlayerAlbum.new(player: players(:shakira)) + assert_not player_album.valid? + assert_not_empty player_album.errors[:album] + end + + test "uniqueness of player scoped to album" do + player_album = PlayerAlbum.new(player: players(:shakira), album: albums(:fijacion)) + assert_not player_album.valid? + assert_not_empty player_album.errors[:player_id] + end +end \ No newline at end of file diff --git a/test/models/player_test.rb b/test/models/player_test.rb index db29e1e..0e99865 100644 --- a/test/models/player_test.rb +++ b/test/models/player_test.rb @@ -11,4 +11,13 @@ class PlayerTest < ActiveSupport::TestCase assert_not player.valid? assert_not_empty player.errors[:name] end -end + + test "can have many albums" do + player = players(:shakira) + + assert_includes player.albums, albums(:fijacion) + assert_includes player.albums, albums(:oral_fixation) + assert_includes player.albums, albums(:she_wolf) + assert_includes player.albums, albums(:collab_album) + end +end \ No newline at end of file