From 2325fd74dfa4af8891df9ee3edf6ccc19d45730f Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Sun, 30 Aug 2026 20:20:51 +0200 Subject: [PATCH 1/3] feat: create Students/Coaches groups on chapter creation ChapterCreationService wraps chapter + groups creation in a single transaction, so a new chapter always has its required Students and Coaches groups (issue #2823). Missing groups meant no subscription buttons on the chapter page and workshop invitations silently sent to zero members. - Admin::ChaptersController#create authorizes the unsaved chapter before calling the service, so a denied request persists nothing - The service rolls back the chapter if group creation fails - Service spec proves the rollback with a real group validation failure; controller spec covers the denial path; feature spec covers group creation and the invalid-form failure branch --- .rubocop_todo.yml | 1 + app/controllers/admin/chapters_controller.rb | 7 ++- app/services/chapter_creation_service.rb | 17 ++++++++ .../admin/chapters_controller_spec.rb | 23 ++++++++++ spec/features/admin/chapters_spec.rb | 17 ++++++++ .../services/chapter_creation_service_spec.rb | 43 +++++++++++++++++++ 6 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 app/services/chapter_creation_service.rb create mode 100644 spec/services/chapter_creation_service_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6b9cffc92..bb4a0ee90 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -40,6 +40,7 @@ Metrics/AbcSize: # Configuration parameters: CountComments, Max, CountAsOne. Metrics/ClassLength: Exclude: + - 'app/controllers/admin/chapters_controller.rb' - 'app/controllers/admin/workshops_controller.rb' - 'app/controllers/application_controller.rb' - 'app/controllers/events_controller.rb' diff --git a/app/controllers/admin/chapters_controller.rb b/app/controllers/admin/chapters_controller.rb index 06d0f8127..4c472ae29 100644 --- a/app/controllers/admin/chapters_controller.rb +++ b/app/controllers/admin/chapters_controller.rb @@ -9,9 +9,12 @@ def new def create @chapter = Chapter.new(chapter_params) - authorize(@chapter) + authorize @chapter + + result = ChapterCreationService.call(chapter_params) + @chapter = result.chapter - if @chapter.save + if result.success flash[:notice] = "Chapter #{@chapter.name} has been successfully created" redirect_to [:admin, @chapter] else diff --git a/app/services/chapter_creation_service.rb b/app/services/chapter_creation_service.rb new file mode 100644 index 000000000..fe171984c --- /dev/null +++ b/app/services/chapter_creation_service.rb @@ -0,0 +1,17 @@ +class ChapterCreationService + Result = Struct.new(:chapter, :success, :errors, keyword_init: true) + + def self.call(params) + chapter = Chapter.new(params) + + ActiveRecord::Base.transaction do + chapter.save! + chapter.groups.create!(name: 'Students') + chapter.groups.create!(name: 'Coaches') + end + + Result.new(chapter: chapter, success: true) + rescue ActiveRecord::RecordInvalid => e + Result.new(chapter: chapter, success: false, errors: e.message) + end +end diff --git a/spec/controllers/admin/chapters_controller_spec.rb b/spec/controllers/admin/chapters_controller_spec.rb index 01bdea3a3..ea641430f 100644 --- a/spec/controllers/admin/chapters_controller_spec.rb +++ b/spec/controllers/admin/chapters_controller_spec.rb @@ -5,6 +5,29 @@ login_as_admin(admin) end + describe '#create' do + context 'when a chapter-scoped organiser creates a chapter' do + let(:organiser) { Fabricate(:chapter_organiser) } + + before do + login(organiser) + end + + it 'is denied before anything is persisted' do + expect do + post :create, params: { chapter: { + name: 'codebar Brighton', + email: 'brighton@codebar.io', + city: 'Brighton', + time_zone: 'London' + } } + end.not_to(change { [Chapter.count, Group.count] }) + + expect(response).to redirect_to(root_path) + end + end + end + describe '#status' do it 'renders successfully with default 6 months' do get :status diff --git a/spec/features/admin/chapters_spec.rb b/spec/features/admin/chapters_spec.rb index 4f6dac954..0335024c5 100644 --- a/spec/features/admin/chapters_spec.rb +++ b/spec/features/admin/chapters_spec.rb @@ -27,6 +27,23 @@ click_on 'Create chapter' expect(page).to have_text('Chapter codebar Brighton has been successfully created') + + chapter = Chapter.find_by(name: 'codebar Brighton') + expect(chapter.groups.pluck(:name)).to match_array(%w[Students Coaches]) + end + + scenario 'an admin submitting an invalid form sees validation errors and no chapter is created' do + visit new_admin_chapter_path + + fill_in 'Name', with: '' + fill_in 'Email', with: '' + fill_in 'City', with: 'Brighton' + + click_on 'Create chapter' + + expect(page).to have_text("Name can't be blank") + expect(page).to have_text("Email can't be blank") + expect(Chapter.count).to eq(0) end end diff --git a/spec/services/chapter_creation_service_spec.rb b/spec/services/chapter_creation_service_spec.rb new file mode 100644 index 000000000..c9a56ab31 --- /dev/null +++ b/spec/services/chapter_creation_service_spec.rb @@ -0,0 +1,43 @@ +RSpec.describe ChapterCreationService do + let(:valid_params) do + { + name: 'codebar Brighton', + email: 'brighton@codebar.io', + city: 'Brighton', + time_zone: 'London' + } + end + + describe '.call' do + it 'creates chapter with Students and Coaches groups' do + result = described_class.call(valid_params) + + expect(result.success).to be true + expect(result.chapter.persisted?).to be true + expect(result.chapter.groups.pluck(:name)).to match_array(%w[Students Coaches]) + end + + it 'fails when chapter params are invalid' do + invalid_params = valid_params.merge(name: '') + result = described_class.call(invalid_params) + + expect(result.success).to be false + expect(result.errors).to be_present + expect(Chapter.where(name: '')).not_to exist + end + + it 'rolls back chapter if groups fail' do + # 'bogus' is not in Group::NAMES, so the group fails its real + # inclusion validation and save! raises RecordInvalid inside the + # service's transaction. + invalid_group = Group.new(name: 'bogus') + allow(Group).to receive(:new).and_return(invalid_group) + + result = described_class.call(valid_params) + + expect(result.success).to be false + expect(result.errors).to be_present + expect(Chapter.where(name: valid_params[:name])).not_to exist + end + end +end From 4686beed086db07493b02704b351cc4dfe14192a Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Sun, 30 Aug 2026 20:43:01 +0200 Subject: [PATCH 2/3] refactor: remove manual group creation UI Now that every chapter gets its Students and Coaches groups automatically on creation, the manual group creation flow is dead code (follow-up to #2823). - Remove Admin::GroupsController#new/#create and the new.html.haml view (also drops the last Chosen-enhanced admin