From 872b6fdb591ae344fa83cb224818dab33e010c56 Mon Sep 17 00:00:00 2001 From: Justin Edmund Date: Wed, 3 Dec 2025 22:51:34 -0800 Subject: [PATCH] add crew specs and fix error handling - add transactional fixtures to rails_helper for test isolation - restructure crew errors to CrewErrors module for Zeitwerk - add rescue_from for CrewErrors::CrewError in api_controller - add model specs for Crew and CrewMembership (34 examples) - add controller specs for crews and memberships (28 examples) - add crew-related specs to User model (22 examples) - add factories for crews and crew_memberships --- app/controllers/api/v1/api_controller.rb | 9 + .../api/v1/crew_memberships_controller.rb | 8 +- app/controllers/api/v1/crews_controller.rb | 14 +- app/errors/api/v1/crew_errors.rb | 89 ------ app/errors/crew_errors.rb | 119 ++++++++ spec/factories/crew_memberships.rb | 21 ++ spec/factories/crews.rb | 8 + spec/models/crew_membership_spec.rb | 137 ++++++++++ spec/models/crew_spec.rb | 134 ++++++++++ spec/models/user_spec.rb | 119 +++++++- spec/rails_helper.rb | 9 + .../crew_memberships_controller_spec.rb | 144 ++++++++++ spec/requests/crews_controller_spec.rb | 253 ++++++++++++++++++ 13 files changed, 963 insertions(+), 101 deletions(-) delete mode 100644 app/errors/api/v1/crew_errors.rb create mode 100644 app/errors/crew_errors.rb create mode 100644 spec/factories/crew_memberships.rb create mode 100644 spec/factories/crews.rb create mode 100644 spec/models/crew_membership_spec.rb create mode 100644 spec/models/crew_spec.rb create mode 100644 spec/requests/crew_memberships_controller_spec.rb create mode 100644 spec/requests/crews_controller_spec.rb diff --git a/app/controllers/api/v1/api_controller.rb b/app/controllers/api/v1/api_controller.rb index 843a370..1bab0b2 100644 --- a/app/controllers/api/v1/api_controller.rb +++ b/app/controllers/api/v1/api_controller.rb @@ -31,10 +31,19 @@ module Api render json: e.to_hash, status: e.http_status end + # Crew errors + rescue_from CrewErrors::CrewError do |e| + render json: e.to_hash, status: e.http_status + end + rescue_from GranblueError do |e| render_error(e) end + rescue_from Api::V1::GranblueError do |e| + render_error(e) + end + ##### Hooks before_action :current_user before_action :default_content_type diff --git a/app/controllers/api/v1/crew_memberships_controller.rb b/app/controllers/api/v1/crew_memberships_controller.rb index 9aa73f4..c5b7285 100644 --- a/app/controllers/api/v1/crew_memberships_controller.rb +++ b/app/controllers/api/v1/crew_memberships_controller.rb @@ -25,7 +25,7 @@ module Api # DELETE /crews/:crew_id/memberships/:id def destroy - raise CannotRemoveCaptainError if @membership.captain? + raise CrewErrors::CannotRemoveCaptainError if @membership.captain? @membership.retire! head :no_content @@ -33,11 +33,11 @@ module Api # POST /crews/:crew_id/memberships/:id/promote def promote - raise CannotRemoveCaptainError if @membership.captain? + raise CrewErrors::CannotRemoveCaptainError if @membership.captain? # Check vice captain limit current_vc_count = @crew.crew_memberships.where(role: :vice_captain, retired: false).count - raise ViceCaptainLimitError if current_vc_count >= 3 && !@membership.vice_captain? + raise CrewErrors::ViceCaptainLimitError if current_vc_count >= 3 && !@membership.vice_captain? @membership.update!(role: :vice_captain) render json: CrewMembershipBlueprint.render(@membership, view: :with_user, root: :membership) @@ -45,7 +45,7 @@ module Api # POST /crews/:crew_id/memberships/:id/demote def demote - raise CannotRemoveCaptainError if @membership.captain? + raise CrewErrors::CannotDemoteCaptainError if @membership.captain? @membership.update!(role: :member) render json: CrewMembershipBlueprint.render(@membership, view: :with_user, root: :membership) diff --git a/app/controllers/api/v1/crews_controller.rb b/app/controllers/api/v1/crews_controller.rb index 88b9113..3cef022 100644 --- a/app/controllers/api/v1/crews_controller.rb +++ b/app/controllers/api/v1/crews_controller.rb @@ -7,6 +7,7 @@ module Api before_action :restrict_access before_action :set_crew, only: %i[show update members leave transfer_captain] + before_action :require_crew!, only: %i[show update members] before_action :authorize_crew_member!, only: %i[show members] before_action :authorize_crew_officer!, only: %i[update] before_action :authorize_crew_captain!, only: %i[transfer_captain] @@ -18,7 +19,7 @@ module Api # POST /crews def create - raise AlreadyInCrewError if current_user.crew.present? + raise CrewErrors::AlreadyInCrewError if current_user.crew.present? @crew = Crew.new(crew_params) @@ -47,10 +48,9 @@ module Api # POST /crew/leave def leave - raise NotInCrewError unless @crew - membership = current_user.active_crew_membership - raise CaptainCannotLeaveError if membership.captain? + raise CrewErrors::NotInCrewError unless membership + raise CrewErrors::CaptainCannotLeaveError if membership.captain? membership.retire! head :no_content @@ -61,7 +61,7 @@ module Api new_captain_id = params[:user_id] new_captain_membership = @crew.active_memberships.find_by(user_id: new_captain_id) - raise MemberNotFoundError unless new_captain_membership + raise CrewErrors::MemberNotFoundError unless new_captain_membership ActiveRecord::Base.transaction do current_user.active_crew_membership.update!(role: :vice_captain) @@ -84,6 +84,10 @@ module Api def crew_params params.require(:crew).permit(:name, :gamertag, :granblue_crew_id, :description) end + + def require_crew! + render_not_found_response('crew') unless @crew + end end end end diff --git a/app/errors/api/v1/crew_errors.rb b/app/errors/api/v1/crew_errors.rb deleted file mode 100644 index 918ad43..0000000 --- a/app/errors/api/v1/crew_errors.rb +++ /dev/null @@ -1,89 +0,0 @@ -# frozen_string_literal: true - -module Api - module V1 - class AlreadyInCrewError < GranblueError - def http_status - 422 - end - - def code - 'already_in_crew' - end - - def message - 'You are already in a crew' - end - end - - class CaptainCannotLeaveError < GranblueError - def http_status - 422 - end - - def code - 'captain_cannot_leave' - end - - def message - 'Captain must transfer ownership before leaving' - end - end - - class CannotRemoveCaptainError < GranblueError - def http_status - 422 - end - - def code - 'cannot_remove_captain' - end - - def message - 'Cannot remove the captain from the crew' - end - end - - class ViceCaptainLimitError < GranblueError - def http_status - 422 - end - - def code - 'vice_captain_limit' - end - - def message - 'Crew can only have up to 3 vice captains' - end - end - - class NotInCrewError < GranblueError - def http_status - 422 - end - - def code - 'not_in_crew' - end - - def message - 'You are not in a crew' - end - end - - class MemberNotFoundError < GranblueError - def http_status - 404 - end - - def code - 'member_not_found' - end - - def message - 'Member not found in this crew' - end - end - end -end diff --git a/app/errors/crew_errors.rb b/app/errors/crew_errors.rb new file mode 100644 index 0000000..56b18a5 --- /dev/null +++ b/app/errors/crew_errors.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +module CrewErrors + # Base class for all crew-related errors + class CrewError < StandardError + def http_status + :unprocessable_entity + end + + def code + self.class.name.demodulize.underscore + end + + def to_hash + { + message: message, + code: code + } + end + end + + class AlreadyInCrewError < CrewError + def http_status + :unprocessable_entity + end + + def code + 'already_in_crew' + end + + def message + 'You are already in a crew' + end + end + + class CaptainCannotLeaveError < CrewError + def http_status + :unprocessable_entity + end + + def code + 'captain_cannot_leave' + end + + def message + 'Captain must transfer ownership before leaving' + end + end + + class CannotRemoveCaptainError < CrewError + def http_status + :unprocessable_entity + end + + def code + 'cannot_remove_captain' + end + + def message + 'Cannot remove the captain from the crew' + end + end + + class ViceCaptainLimitError < CrewError + def http_status + :unprocessable_entity + end + + def code + 'vice_captain_limit' + end + + def message + 'Crew can only have up to 3 vice captains' + end + end + + class NotInCrewError < CrewError + def http_status + :unprocessable_entity + end + + def code + 'not_in_crew' + end + + def message + 'You are not in a crew' + end + end + + class MemberNotFoundError < CrewError + def http_status + :not_found + end + + def code + 'member_not_found' + end + + def message + 'Member not found in this crew' + end + end + + class CannotDemoteCaptainError < CrewError + def http_status + :unprocessable_entity + end + + def code + 'cannot_demote_captain' + end + + def message + 'Cannot demote the captain' + end + end +end diff --git a/spec/factories/crew_memberships.rb b/spec/factories/crew_memberships.rb new file mode 100644 index 0000000..ef4f97e --- /dev/null +++ b/spec/factories/crew_memberships.rb @@ -0,0 +1,21 @@ +FactoryBot.define do + factory :crew_membership do + crew + user + role { :member } + retired { false } + + trait :captain do + role { :captain } + end + + trait :vice_captain do + role { :vice_captain } + end + + trait :retired do + retired { true } + retired_at { Time.current } + end + end +end diff --git a/spec/factories/crews.rb b/spec/factories/crews.rb new file mode 100644 index 0000000..e8a73b6 --- /dev/null +++ b/spec/factories/crews.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :crew do + name { Faker::Team.name } + gamertag { Faker::Alphanumeric.alpha(number: 5).upcase } + granblue_crew_id { Faker::Number.number(digits: 8).to_s } + description { Faker::Lorem.paragraph } + end +end diff --git a/spec/models/crew_membership_spec.rb b/spec/models/crew_membership_spec.rb new file mode 100644 index 0000000..8767c44 --- /dev/null +++ b/spec/models/crew_membership_spec.rb @@ -0,0 +1,137 @@ +require 'rails_helper' + +RSpec.describe CrewMembership, type: :model do + describe 'associations' do + it { should belong_to(:crew) } + it { should belong_to(:user) } + end + + describe 'validations' do + it 'validates uniqueness of user_id scoped to crew_id' do + crew = create(:crew) + user = create(:user) + create(:crew_membership, crew: crew, user: user) + + duplicate = build(:crew_membership, crew: crew, user: user) + expect(duplicate).not_to be_valid + expect(duplicate.errors[:user_id]).to include('has already been taken') + end + end + + describe 'role enum' do + it { should define_enum_for(:role).with_values(member: 0, vice_captain: 1, captain: 2) } + end + + describe 'scopes' do + let(:crew) { create(:crew) } + + describe '.active' do + it 'returns only non-retired memberships' do + active = create(:crew_membership, crew: crew) + retired = create(:crew_membership, :retired, crew: crew) + + expect(CrewMembership.active).to include(active) + expect(CrewMembership.active).not_to include(retired) + end + end + + describe '.retired' do + it 'returns only retired memberships' do + active = create(:crew_membership, crew: crew) + retired = create(:crew_membership, :retired, crew: crew) + + expect(CrewMembership.retired).to include(retired) + expect(CrewMembership.retired).not_to include(active) + end + end + end + + describe 'one active crew per user validation' do + let(:crew1) { create(:crew) } + let(:crew2) { create(:crew) } + let(:user) { create(:user) } + + it 'prevents user from joining multiple active crews' do + create(:crew_membership, crew: crew1, user: user) + membership2 = build(:crew_membership, crew: crew2, user: user) + + expect(membership2).not_to be_valid + expect(membership2.errors[:user]).to include('can only be in one active crew') + end + + it 'allows user to join new crew after retiring from old one' do + membership1 = create(:crew_membership, crew: crew1, user: user) + membership1.retire! + + membership2 = build(:crew_membership, crew: crew2, user: user) + expect(membership2).to be_valid + end + end + + describe 'captain limit validation' do + let(:crew) { create(:crew) } + let(:captain_user) { create(:user) } + + before do + create(:crew_membership, :captain, crew: crew, user: captain_user) + end + + it 'prevents multiple captains' do + new_captain = build(:crew_membership, :captain, crew: crew) + + expect(new_captain).not_to be_valid + expect(new_captain.errors[:role]).to include('crew can only have one captain') + end + + it 'allows captain after previous captain retires' do + crew.crew_memberships.find_by(role: :captain).retire! + new_captain = build(:crew_membership, :captain, crew: crew) + + expect(new_captain).to be_valid + end + end + + describe 'vice captain limit validation' do + let(:crew) { create(:crew) } + + before do + create(:crew_membership, :captain, crew: crew) + 3.times { create(:crew_membership, :vice_captain, crew: crew) } + end + + it 'prevents more than 3 vice captains' do + fourth_vc = build(:crew_membership, :vice_captain, crew: crew) + + expect(fourth_vc).not_to be_valid + expect(fourth_vc.errors[:role]).to include('crew can only have up to 3 vice captains') + end + + it 'allows new vice captain after one retires' do + crew.crew_memberships.where(role: :vice_captain).first.retire! + new_vc = build(:crew_membership, :vice_captain, crew: crew) + + expect(new_vc).to be_valid + end + end + + describe '#retire!' do + let(:crew) { create(:crew) } + let(:user) { create(:user) } + let(:membership) { create(:crew_membership, :vice_captain, crew: crew, user: user) } + + it 'sets retired to true' do + membership.retire! + expect(membership.retired).to be true + end + + it 'sets retired_at timestamp' do + membership.retire! + expect(membership.retired_at).to be_within(1.second).of(Time.current) + end + + it 'demotes to member role' do + membership.retire! + expect(membership.role).to eq('member') + end + end +end diff --git a/spec/models/crew_spec.rb b/spec/models/crew_spec.rb new file mode 100644 index 0000000..1690278 --- /dev/null +++ b/spec/models/crew_spec.rb @@ -0,0 +1,134 @@ +require 'rails_helper' + +RSpec.describe Crew, type: :model do + describe 'associations' do + it { should have_many(:crew_memberships).dependent(:destroy) } + it { should have_many(:users).through(:crew_memberships) } + it { should have_many(:active_memberships) } + it { should have_many(:active_members).through(:active_memberships) } + end + + describe 'validations' do + it { should validate_presence_of(:name) } + it { should validate_length_of(:name).is_at_most(100) } + it { should validate_length_of(:gamertag).is_at_most(50) } + + context 'granblue_crew_id uniqueness' do + it 'validates uniqueness' do + crew1 = create(:crew, granblue_crew_id: 'ABC123') + crew2 = build(:crew, granblue_crew_id: 'ABC123') + expect(crew2).not_to be_valid + expect(crew2.errors[:granblue_crew_id]).to include('has already been taken') + end + + it 'allows nil values' do + create(:crew, granblue_crew_id: nil) + crew2 = build(:crew, granblue_crew_id: nil) + expect(crew2).to be_valid + end + end + end + + describe '#captain' do + let(:crew) { create(:crew) } + let(:captain_user) { create(:user) } + let(:member_user) { create(:user) } + + before do + create(:crew_membership, :captain, crew: crew, user: captain_user) + create(:crew_membership, crew: crew, user: member_user) + end + + it 'returns the captain user' do + expect(crew.captain).to eq(captain_user) + end + + it 'returns nil if no captain' do + crew.crew_memberships.find_by(role: :captain).update!(role: :member) + expect(crew.captain).to be_nil + end + + it 'does not return retired captains' do + crew.crew_memberships.find_by(role: :captain).retire! + expect(crew.captain).to be_nil + end + end + + describe '#vice_captains' do + let(:crew) { create(:crew) } + let(:captain_user) { create(:user) } + let(:vc1) { create(:user) } + let(:vc2) { create(:user) } + let(:member_user) { create(:user) } + + before do + create(:crew_membership, :captain, crew: crew, user: captain_user) + create(:crew_membership, :vice_captain, crew: crew, user: vc1) + create(:crew_membership, :vice_captain, crew: crew, user: vc2) + create(:crew_membership, crew: crew, user: member_user) + end + + it 'returns all vice captains' do + expect(crew.vice_captains).to contain_exactly(vc1, vc2) + end + + it 'does not include retired vice captains' do + crew.crew_memberships.find_by(user: vc1).retire! + expect(crew.vice_captains).to contain_exactly(vc2) + end + end + + describe '#officers' do + let(:crew) { create(:crew) } + let(:captain_user) { create(:user) } + let(:vc1) { create(:user) } + let(:member_user) { create(:user) } + + before do + create(:crew_membership, :captain, crew: crew, user: captain_user) + create(:crew_membership, :vice_captain, crew: crew, user: vc1) + create(:crew_membership, crew: crew, user: member_user) + end + + it 'returns captain and vice captains' do + expect(crew.officers).to contain_exactly(captain_user, vc1) + end + + it 'does not include regular members' do + expect(crew.officers).not_to include(member_user) + end + end + + describe '#member_count' do + let(:crew) { create(:crew) } + + before do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, crew: crew) + create(:crew_membership, :retired, crew: crew) + end + + it 'returns count of active members only' do + expect(crew.member_count).to eq(2) + end + end + + describe 'active_members scope' do + let(:crew) { create(:crew) } + let(:active_user) { create(:user) } + let(:retired_user) { create(:user) } + + before do + create(:crew_membership, crew: crew, user: active_user) + create(:crew_membership, :retired, crew: crew, user: retired_user) + end + + it 'returns only active members' do + expect(crew.active_members).to contain_exactly(active_user) + end + + it 'does not include retired members' do + expect(crew.active_members).not_to include(retired_user) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9a66e71..5ecc83d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -8,6 +8,9 @@ RSpec.describe User, type: :model do it { should have_many(:collection_weapons).dependent(:destroy) } it { should have_many(:collection_summons).dependent(:destroy) } it { should have_many(:collection_job_accessories).dependent(:destroy) } + it { should have_many(:crew_memberships).dependent(:destroy) } + it { should have_one(:active_crew_membership) } + it { should have_one(:crew).through(:active_crew_membership) } end describe 'validations' do @@ -72,8 +75,20 @@ RSpec.describe User, type: :model do end context 'when collection privacy is crew_only' do - it 'returns false for non-owner (crews not yet implemented)' do + let(:crew) { create(:crew) } + + it 'returns true for crew members' do + create(:crew_membership, :captain, crew: crew, user: owner) + create(:crew_membership, crew: crew, user: viewer) owner.update(collection_privacy: :crew_only) + + expect(owner.collection_viewable_by?(viewer)).to be true + end + + it 'returns false for non-crew members' do + create(:crew_membership, :captain, crew: crew, user: owner) + owner.update(collection_privacy: :crew_only) + expect(owner.collection_viewable_by?(viewer)).to be false end @@ -87,14 +102,112 @@ RSpec.describe User, type: :model do describe '#in_same_crew_as?' do let(:user1) { create(:user) } let(:user2) { create(:user) } + let(:crew) { create(:crew) } - it 'returns false (placeholder until crews are implemented)' do + it 'returns false when neither user is in a crew' do expect(user1.in_same_crew_as?(user2)).to be false end - it 'returns false when other_user is present' do + it 'returns false when only one user is in a crew' do + create(:crew_membership, :captain, crew: crew, user: user1) expect(user1.in_same_crew_as?(user2)).to be false end + + it 'returns true when both users are in the same crew' do + create(:crew_membership, :captain, crew: crew, user: user1) + create(:crew_membership, crew: crew, user: user2) + expect(user1.in_same_crew_as?(user2)).to be true + end + + it 'returns false when users are in different crews' do + crew2 = create(:crew) + create(:crew_membership, :captain, crew: crew, user: user1) + create(:crew_membership, :captain, crew: crew2, user: user2) + expect(user1.in_same_crew_as?(user2)).to be false + end + + it 'returns false when other_user is nil' do + create(:crew_membership, :captain, crew: crew, user: user1) + expect(user1.in_same_crew_as?(nil)).to be false + end + end + + describe '#crew_role' do + let(:user) { create(:user) } + let(:crew) { create(:crew) } + + it 'returns nil when user is not in a crew' do + expect(user.crew_role).to be_nil + end + + it 'returns captain when user is captain' do + create(:crew_membership, :captain, crew: crew, user: user) + expect(user.crew_role).to eq('captain') + end + + it 'returns vice_captain when user is vice captain' do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, :vice_captain, crew: crew, user: user) + expect(user.crew_role).to eq('vice_captain') + end + + it 'returns member when user is regular member' do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, crew: crew, user: user) + expect(user.crew_role).to eq('member') + end + end + + describe '#crew_officer?' do + let(:user) { create(:user) } + let(:crew) { create(:crew) } + + it 'returns false when user is not in a crew' do + expect(user.crew_officer?).to be false + end + + it 'returns true for captain' do + create(:crew_membership, :captain, crew: crew, user: user) + expect(user.crew_officer?).to be true + end + + it 'returns true for vice captain' do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, :vice_captain, crew: crew, user: user) + expect(user.crew_officer?).to be true + end + + it 'returns false for regular member' do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, crew: crew, user: user) + expect(user.crew_officer?).to be false + end + end + + describe '#crew_captain?' do + let(:user) { create(:user) } + let(:crew) { create(:crew) } + + it 'returns false when user is not in a crew' do + expect(user.crew_captain?).to be false + end + + it 'returns true for captain' do + create(:crew_membership, :captain, crew: crew, user: user) + expect(user.crew_captain?).to be true + end + + it 'returns false for vice captain' do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, :vice_captain, crew: crew, user: user) + expect(user.crew_captain?).to be false + end + + it 'returns false for regular member' do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, crew: crew, user: user) + expect(user.crew_captain?).to be false + end end describe 'collection associations behavior' do diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7aae24f..f54958f 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -39,6 +39,15 @@ rescue ActiveRecord::PendingMigrationError => e end RSpec.configure do |config| + # ----------------------------------------------------------------------------- + # Use transactional fixtures: + # + # Wrap each example in a database transaction that is rolled back after the + # example completes. This ensures test isolation without needing to truncate + # or delete data between tests. + # ----------------------------------------------------------------------------- + config.use_transactional_fixtures = true + # Disable ActiveRecord logging during tests for a cleaner test output. ActiveRecord::Base.logger = nil if Rails.env.test? diff --git a/spec/requests/crew_memberships_controller_spec.rb b/spec/requests/crew_memberships_controller_spec.rb new file mode 100644 index 0000000..22b872b --- /dev/null +++ b/spec/requests/crew_memberships_controller_spec.rb @@ -0,0 +1,144 @@ +require 'rails_helper' + +RSpec.describe 'Crew Memberships API', type: :request do + let(:captain) { create(:user) } + let(:vice_captain) { create(:user) } + let(:member) { create(:user) } + let(:crew) { create(:crew) } + + let(:captain_token) do + Doorkeeper::AccessToken.create!(resource_owner_id: captain.id, expires_in: 30.days, scopes: 'public') + end + let(:vc_token) do + Doorkeeper::AccessToken.create!(resource_owner_id: vice_captain.id, expires_in: 30.days, scopes: 'public') + end + let(:member_token) do + Doorkeeper::AccessToken.create!(resource_owner_id: member.id, expires_in: 30.days, scopes: 'public') + end + + let(:captain_headers) do + { 'Authorization' => "Bearer #{captain_token.token}", 'Content-Type' => 'application/json' } + end + let(:vc_headers) do + { 'Authorization' => "Bearer #{vc_token.token}", 'Content-Type' => 'application/json' } + end + let(:member_headers) do + { 'Authorization' => "Bearer #{member_token.token}", 'Content-Type' => 'application/json' } + end + + before do + create(:crew_membership, :captain, crew: crew, user: captain) + create(:crew_membership, :vice_captain, crew: crew, user: vice_captain) + create(:crew_membership, crew: crew, user: member) + end + + describe 'DELETE /api/v1/crews/:crew_id/memberships/:id' do + context 'as captain' do + it 'removes a member' do + membership = crew.crew_memberships.find_by(user: member) + + delete "/api/v1/crews/#{crew.id}/memberships/#{membership.id}", headers: captain_headers + + expect(response).to have_http_status(:no_content) + membership.reload + expect(membership.retired).to be true + end + + it 'cannot remove the captain' do + captain_membership = crew.crew_memberships.find_by(user: captain) + + delete "/api/v1/crews/#{crew.id}/memberships/#{captain_membership.id}", headers: captain_headers + + expect(response).to have_http_status(:unprocessable_entity) + json = JSON.parse(response.body) + expect(json['message']).to eq('Cannot remove the captain from the crew') + end + end + + context 'as vice captain' do + it 'removes a member' do + membership = crew.crew_memberships.find_by(user: member) + + delete "/api/v1/crews/#{crew.id}/memberships/#{membership.id}", headers: vc_headers + + expect(response).to have_http_status(:no_content) + end + end + + context 'as member' do + it 'returns unauthorized' do + other_member = create(:user) + create(:crew_membership, crew: crew, user: other_member) + membership = crew.crew_memberships.find_by(user: other_member) + + delete "/api/v1/crews/#{crew.id}/memberships/#{membership.id}", headers: member_headers + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'POST /api/v1/crews/:crew_id/memberships/:id/promote' do + it 'promotes a member to vice captain' do + membership = crew.crew_memberships.find_by(user: member) + + post "/api/v1/crews/#{crew.id}/memberships/#{membership.id}/promote", headers: captain_headers + + expect(response).to have_http_status(:ok) + json = JSON.parse(response.body) + expect(json['membership']['role']).to eq('vice_captain') + end + + it 'returns error when vice captain limit reached' do + # Add 2 more vice captains (already have 1) + 2.times do + vc_user = create(:user) + create(:crew_membership, :vice_captain, crew: crew, user: vc_user) + end + + membership = crew.crew_memberships.find_by(user: member) + + post "/api/v1/crews/#{crew.id}/memberships/#{membership.id}/promote", headers: captain_headers + + expect(response).to have_http_status(:unprocessable_entity) + json = JSON.parse(response.body) + expect(json['message']).to eq('Crew can only have up to 3 vice captains') + end + + it 'requires captain role' do + membership = crew.crew_memberships.find_by(user: member) + + post "/api/v1/crews/#{crew.id}/memberships/#{membership.id}/promote", headers: vc_headers + + expect(response).to have_http_status(:unauthorized) + end + end + + describe 'POST /api/v1/crews/:crew_id/memberships/:id/demote' do + it 'demotes a vice captain to member' do + membership = crew.crew_memberships.find_by(user: vice_captain) + + post "/api/v1/crews/#{crew.id}/memberships/#{membership.id}/demote", headers: captain_headers + + expect(response).to have_http_status(:ok) + json = JSON.parse(response.body) + expect(json['membership']['role']).to eq('member') + end + + it 'cannot demote the captain' do + captain_membership = crew.crew_memberships.find_by(user: captain) + + post "/api/v1/crews/#{crew.id}/memberships/#{captain_membership.id}/demote", headers: captain_headers + + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'requires captain role' do + membership = crew.crew_memberships.find_by(user: member) + + post "/api/v1/crews/#{crew.id}/memberships/#{membership.id}/demote", headers: vc_headers + + expect(response).to have_http_status(:unauthorized) + end + end +end diff --git a/spec/requests/crews_controller_spec.rb b/spec/requests/crews_controller_spec.rb new file mode 100644 index 0000000..c795dd3 --- /dev/null +++ b/spec/requests/crews_controller_spec.rb @@ -0,0 +1,253 @@ +require 'rails_helper' + +RSpec.describe 'Crews API', type: :request do + let(:user) { create(:user) } + let(:other_user) { create(:user) } + let(:access_token) do + Doorkeeper::AccessToken.create!(resource_owner_id: user.id, expires_in: 30.days, scopes: 'public') + end + let(:other_access_token) do + Doorkeeper::AccessToken.create!(resource_owner_id: other_user.id, expires_in: 30.days, scopes: 'public') + end + let(:headers) do + { 'Authorization' => "Bearer #{access_token.token}", 'Content-Type' => 'application/json' } + end + let(:other_headers) do + { 'Authorization' => "Bearer #{other_access_token.token}", 'Content-Type' => 'application/json' } + end + + describe 'POST /api/v1/crews' do + let(:valid_params) do + { + crew: { + name: 'Test Crew', + gamertag: 'TEST', + granblue_crew_id: '12345678', + description: 'A test crew' + } + } + end + + it 'creates a crew and makes user captain' do + post '/api/v1/crews', params: valid_params.to_json, headers: headers + + expect(response).to have_http_status(:created) + json = JSON.parse(response.body) + expect(json['crew']['name']).to eq('Test Crew') + expect(json['crew']['gamertag']).to eq('TEST') + + user.reload + expect(user.crew).to be_present + expect(user.crew_captain?).to be true + end + + it 'returns error if user already in a crew' do + crew = create(:crew) + create(:crew_membership, :captain, crew: crew, user: user) + + post '/api/v1/crews', params: valid_params.to_json, headers: headers + + expect(response).to have_http_status(:unprocessable_entity) + json = JSON.parse(response.body) + expect(json['message']).to eq('You are already in a crew') + end + + it 'returns unauthorized without authentication' do + post '/api/v1/crews', params: valid_params.to_json + + expect(response).to have_http_status(:unauthorized) + end + + it 'validates crew name presence' do + invalid_params = { crew: { name: '' } } + post '/api/v1/crews', params: invalid_params.to_json, headers: headers + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + describe 'GET /api/v1/crew' do + let(:crew) { create(:crew) } + + before do + create(:crew_membership, :captain, crew: crew, user: user) + end + + it 'returns the current user crew' do + get '/api/v1/crew', headers: headers + + expect(response).to have_http_status(:ok) + json = JSON.parse(response.body) + expect(json['crew']['name']).to eq(crew.name) + end + + it 'returns 404 if user has no crew' do + user.active_crew_membership.retire! + + get '/api/v1/crew', headers: headers + + expect(response).to have_http_status(:not_found) + end + + it 'returns unauthorized without authentication' do + get '/api/v1/crew' + + expect(response).to have_http_status(:unauthorized) + end + end + + describe 'PUT /api/v1/crew' do + let(:crew) { create(:crew) } + + context 'as captain' do + before do + create(:crew_membership, :captain, crew: crew, user: user) + end + + it 'updates the crew' do + put '/api/v1/crew', params: { crew: { name: 'New Name' } }.to_json, headers: headers + + expect(response).to have_http_status(:ok) + json = JSON.parse(response.body) + expect(json['crew']['name']).to eq('New Name') + end + end + + context 'as vice captain' do + before do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, :vice_captain, crew: crew, user: user) + end + + it 'updates the crew' do + put '/api/v1/crew', params: { crew: { name: 'New Name' } }.to_json, headers: headers + + expect(response).to have_http_status(:ok) + end + end + + context 'as member' do + before do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, crew: crew, user: user) + end + + it 'returns unauthorized' do + put '/api/v1/crew', params: { crew: { name: 'New Name' } }.to_json, headers: headers + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'GET /api/v1/crew/members' do + let(:crew) { create(:crew) } + let(:captain) { create(:user) } + let(:member) { create(:user) } + + before do + create(:crew_membership, :captain, crew: crew, user: captain) + create(:crew_membership, crew: crew, user: user) + create(:crew_membership, crew: crew, user: member) + end + + it 'returns all active members' do + get '/api/v1/crew/members', headers: headers + + expect(response).to have_http_status(:ok) + json = JSON.parse(response.body) + expect(json['members'].length).to eq(3) + end + + it 'does not include retired members' do + crew.crew_memberships.find_by(user: member).retire! + + get '/api/v1/crew/members', headers: headers + + expect(response).to have_http_status(:ok) + json = JSON.parse(response.body) + expect(json['members'].length).to eq(2) + end + end + + describe 'POST /api/v1/crew/leave' do + let(:crew) { create(:crew) } + + context 'as regular member' do + before do + create(:crew_membership, :captain, crew: crew) + create(:crew_membership, crew: crew, user: user) + end + + it 'retires the membership' do + post '/api/v1/crew/leave', headers: headers + + expect(response).to have_http_status(:no_content) + user.reload + expect(user.crew).to be_nil + end + end + + context 'as captain' do + before do + create(:crew_membership, :captain, crew: crew, user: user) + end + + it 'returns error' do + post '/api/v1/crew/leave', headers: headers + + expect(response).to have_http_status(:unprocessable_entity) + json = JSON.parse(response.body) + expect(json['message']).to eq('Captain must transfer ownership before leaving') + end + end + + context 'when not in crew' do + it 'returns error' do + post '/api/v1/crew/leave', headers: headers + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe 'POST /api/v1/crews/:id/transfer_captain' do + let(:crew) { create(:crew) } + let(:vice_captain) { create(:user) } + + before do + create(:crew_membership, :captain, crew: crew, user: user) + create(:crew_membership, :vice_captain, crew: crew, user: vice_captain) + end + + it 'transfers captain role to another member' do + post "/api/v1/crews/#{crew.id}/transfer_captain", + params: { user_id: vice_captain.id }.to_json, + headers: headers + + expect(response).to have_http_status(:ok) + user.reload + vice_captain.reload + expect(user.crew_role).to eq('vice_captain') + expect(vice_captain.crew_role).to eq('captain') + end + + it 'returns error if target user is not in crew' do + post "/api/v1/crews/#{crew.id}/transfer_captain", + params: { user_id: other_user.id }.to_json, + headers: headers + + expect(response).to have_http_status(:not_found) + end + + it 'requires captain role' do + create(:crew_membership, crew: crew, user: other_user) + + post "/api/v1/crews/#{crew.id}/transfer_captain", + params: { user_id: vice_captain.id }.to_json, + headers: other_headers + + expect(response).to have_http_status(:unauthorized) + end + end +end