-
-
Notifications
You must be signed in to change notification settings - Fork 199
feat: add checkboxes for how you found us options during onboarding #2303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
$(document).ready(function() { | ||
const $otherRadioButton = $('#member_how_you_found_us_other'); | ||
const $otherReason = $('#member_how_you_found_us_other_reason'); | ||
const $elementToToggle = $otherReason.parent(); | ||
|
||
function toggleOtherReason() { | ||
if ($otherRadioButton.is(':checked')) { | ||
$elementToToggle.removeClass('d-none').hide().slideDown(50); | ||
$otherReason.prop('disabled', false).focus(); // Optional — disabling is not needed | ||
} else { | ||
$elementToToggle.slideUp(50, function() { | ||
$elementToToggle.addClass('d-none'); | ||
$otherReason.val(''); | ||
}); | ||
} | ||
} | ||
|
||
toggleOtherReason(); | ||
$('input[name="member[how_you_found_us]"]').on('change', toggleOtherReason); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,30 @@ | |
label_method: ->(r) { r.humanize.upcase_first } | ||
= f.input :other_dietary_restrictions, placeholder: 'Other dietary restrictions', | ||
wrapper_html: { class: class_names('mt-n3', 'd-none': [email protected]_dietary_restrictions?) }, label_html: { class: 'sr-only' } | ||
- 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? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be simplifiable into two Simple Form can take care of displaying the labels for each check box value and for displaying any validation errors for the fields. Simple Form detects that an attribute on the model is an array so you should just need to provide the collection of allowed values and the indication of whether to use check boxes vs. radio buttons. Something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Typo: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've implemented this for the most part. |
||
%span.text-danger= @member.errors[:how_you_found_us].first | ||
.col-12.mb-3 | ||
%fieldset | ||
= f.input :how_you_found_us, | ||
as: :radio_buttons, | ||
collection: Member.how_you_found_us.keys, | ||
label_method: ->(option) { t("member.details.edit.how_you_found_us_options.#{option}") }, | ||
value_method: :to_s, | ||
label: t('member.details.edit.how_you_found_us_label'), | ||
item_wrapper_class: 'form-check d-flex align-items-center mb-2', | ||
label_item: true, | ||
input_html: { class: 'form-check-input me-2', style: 'margin-top: 0;' }, | ||
label_html: { class: 'form-check-label', style: 'margin-left: 0;' } | ||
|
||
= f.input :how_you_found_us_other_reason, | ||
label: t('member.details.edit.how_you_found_us_other_reason'), | ||
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) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
class AddHowYouFoundUsOptions < ActiveRecord::Migration[7.0] | ||
def change | ||
add_column :members, :how_you_found_us, :integer | ||
add_column :members, :how_you_found_us_other_reason, :string | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
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 | ||
context 'with valid params' do | ||
it 'updates how_you_found_us with radio option' do | ||
patch :update, params: { | ||
id: member.id, | ||
member: { | ||
how_you_found_us: 'social_media', | ||
newsletter: 'true' | ||
} | ||
} | ||
|
||
member.reload | ||
expect(I18n.t("member.details.edit.how_you_found_us_options.#{member.how_you_found_us}")).to eq('Social media') | ||
expect(member.how_you_found_us_other_reason).to eq(nil) | ||
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: 'other', | ||
how_you_found_us_other_reason: 'Saw a pamphlet', | ||
newsletter: 'false' | ||
}, | ||
} | ||
|
||
member.reload | ||
expect(member.how_you_found_us).to eq('other') | ||
expect(member.how_you_found_us_other_reason).to eq('Saw a pamphlet') | ||
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: 'other', | ||
how_you_found_us_other_reason: 'At a meetup', | ||
newsletter: 'true' | ||
}, | ||
} | ||
|
||
member.reload | ||
expect(member.how_you_found_us).to eq('other') | ||
expect(member.how_you_found_us_other_reason).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: 'other', | ||
how_you_found_us_other_reason: 'From a colleague', | ||
newsletter: 'true' | ||
}, | ||
} | ||
|
||
member.reload | ||
expect(member.how_you_found_us).to eq('other') | ||
expect(member.how_you_found_us_other_reason).to eq('From a colleague') | ||
expect(response).to redirect_to(step2_member_path) | ||
end | ||
end | ||
|
||
context 'when update fails (invalid data)' do | ||
it 'error raised when no how you found us selection given' do | ||
patch :update, params: { | ||
id: member.id, | ||
member: { | ||
how_you_found_us: 'other', | ||
how_you_found_us_other_reason: nil, | ||
} | ||
} | ||
|
||
expect(response.body).to include('You must select one option') | ||
end | ||
|
||
it 'error raised when both how you found us fields popoulated' do | ||
patch :update, params: { | ||
id: member.id, | ||
member: { | ||
how_you_found_us: 'from_a_friend', | ||
how_you_found_us_other_reason: 'something else', | ||
} | ||
} | ||
|
||
expect(response.body).to include('You must select one option') | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ | |
fill_in 'member_surname', with: 'Doe' | ||
fill_in 'member_email', with: '[email protected]' | ||
fill_in 'member_about_you', with: Faker::Lorem.paragraph | ||
find('#member_how_you_found_us_from_a_friend').click | ||
|
||
click_on 'Next' | ||
end | ||
|
@@ -46,6 +47,8 @@ | |
fill_in 'member_surname', with: 'Doe' | ||
fill_in 'member_email', with: '[email protected]' | ||
fill_in 'member_about_you', with: Faker::Lorem.paragraph | ||
find('#member_how_you_found_us_other').click | ||
fill_in 'member_how_you_found_us_other_reason', with: Faker::Lorem.paragraph, id: true | ||
|
||
uncheck 'newsletter' | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change ❤️