Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions app/components/chapter_picker_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<%= text_field_tag @name, @selected,
list: datalist_id,
placeholder: @placeholder,
class: 'form-control',
autocomplete: 'off' %>

<%= tag.datalist id: datalist_id do %>
<% @chapters.each do |chapter| %>
<%= tag.option chapter.name, value: chapter.name %>
<% end %>
<% end %>
15 changes: 15 additions & 0 deletions app/components/chapter_picker_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

class ChapterPickerComponent < ViewComponent::Base

@olleolleolle olleolleolle Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: We should have an ApplicationComponent base class, I think, for an easier control point. It won't change anything right now, but it's neat to have prepared.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm new to using ViewComponents, so this might be a silly question. What would this ApplicationComponent give us?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like ApplicationController or ApplicationRecord, a single base class, where you can do shared things like "ah, always use this custom layout", to take an example from ApplicationController.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, let's install that thing whenever we need to.

def initialize(name:, chapters:, selected: nil, placeholder: 'Select a chapter')
super()
@name = name
@chapters = chapters
@selected = selected
@placeholder = placeholder
end

def datalist_id
"#{@name.parameterize}-options"
end
end
14 changes: 10 additions & 4 deletions app/controllers/admin/workshops_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,15 @@ def new
end

def create
resolve_chapter_name_to_id
@workshop = Workshop.new(workshop_params)
authorize(@workshop)

if workshop_type_valid? && @workshop.save
assign_organisers_or_default
assign_host(host_id)

redirect_to admin_workshop_path(@workshop), notice: I18n.t('admin.messages.workshop.created')
else
flash[:warning] = @workshop.errors.full_messages
render 'new'
flash[:warning] = @workshop.errors.full_messages; render 'new'
end
end

Expand Down Expand Up @@ -166,6 +164,14 @@ def destroy_host

private

def resolve_chapter_name_to_id
chapter_value = params.dig(:workshop, :chapter_id)
return if chapter_value.blank? || chapter_value.match?(/\A\d+\z/)

chapter = Chapter.find_by('LOWER(name) = LOWER(?)', chapter_value.strip)
params[:workshop][:chapter_id] = chapter&.id
end

def paginate_matching_invitations(query)
eligible = @workshop.invitations
.joins(:member)
Expand Down
13 changes: 10 additions & 3 deletions app/queries/sponsors_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,15 @@ def by_name
end

def by_chapter
if chapter.present?
@sponsors = sponsors.joins(:workshops).where('workshops.chapter_id' => chapter).group('sponsors.id')
end
return if chapter.blank?

chapter_id = chapter.to_s.match?(/\A\d+\z/) ? chapter : lookup_chapter_id
return unless chapter_id

@sponsors = sponsors.joins(:workshops).where('workshops.chapter_id' => chapter_id).group('sponsors.id')
end

def lookup_chapter_id
Chapter.find_by('LOWER(name) = LOWER(?)', chapter.strip)&.id
end
end
8 changes: 4 additions & 4 deletions app/views/admin/sponsors/index.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

.col-12
.row.row-cols-md-auto.align-items-center
= simple_form_for @sponsors_search, url: admin_sponsors_path, method: :get, wrapper: :inline_form, html: { class: 'row row-cols-1 row-cols-md-auto align-items-center' } do |f|
= f.collection_select :chapter, @chapters, :id, :name, { include_blank: true, prompt: 'Select a chapter' }, { class: 'chosen-select'}
= f.input :name, required: false, label: false, placeholder: 'Filter by sponsor name', input_html: { class: 'my-2 my-md-0' }
= f.button :button, 'Filter', class: 'btn btn-primary'
= form_with model: @sponsors_search, url: admin_sponsors_path, method: :get, class: 'row row-cols-1 row-cols-md-auto align-items-center' do |f|
= render(ChapterPickerComponent.new(name: 'sponsors_search[chapter]', chapters: @chapters, selected: @sponsors_search.chapter, placeholder: 'Filter by chapter'))
= f.text_field :name, placeholder: 'Filter by sponsor name', class: 'form-control w-auto my-2 my-md-0'
= f.button 'Filter', class: 'btn btn-primary'
= link_to 'Reset form', admin_sponsors_path

= render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'sponsor' }
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/workshops/_shared_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
= f.hidden_field :chapter_id
- else
.col-12
= f.association :chapter, as: :select, collection: Chapter.available_to_user(current_user)
= render(ChapterPickerComponent.new(name: 'workshop[chapter_id]', chapters: Chapter.available_to_user(current_user), placeholder: 'Select a chapter'))
.col-12
= f.input :local_date, label: 'Date', required: true, input_html: { value: @workshop.date_and_time.try(:strftime, '%Y-%m-%d'), type: :date }
.col-12.col-md-6
Expand Down
40 changes: 40 additions & 0 deletions spec/components/chapter_picker_component_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
require 'rails_helper'
require 'view_component/test_helpers'

RSpec.describe ChapterPickerComponent do
include ViewComponent::TestHelpers

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: we should make it so that each spec/components/ test get these by dint of having derived RSpec Metadata about their "type".

Or, check whether that's already true, and these helpers are already included.


let(:chapters) { Fabricate.times(3, :chapter) }

it 'renders a text input with datalist attributes' do
render_inline described_class.new(name: 'sponsors_search[chapter]', chapters: chapters, placeholder: 'Filter by chapter')

expect(page).to have_field('sponsors_search[chapter]')
input = page.find('input')
expect(input['placeholder']).to eq('Filter by chapter')
expect(input['autocomplete']).to eq('off')
expect(input['class']).to include('form-control')
end

it 'renders a datalist with chapter names' do
render_inline described_class.new(name: 'sponsors_search[chapter]', chapters: chapters)

expect(page).to have_css('datalist#sponsors_search-chapter-options')
chapters.each do |chapter|
expect(page).to have_css("option[value='#{chapter.name}']")
end
end

it 'sanitises bracket characters in the datalist id' do
render_inline described_class.new(name: 'workshop[chapter_id]', chapters: chapters)

expect(page).to have_css('datalist#workshop-chapter_id-options')
end

it 'pre-fills the input when selected value is provided' do
render_inline described_class.new(name: 'sponsors_search[chapter]', chapters: chapters, selected: 'London')

input = page.find('input')
expect(input['value']).to eq('London')
end
end
20 changes: 20 additions & 0 deletions spec/features/admin/filtering_sponsors_list_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,25 @@
expect(page).to have_text(sponsors.first.name)
end
end

describe 'when filtering by chapter' do
let!(:chapter) { Fabricate(:chapter, name: 'London') }
let!(:workshop) { Fabricate(:workshop_no_sponsor, chapter: chapter) }
let!(:matching_sponsor) { Fabricate(:sponsor) }

before do
Fabricate(:workshop_sponsor, workshop: workshop, sponsor: matching_sponsor)
Fabricate(:sponsor)
visit admin_sponsors_path
end

scenario 'only sponsors for that chapter are displayed' do
fill_in 'sponsors_search[chapter]', with: 'London'
click_on 'Filter'

expect(page).to have_css('.sponsor', count: 1)
expect(page).to have_text(matching_sponsor.name)
end
end
end
end
12 changes: 6 additions & 6 deletions spec/features/admin/workshops_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
scenario 'requires a host and a start and end datetime to be set' do
visit new_admin_workshop_path

select chapter.name
fill_in 'workshop[chapter_id]', with: chapter.id
fill_in 'Date', with: Date.current
fill_in 'Begins at', with: '11:30'
fill_in 'Ends at', with: '12:45'
Expand Down Expand Up @@ -82,7 +82,7 @@
scenario 'must have a host set' do
visit new_admin_workshop_path

select chapter.name
fill_in 'workshop[chapter_id]', with: chapter.id
fill_in 'Date', with: Date.current
fill_in 'Begins at', with: '11:30'

Expand All @@ -108,7 +108,7 @@
chapter = Fabricate(:chapter, time_zone: 'Berlin')
visit new_admin_workshop_path

select chapter.name
fill_in 'workshop[chapter_id]', with: chapter.id
fill_in 'Date', with: Date.current
fill_in 'Begins at', with: '18:30'
fill_in 'Ends at', with: '20:45'
Expand All @@ -130,7 +130,7 @@

check 'Virtual'

select chapter.name
fill_in 'workshop[chapter_id]', with: chapter.id
fill_in 'Date', with: Date.current
fill_in 'Begins at', with: '11:30'

Expand All @@ -151,7 +151,7 @@
fill_in 'Student spaces', with: '10'
fill_in 'Coach spaces', with: '5'

select chapter.name
fill_in 'workshop[chapter_id]', with: chapter.id
fill_in 'Date', with: Date.current
fill_in 'Begins at', with: '11:30'
fill_in 'Ends at', with: '14:30'
Expand Down Expand Up @@ -211,7 +211,7 @@

expect(page).to have_css('h1', text: 'New Workshop')
expect(page).to have_title('New Workshop')
expect(page).to have_select('workshop_chapter_id')
expect(page).to have_field('workshop[chapter_id]')
expect(page).to have_no_select('workshop_organisers')
end
end
Expand Down
23 changes: 23 additions & 0 deletions spec/queries/sponsors_search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,29 @@
expect(results).to contain_exactly(matching)
end

it 'filters by chapter name' do
chapter = Fabricate(:chapter, name: 'London')
matching = Fabricate(:sponsor)
Fabricate(:workshop_sponsor, workshop: Fabricate(:workshop_no_sponsor, chapter: chapter), sponsor: matching)
Fabricate(:sponsor)

results = described_class.new(name: nil, chapter: 'London').call

expect(results).to contain_exactly(matching)
end

it 'is case insensitive when filtering by chapter name' do
chapter = Fabricate(:chapter, name: 'London')
matching = Fabricate(:sponsor)
Fabricate(:workshop_sponsor, workshop: Fabricate(:workshop_no_sponsor, chapter: chapter), sponsor: matching)

results = described_class.new(name: nil, chapter: 'london').call
expect(results).to contain_exactly(matching)

results = described_class.new(name: nil, chapter: 'LONDON').call
expect(results).to contain_exactly(matching)
end

it 'filters by chapter' do
chapter = Fabricate(:chapter)
matching = Fabricate(:sponsor)
Expand Down