From 46d00f2b073762974b2ecb62d24f401996627bd9 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Wed, 27 Aug 2025 13:08:41 +0100 Subject: [PATCH 1/7] chore: fix merge conflict in schema.rb --- app/controllers/concerns/member_concerns.rb | 13 ++- app/controllers/member/details_controller.rb | 22 ++++- app/controllers/members_controller.rb | 5 +- app/models/member.rb | 7 ++ app/views/member/details/edit.html.haml | 15 ++++ config/locales/en.yml | 2 + ...0823151717_add_how_you_found_us_options.rb | 5 ++ db/schema.rb | 1 + .../member/details_controller_spec.rb | 84 +++++++++++++++++++ spec/fabricators/member_fabricator.rb | 1 + spec/models/member_spec.rb | 22 +++++ 11 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20250823151717_add_how_you_found_us_options.rb create mode 100644 spec/controllers/member/details_controller_spec.rb diff --git a/app/controllers/concerns/member_concerns.rb b/app/controllers/concerns/member_concerns.rb index 8c75a43f6..f059608a5 100644 --- a/app/controllers/concerns/member_concerns.rb +++ b/app/controllers/concerns/member_concerns.rb @@ -9,8 +9,17 @@ module InstanceMethods private def member_params - params.require(:member).permit( - :pronouns, :name, :surname, :email, :mobile, :about_you, :skill_list, :newsletter + params.fetch(:member, {}).permit( + :pronouns, + :name, + :surname, + :email, + :mobile, + :about_you, + :skill_list, + :newsletter, + :other_reason, + how_you_found_us: [] ) end diff --git a/app/controllers/member/details_controller.rb b/app/controllers/member/details_controller.rb index 403e460d2..eb3f82caf 100644 --- a/app/controllers/member/details_controller.rb +++ b/app/controllers/member/details_controller.rb @@ -8,13 +8,31 @@ class Member::DetailsController < ApplicationController def edit accept_terms - flash[notice] = I18n.t('notifications.signing_up') @member.newsletter ||= true end def update - return render :edit unless @member.update(member_params) + attrs = member_params.to_h + + how_found = params.dig(:member, :how_you_found_us) + attrs[:how_you_found_us] = + if how_found.is_a?(Array) + how_found + elsif how_found.blank? + [] + else + [how_found] + end + + if params[:other_reason].present? + attrs[:how_you_found_us] << params[:other_reason] + end + + attrs[:how_you_found_us].uniq! + attrs[:how_you_found_us].reject!(&:blank?) + + return render :edit unless @member.update(attrs) member_params[:newsletter] ? subscribe_to_newsletter(@member) : unsubscribe_from_newsletter(@member) redirect_to step2_member_path diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index e08932e12..72f7e2286 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -9,9 +9,12 @@ class MembersController < ApplicationController def new @page_title = 'Sign up' + end - def edit; end + def edit + binding.pry + end def step2 @type = cookies[:member_type] diff --git a/app/models/member.rb b/app/models/member.rb index 59ef928aa..192de9da2 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -19,6 +19,7 @@ class Member < ApplicationRecord validates :name, :surname, :email, :about_you, presence: true, if: :can_log_in? validates :email, uniqueness: true validates :about_you, length: { maximum: 255 } + validate :how_you_found_us_must_have_at_least_one_option scope :accepted_toc, -> { where.not(accepted_toc_at: nil) } scope :order_by_email, -> { order(:email) } @@ -125,6 +126,12 @@ def self.find_members_by_name(name) private + def how_you_found_us_must_have_at_least_one_option + if how_you_found_us.blank? || how_you_found_us.reject(&:blank?).empty? + errors.add(:how_you_found_us, 'You must select at least one option') + end + end + def invitations_on(date) workshop_invitations .joins(:workshop) diff --git a/app/views/member/details/edit.html.haml b/app/views/member/details/edit.html.haml index 60bc0fbec..ff84b4e8f 100644 --- a/app/views/member/details/edit.html.haml +++ b/app/views/member/details/edit.html.haml @@ -15,6 +15,21 @@ = f.input :about_you, as: :text, label: t('member.details.edit.coach.about_you'), input_html: { rows: 3 }, required: true - else = f.input :about_you, as: :text, label: t('member.details.edit.student.about_you'), input_html: { rows: 3 }, required: true + - if @member.errors.any? + #error_explanation + %h2= "#{pluralize(@member.errors.count, 'error')} prohibited this member from being saved:" + %ul + - @member.errors.full_messages.each do |msg| + %li= msg + .col-12 + %label{ for: 'how_you_found_us' }= t('member.details.edit.how_you_found_us') + - options = ['From a friend', 'Search engine (Google etc.)', 'Social Media', "One of Codebar's hosts or partners", 'Other reason'] + - 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 :other_reason, t('member.details.edit.other_reason') + = text_field_tag :other_reason, nil, placeholder: "Please specify", style: 'width: 500px;' = 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) diff --git a/config/locales/en.yml b/config/locales/en.yml index 7f3b81ff3..8b501a352 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -442,6 +442,8 @@ en: edit: title: Almost there... summary: We need some more details from you to finish creating your account. We use these to help run our events. + how_you_found_us: "How did you find out about us?" + other_reason: "Please specify (if Other)" coach: about_you: What experience do you have? What languages do you like to use? Tell us a little bit about yourself! student: diff --git a/db/migrate/20250823151717_add_how_you_found_us_options.rb b/db/migrate/20250823151717_add_how_you_found_us_options.rb new file mode 100644 index 000000000..cfe4c8fd5 --- /dev/null +++ b/db/migrate/20250823151717_add_how_you_found_us_options.rb @@ -0,0 +1,5 @@ +class AddHowYouFoundUsOptions < ActiveRecord::Migration[7.0] + def change + add_column :members, :how_you_found_us, :text, array: true, default: [] + end +end diff --git a/db/schema.rb b/db/schema.rb index 192b8fd69..a6c772737 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -391,6 +391,7 @@ t.string "pronouns" t.datetime "accepted_toc_at", precision: nil t.datetime "opt_in_newsletter_at", precision: nil + t.text "how_you_found_us", default: [], array: true t.index ["email"], name: "index_members_on_email", unique: true end diff --git a/spec/controllers/member/details_controller_spec.rb b/spec/controllers/member/details_controller_spec.rb new file mode 100644 index 000000000..1953f282c --- /dev/null +++ b/spec/controllers/member/details_controller_spec.rb @@ -0,0 +1,84 @@ +RSpec.describe Member::DetailsController, type: :controller do + render_views + let(:member) { Fabricate(:member) } + + before do + allow(controller).to receive(:current_user).and_return(member) + end + + describe 'PATCH #update' do + context 'with valid params' do + it 'updates how_you_found_us with checkbox options' do + patch :update, params: { + id: member.id, + member: { + how_you_found_us: ['Social Media', 'From a friend'], + newsletter: 'true' + } + } + + member.reload + expect(member.how_you_found_us).to contain_exactly('Social Media', 'From a friend') + expect(response).to redirect_to(step2_member_path) + end + + it 'adds other_reason to how_you_found_us when provided' do + patch :update, params: { + id: member.id, + member: { + how_you_found_us: ['Search engine (Google etc.)'], + newsletter: 'false' + }, + other_reason: 'Saw a flyer' + } + + member.reload + expect(member.how_you_found_us).to contain_exactly('Search engine (Google etc.)', 'Saw a flyer') + expect(response).to redirect_to(step2_member_path) + end + + it 'updates how_you_found_us with only other_reason' do + patch :update, params: { + id: member.id, + member: { + how_you_found_us: [], + newsletter: 'true' + }, + other_reason: 'At a meetup' + } + + member.reload + expect(member.how_you_found_us).to eq(['At a meetup']) + expect(response).to redirect_to(step2_member_path) + end + + it 'removes duplicates and blank entries' do + patch :update, params: { + id: member.id, + member: { + how_you_found_us: ['From a friend', '', 'From a friend'], + newsletter: 'true' + }, + other_reason: 'From a friend' + } + + member.reload + expect(member.how_you_found_us).to eq(['From a friend']) + expect(response).to redirect_to(step2_member_path) + end + end + + context 'when update fails (invalid data)' do + it 'renders the edit template' do + patch :update, params: { + id: member.id, + member: { + how_you_found_us: [] + } + } + + expect(response.body).to include('You must select at least one option') + end + end + end +end diff --git a/spec/fabricators/member_fabricator.rb b/spec/fabricators/member_fabricator.rb index 0564bcb48..568589f01 100644 --- a/spec/fabricators/member_fabricator.rb +++ b/spec/fabricators/member_fabricator.rb @@ -4,6 +4,7 @@ surname { Faker::Name.last_name } email { Faker::Internet.email } about_you { Faker::Lorem.sentence } + how_you_found_us { ['From a friend'] } auth_services(count: 1) { Fabricate(:auth_service) } accepted_toc_at { Time.zone.now } end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 7b7ab26ae..e7e1d6410 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -15,6 +15,28 @@ it { expect(member).to validate_presence_of(:email) } it { expect(member).to validate_presence_of(:about_you) } end + + + context 'how_you_found_us validation' do + let(:new_member) { Fabricate(:member) } + it 'is invalid if how_you_found_us is nil' do + new_member.how_you_found_us = nil + new_member.valid? + expect(new_member.errors[:how_you_found_us]).to include("You must select at least one option") + end + + it 'is invalid if how_you_found_us is an empty array' do + new_member.how_you_found_us = [] + new_member.valid? + expect(new_member.errors[:how_you_found_us]).to include("You must select at least one option") + end + + it 'is valid if how_you_found_us has at least one non-blank option' do + new_member.how_you_found_us = ['From a friend'] + new_member.valid? + expect(new_member.errors[:how_you_found_us]).to be_empty + end + end end describe 'properties' do From db440b61675ce18d9822b1fff188960b3b372757 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:00:59 +0100 Subject: [PATCH 2/7] chore: update styling of form Add spacing around how you found us section --- app/views/member/details/edit.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/member/details/edit.html.haml b/app/views/member/details/edit.html.haml index ff84b4e8f..2dc095076 100644 --- a/app/views/member/details/edit.html.haml +++ b/app/views/member/details/edit.html.haml @@ -21,14 +21,14 @@ %ul - @member.errors.full_messages.each do |msg| %li= msg - .col-12 + .col-12.mb-3 %label{ for: 'how_you_found_us' }= t('member.details.edit.how_you_found_us') - - options = ['From a friend', 'Search engine (Google etc.)', 'Social Media', "One of Codebar's hosts or partners", 'Other reason'] + - 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 :other_reason, t('member.details.edit.other_reason') + = label_tag :other_reason, t('member.details.edit.other_reason'), class: 'my-1' = text_field_tag :other_reason, nil, placeholder: "Please specify", style: 'width: 500px;' = f.input :newsletter, as: :boolean, checked_value: true, unchecked_value: false .text-right.mb-4 From 9f5ea814bb1845b8aa99a4ef81377643d7bf9f16 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:54:26 +0100 Subject: [PATCH 3/7] fix: validation and styling Move validation from model to controller so validation doesn't block new user creation and is only valid for the member details page. --- app/controllers/member/details_controller.rb | 8 +++++++- app/controllers/members_controller.rb | 4 +--- app/models/member.rb | 7 ------- app/views/member/details/edit.html.haml | 11 ++++------- spec/features/member_joining_spec.rb | 3 +++ 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/app/controllers/member/details_controller.rb b/app/controllers/member/details_controller.rb index eb3f82caf..38bbc800b 100644 --- a/app/controllers/member/details_controller.rb +++ b/app/controllers/member/details_controller.rb @@ -15,6 +15,12 @@ def edit def update attrs = member_params.to_h + if attrs[:how_you_found_us].blank? + @member.assign_attributes(attrs) + @member.errors.add(:how_you_found_us, 'You must select at least one option') + return render :edit + end + how_found = params.dig(:member, :how_you_found_us) attrs[:how_you_found_us] = if how_found.is_a?(Array) @@ -37,4 +43,4 @@ def update member_params[:newsletter] ? subscribe_to_newsletter(@member) : unsubscribe_from_newsletter(@member) redirect_to step2_member_path end -end +end \ No newline at end of file diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 72f7e2286..3785d4477 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -12,9 +12,7 @@ def new end - def edit - binding.pry - end + def edit; end def step2 @type = cookies[:member_type] diff --git a/app/models/member.rb b/app/models/member.rb index 192de9da2..59ef928aa 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -19,7 +19,6 @@ class Member < ApplicationRecord validates :name, :surname, :email, :about_you, presence: true, if: :can_log_in? validates :email, uniqueness: true validates :about_you, length: { maximum: 255 } - validate :how_you_found_us_must_have_at_least_one_option scope :accepted_toc, -> { where.not(accepted_toc_at: nil) } scope :order_by_email, -> { order(:email) } @@ -126,12 +125,6 @@ def self.find_members_by_name(name) private - def how_you_found_us_must_have_at_least_one_option - if how_you_found_us.blank? || how_you_found_us.reject(&:blank?).empty? - errors.add(:how_you_found_us, 'You must select at least one option') - end - end - def invitations_on(date) workshop_invitations .joins(:workshop) diff --git a/app/views/member/details/edit.html.haml b/app/views/member/details/edit.html.haml index 2dc095076..689d916f4 100644 --- a/app/views/member/details/edit.html.haml +++ b/app/views/member/details/edit.html.haml @@ -15,21 +15,18 @@ = f.input :about_you, as: :text, label: t('member.details.edit.coach.about_you'), input_html: { rows: 3 }, required: true - else = f.input :about_you, as: :text, label: t('member.details.edit.student.about_you'), input_html: { rows: 3 }, required: true - - if @member.errors.any? - #error_explanation - %h2= "#{pluralize(@member.errors.count, 'error')} prohibited this member from being saved:" - %ul - - @member.errors.full_messages.each do |msg| - %li= msg + - 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 :other_reason, t('member.details.edit.other_reason'), class: 'my-1' - = text_field_tag :other_reason, nil, placeholder: "Please specify", style: 'width: 500px;' + = text_field_tag :other_reason, nil, placeholder: "Please specify how you heard about us", 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) diff --git a/spec/features/member_joining_spec.rb b/spec/features/member_joining_spec.rb index 8cfcf27b7..ea6868ffa 100644 --- a/spec/features/member_joining_spec.rb +++ b/spec/features/member_joining_spec.rb @@ -27,6 +27,7 @@ expect(page).to have_content "Surname can't be blank" expect(page).to have_content "Email address can't be blank" expect(page).to have_content "About you can't be blank" + expect(page).to have_content "You must select at least one option" end scenario 'A new member details are successfully captured' do @@ -40,6 +41,8 @@ fill_in 'member_surname', with: 'Doe' fill_in 'member_email', with: 'jane@codebar.io' fill_in 'member_about_you', with: Faker::Lorem.paragraph + find('#how_you_found_us_from-a-friend').click + fill_in 'other_reason', with: 'found on a poster', id: true click_on 'Next' expect(page).to have_current_path(step2_member_path) From 73f9c9ea1cebb100b75c1736d5c40bc52deb6398 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Tue, 26 Aug 2025 14:08:51 +0100 Subject: [PATCH 4/7] fix: failing tests Fix failing tests and move other_reason into member params and give param name more description --- app/controllers/concerns/member_concerns.rb | 2 +- app/controllers/member/details_controller.rb | 28 ++++++------------- app/views/member/details/edit.html.haml | 4 +-- config/locales/en.yml | 2 +- .../member/details_controller_spec.rb | 8 +++--- spec/features/member_joining_spec.rb | 2 +- .../subscribing_to_newsletter_spec.rb | 4 +++ spec/models/member_spec.rb | 22 --------------- 8 files changed, 22 insertions(+), 50 deletions(-) diff --git a/app/controllers/concerns/member_concerns.rb b/app/controllers/concerns/member_concerns.rb index f059608a5..76c5594d0 100644 --- a/app/controllers/concerns/member_concerns.rb +++ b/app/controllers/concerns/member_concerns.rb @@ -18,7 +18,7 @@ def member_params :about_you, :skill_list, :newsletter, - :other_reason, + :how_you_found_us_other_reason, how_you_found_us: [] ) end diff --git a/app/controllers/member/details_controller.rb b/app/controllers/member/details_controller.rb index 38bbc800b..19e940a8b 100644 --- a/app/controllers/member/details_controller.rb +++ b/app/controllers/member/details_controller.rb @@ -13,30 +13,20 @@ def edit end def update - attrs = member_params.to_h + how_found = Array(params.dig(:member, :how_you_found_us)).reject(&:blank?) + other_reason = params.dig(:member, :how_you_found_us_other_reason) - if attrs[:how_you_found_us].blank? - @member.assign_attributes(attrs) + 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)) @member.errors.add(:how_you_found_us, 'You must select at least one option') return render :edit end - how_found = params.dig(:member, :how_you_found_us) - attrs[:how_you_found_us] = - if how_found.is_a?(Array) - how_found - elsif how_found.blank? - [] - else - [how_found] - end - - if params[:other_reason].present? - attrs[:how_you_found_us] << params[:other_reason] - end - - attrs[:how_you_found_us].uniq! - attrs[:how_you_found_us].reject!(&:blank?) + attrs = member_params.to_h.except(:how_you_found_us_other_reason) + attrs[:how_you_found_us] = how_found return render :edit unless @member.update(attrs) diff --git a/app/views/member/details/edit.html.haml b/app/views/member/details/edit.html.haml index 689d916f4..6a7980b9c 100644 --- a/app/views/member/details/edit.html.haml +++ b/app/views/member/details/edit.html.haml @@ -25,8 +25,8 @@ .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 :other_reason, t('member.details.edit.other_reason'), class: 'my-1' - = text_field_tag :other_reason, nil, placeholder: "Please specify how you heard about us", class: 'form-control w-100;' + = 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' = 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) diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b501a352..af6cdb5ff 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -443,7 +443,7 @@ en: title: Almost there... summary: We need some more details from you to finish creating your account. We use these to help run our events. how_you_found_us: "How did you find out about us?" - other_reason: "Please specify (if Other)" + how_you_found_us_other_reason: "Please specify (if Other)" coach: about_you: What experience do you have? What languages do you like to use? Tell us a little bit about yourself! student: diff --git a/spec/controllers/member/details_controller_spec.rb b/spec/controllers/member/details_controller_spec.rb index 1953f282c..259c5af9a 100644 --- a/spec/controllers/member/details_controller_spec.rb +++ b/spec/controllers/member/details_controller_spec.rb @@ -27,13 +27,13 @@ id: member.id, member: { how_you_found_us: ['Search engine (Google etc.)'], + how_you_found_us_other_reason: 'Saw a pamphlet', newsletter: 'false' }, - other_reason: 'Saw a flyer' } member.reload - expect(member.how_you_found_us).to contain_exactly('Search engine (Google etc.)', 'Saw a flyer') + expect(member.how_you_found_us).to contain_exactly('Search engine (Google etc.)', 'Saw a pamphlet') expect(response).to redirect_to(step2_member_path) end @@ -42,9 +42,9 @@ id: member.id, member: { how_you_found_us: [], + how_you_found_us_other_reason: 'At a meetup', newsletter: 'true' }, - other_reason: 'At a meetup' } member.reload @@ -57,9 +57,9 @@ id: member.id, member: { how_you_found_us: ['From a friend', '', 'From a friend'], + how_you_found_us_other_reason: 'From a friend', newsletter: 'true' }, - other_reason: 'From a friend' } member.reload diff --git a/spec/features/member_joining_spec.rb b/spec/features/member_joining_spec.rb index ea6868ffa..c2948f9c4 100644 --- a/spec/features/member_joining_spec.rb +++ b/spec/features/member_joining_spec.rb @@ -42,7 +42,7 @@ fill_in 'member_email', with: 'jane@codebar.io' fill_in 'member_about_you', with: Faker::Lorem.paragraph find('#how_you_found_us_from-a-friend').click - fill_in 'other_reason', with: 'found on a poster', id: true + fill_in 'member_how_you_found_us_other_reason', with: 'found on a poster', id: true click_on 'Next' expect(page).to have_current_path(step2_member_path) diff --git a/spec/features/subscribing_to_newsletter_spec.rb b/spec/features/subscribing_to_newsletter_spec.rb index 38c24b940..2e5101615 100644 --- a/spec/features/subscribing_to_newsletter_spec.rb +++ b/spec/features/subscribing_to_newsletter_spec.rb @@ -27,6 +27,8 @@ fill_in 'member_surname', with: 'Doe' fill_in 'member_email', with: 'jane@codebar.io' fill_in 'member_about_you', with: Faker::Lorem.paragraph + find('#how_you_found_us_from-a-friend').click + fill_in 'member_how_you_found_us_other_reason', with: Faker::Lorem.paragraph, id: true click_on 'Next' end @@ -46,6 +48,8 @@ fill_in 'member_surname', with: 'Doe' fill_in 'member_email', with: 'jane@codebar.io' fill_in 'member_about_you', with: Faker::Lorem.paragraph + find('#how_you_found_us_from-a-friend').click + fill_in 'member_how_you_found_us_other_reason', with: Faker::Lorem.paragraph, id: true uncheck 'newsletter' diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index e7e1d6410..7b7ab26ae 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -15,28 +15,6 @@ it { expect(member).to validate_presence_of(:email) } it { expect(member).to validate_presence_of(:about_you) } end - - - context 'how_you_found_us validation' do - let(:new_member) { Fabricate(:member) } - it 'is invalid if how_you_found_us is nil' do - new_member.how_you_found_us = nil - new_member.valid? - expect(new_member.errors[:how_you_found_us]).to include("You must select at least one option") - end - - it 'is invalid if how_you_found_us is an empty array' do - new_member.how_you_found_us = [] - new_member.valid? - expect(new_member.errors[:how_you_found_us]).to include("You must select at least one option") - end - - it 'is valid if how_you_found_us has at least one non-blank option' do - new_member.how_you_found_us = ['From a friend'] - new_member.valid? - expect(new_member.errors[:how_you_found_us]).to be_empty - end - end end describe 'properties' do From 51fdf4720e27fedaae0b9fe1b81a032162e5381c Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Thu, 28 Aug 2025 11:15:58 +0100 Subject: [PATCH 5/7] chore: fix code review issues --- app/controllers/member/details_controller.rb | 9 +++++---- app/controllers/members_controller.rb | 1 - app/views/member/details/edit.html.haml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/member/details_controller.rb b/app/controllers/member/details_controller.rb index 19e940a8b..2321b616a 100644 --- a/app/controllers/member/details_controller.rb +++ b/app/controllers/member/details_controller.rb @@ -13,19 +13,20 @@ 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 = 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! + member_params_without_other_reason = member_params.to_h.except(:how_you_found_us_other_reason) if how_found.blank? - @member.assign_attributes(member_params.to_h.except(:how_you_found_us_other_reason)) + @member.assign_attributes(member_params_without_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 = member_params_without_other_reason attrs[:how_you_found_us] = how_found return render :edit unless @member.update(attrs) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 3785d4477..e08932e12 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -9,7 +9,6 @@ class MembersController < ApplicationController def new @page_title = 'Sign up' - end def edit; end diff --git a/app/views/member/details/edit.html.haml b/app/views/member/details/edit.html.haml index 6a7980b9c..9bfaa6fef 100644 --- a/app/views/member/details/edit.html.haml +++ b/app/views/member/details/edit.html.haml @@ -15,7 +15,7 @@ = f.input :about_you, as: :text, label: t('member.details.edit.coach.about_you'), input_html: { rows: 3 }, required: true - else = f.input :about_you, as: :text, label: t('member.details.edit.student.about_you'), input_html: { rows: 3 }, required: true - - 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') From e8dceedb59101a53fe9231d1caf167633fa9068e Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Thu, 28 Aug 2025 12:11:25 +0100 Subject: [PATCH 6/7] chore: add new line --- app/controllers/member/details_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/member/details_controller.rb b/app/controllers/member/details_controller.rb index 2321b616a..1d3985ba5 100644 --- a/app/controllers/member/details_controller.rb +++ b/app/controllers/member/details_controller.rb @@ -34,4 +34,4 @@ def update member_params[:newsletter] ? subscribe_to_newsletter(@member) : unsubscribe_from_newsletter(@member) redirect_to step2_member_path end -end \ No newline at end of file +end From a4027dcb5708111f69a2b1873430599622e0396a Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Fri, 29 Aug 2025 14:04:15 +0100 Subject: [PATCH 7/7] feat: add how you found us to member profile page --- app/views/members/show.html.haml | 8 ++++++++ spec/features/member_portal_spec.rb | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/app/views/members/show.html.haml b/app/views/members/show.html.haml index 433b9539a..9b8a4344c 100644 --- a/app/views/members/show.html.haml +++ b/app/views/members/show.html.haml @@ -26,6 +26,14 @@ %dt About %dd.mb-3= @member.about_you + %dt How you found us + - if @member.how_you_found_us.present? + %ul + - @member.how_you_found_us.each do |reason| + %li= reason + - else + %dd Not specified + - if @member.skills.any? %dt Skills %dd.mb-3 diff --git a/spec/features/member_portal_spec.rb b/spec/features/member_portal_spec.rb index 2d26e0d0a..0e31a2755 100644 --- a/spec/features/member_portal_spec.rb +++ b/spec/features/member_portal_spec.rb @@ -45,6 +45,25 @@ end end + it 'can access and view their profile' do + visit profile_path + + expect(page).to have_content('Details') + expect(page).to have_content(member.full_name) + + expect(page).to have_content('Email') + expect(page).to have_content(member.email) + + expect(page).to have_content('Phone number') + expect(page).to have_content(member.mobile) + + expect(page).to have_content('About') + expect(page).to have_content(member.about_you) + + expect(page).to have_content('How you found us') + expect(page).to have_content(member.how_you_found_us.first) + end + it 'can access and update their profile' do visit profile_path