chore: fix code review issues

Author: davidmillen50

app/controllers/concerns/member_concerns.rb

@@ -28,5 +28,19 @@ def suppress_notices
     def set_member
       @member = current_user
     end
+
+    def how_you_found_us_selections
+      how_found = Array(member_params[:how_you_found_us]).reject(&:blank?)
+      other_reason = member_params[:how_you_found_us_other_reason]
+
+      how_found << other_reason if other_reason.present?
+      how_found.uniq!
+
+      how_found
+    end
+
+    def member_params_without_how_you_found_us_other_reason
+      member_params.to_h.except(:how_you_found_us_other_reason)
+    end
   end
 end

app/controllers/member/details_controller.rb

@@ -13,24 +13,18 @@ def edit
   end
 
   def update
-    how_found = Array(params.dig(:member, :how_you_found_us)).reject(&:blank?)
-    other_reason = params.dig(:member, :how_you_found_us_other_reason)
-
-    how_found << other_reason if other_reason.present?
-    how_found.uniq!
-
-    if how_found.blank?
-      @member.assign_attributes(member_params.to_h.except(:how_you_found_us_other_reason))
+    if how_you_found_us_selections.blank?
+      @member.assign_attributes(member_params_without_how_you_found_us_other_reason)
       @member.errors.add(:how_you_found_us, 'You must select at least one option')
       return render :edit
     end
 
-    attrs = member_params.to_h.except(:how_you_found_us_other_reason)
-    attrs[:how_you_found_us] = how_found
+    attrs = member_params_without_how_you_found_us_other_reason
+    attrs[:how_you_found_us] = how_you_found_us_selections
 
     return render :edit unless @member.update(attrs)
 
     member_params[:newsletter] ? subscribe_to_newsletter(@member) : unsubscribe_from_newsletter(@member)
     redirect_to step2_member_path
   end
-end
+end

app/controllers/members_controller.rb

@@ -9,7 +9,6 @@ class MembersController < ApplicationController
 
   def new
     @page_title = 'Sign up'
-
   end
 
   def edit; end

app/models/member.rb

@@ -1,6 +1,13 @@
 class Member < ApplicationRecord
   include Permissions
 
+  HOW_YOU_FOUND_US_OPTIONS = [
+    'From a friend',
+    'Search engine (Google etc.)',
+    'Social Media',
+    "One of Codebar's hosts or partners"
+  ].freeze
+
   has_many :attendance_warnings
   has_many :bans
   has_many :eligibility_inquiries
@@ -45,7 +52,7 @@ class Member < ApplicationRecord
 
   acts_as_taggable_on :skills
 
-  attr_accessor :attendance, :newsletter
+  attr_accessor :attendance, :newsletter, :how_you_found_us_other_reason
 
   def banned?
     bans.active.present? || bans.permanent.present?

app/views/member/details/edit.html.haml

@@ -25,18 +25,25 @@
             %ul
               - @member.errors.full_messages.each do |msg|
                 %li= msg
-        - if @member.errors[:how_you_found_us].any?
+        - if @member.errors[:how_you_found_us]&.any?
           %span.text-danger= @member.errors[:how_you_found_us].first
         .col-12.mb-3
-          %label{ for: 'how_you_found_us' }= t('member.details.edit.how_you_found_us')
-          %span *
-          - options = ['From a friend', 'Search engine (Google etc.)', 'Social Media', "One of Codebar's hosts or partners"]
-          - options.each do |option|
-            .form-check
-              = check_box_tag 'member[how_you_found_us][]', option, false, id: "how_you_found_us_#{option.parameterize}", class: 'form-check-input'
-              = label_tag "how_you_found_us_#{option.parameterize}", option, class: 'form-check-label', style: 'margin-left: 8px;'
-          = label_tag :how_you_found_us_other_reason, t('member.details.edit.how_you_found_us_other_reason'), class: 'my-1'
-          = text_field_tag 'member[how_you_found_us_other_reason]', nil, placeholder: "Please specify how you heard about us", class: 'form-control w-100'
+          %fieldset
+            %legend= t('member.details.edit.how_you_found_us')
+            %span *
+
+            = f.input :how_you_found_us,
+                      as: :check_boxes,
+                      collection: Member::HOW_YOU_FOUND_US_OPTIONS,
+                      item_wrapper_class: 'form-check',
+                      label_item: true,
+                      input_html: { class: 'form-check-input' },
+                      label_html: { class: 'form-check-label', style: 'margin-left: 8px;' }
+
+          = f.input :how_you_found_us_other_reason,
+                    label: t('member.details.edit.how_you_found_us_other_reason'),
+                    placeholder: 'Please specify how you heard about us',
+                    input_html: { class: 'form-control w-100' }
         = f.input :newsletter, as: :boolean, checked_value: true, unchecked_value: false
         .text-right.mb-4
           = hidden_field_tag :next_page, step2_member_path(member_type: @type)

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2025_08_20_145012) do
+ActiveRecord::Schema[7.1].define(version: 2025_08_23_151717) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 

spec/controllers/member/details_controller_spec.rb

@@ -1,9 +1,10 @@
-RSpec.describe Member::DetailsController, type: :controller do
+RSpec.describe Member::DetailsController do
   render_views
   let(:member) { Fabricate(:member) }
 
   before do
     allow(controller).to receive(:current_user).and_return(member)
+    allow_any_instance_of(Services::MailingList).to receive(:subscribe).and_return(true)
   end
 
   describe 'PATCH #update' do

spec/features/member_joining_spec.rb

@@ -15,7 +15,7 @@
   end
 
   scenario 'A visitor must fill in all mandatory fields in order to sign up' do
-    member = Fabricate(:member, name: nil, surname: nil, email: nil, about_you: nil)
+    member = Fabricate(:member, name: nil, surname: nil, email: nil, about_you: nil, how_you_found_us: [])
     member.update(can_log_in: true)
     login member
 
@@ -44,7 +44,7 @@
     check 'Vegan'
     check 'Other'
     fill_in 'Other dietary restrictions', with: 'peanut allergy'
-    find('#how_you_found_us_from-a-friend').click
+    find('#member_how_you_found_us_from_a_friend').click
     fill_in 'member_how_you_found_us_other_reason', with: 'found on a poster', id: true
     click_on 'Next'