Skip to content
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

4135 Individuals Requests Preserve Values #4142

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def create
flash[:success] = 'Request was successfully created.'
redirect_to partners_request_path(create_service.partner_request.id)
else
@request = FamilyRequest.new({}, initial_items: 1)
@request = FamilyRequest.new_with_attrs(create_service.family_requests_attributes)
@errors = create_service.errors

@requestable_items = PartnerFetchRequestableItemsService.new(partner_id: current_partner.id).call
Expand Down
7 changes: 7 additions & 0 deletions app/models/partners/family_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ def items_attributes=(attributes)
end
end

def self.new_with_attrs(request_attrs)
items_attributes = request_attrs.map.with_index { |x, i| [i, x] }.to_h
request = new({}, initial_items: 1)
request.items_attributes = items_attributes
request
end

class Item
include ActiveModel::Model

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= form.fields_for(:items) do |field| %>
<%= form.fields_for :items, defined?(object) ? object : nil do |field| %>
<tr>
<td>
<%= field.label :item_id, "Item Requested", {class: 'sr-only'} %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/partners/individuals_requests/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

<div>
<%= add_item_button('Add Another Item') do %>
<%= render 'partners/individuals_requests/item_request', form: form %>
<%= render 'partners/individuals_requests/item_request', form: form, object: Partners::FamilyRequest::Item.new %>
<% end %>
</div>

Expand Down
46 changes: 46 additions & 0 deletions spec/models/partners/family_request_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require "rails_helper"

RSpec.describe Partners::FamilyRequest do
describe "Partners::FamilyRequest.new_with_attrs" do
it "creates a new FamilyRequest with attributes" do
attributes = [{item_id: 1, person_count: 3}]
request = Partners::FamilyRequest.new_with_attrs(attributes)

expect(request.items.length).to eq(1)
expect(request.items.first.item_id).to eq(1)
expect(request.items.first.person_count).to eq(3)
end
end

describe "#items_attributes=" do
let(:item_attributes) { {"0" => {item_id: 1, person_count: 2}, "1" => {item_id: 2, person_count: 3}} }
let(:family_request) { Partners::FamilyRequest.new({}) }

it "assigns items based on given attributes" do
family_request.items_attributes = item_attributes
expect(family_request.items.length).to eq(2)
expect(family_request.items.first.item_id).to eq(1)
expect(family_request.items.first.person_count).to eq(2)
expect(family_request.items.last.item_id).to eq(2)
expect(family_request.items.last.person_count).to eq(3)
end

it "creates instances of Item with correct attributes" do
family_request.items_attributes = item_attributes
expect(family_request.items.first).to be_an_instance_of(Partners::FamilyRequest::Item)
expect(family_request.items.last).to be_an_instance_of(Partners::FamilyRequest::Item)
end

it "overrides existing items" do
family_request = Partners::FamilyRequest.new({}, initial_items: 6)
expect(family_request.items.length).to eq(6)

family_request.items_attributes = item_attributes
expect(family_request.items.length).to eq(2)
expect(family_request.items.first.item_id).to eq(1)
expect(family_request.items.first.person_count).to eq(2)
expect(family_request.items.last.item_id).to eq(2)
expect(family_request.items.last.person_count).to eq(3)
end
end
end
52 changes: 52 additions & 0 deletions spec/system/partners/managing_requests_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@

context 'WHEN they create a request completely empty request' do
before do
fill_in 'Comments', with: Faker::Lorem.paragraph

# Select items
3.times do |idx|
if idx != 0
click_link 'Add Another Item'
end

last_row = find_all('tr').last
last_row.find_all('.form-control').last.fill_in(with: 1)
end

click_button 'Submit Essentials Request'
end

Expand All @@ -95,6 +107,14 @@
expect(page).to have_content('Ensure each line item has a item selected AND a quantity greater than 0.')
expect(page).to have_content('Still need help? Submit a support ticket here and we will do our best to follow up with you via email.')
end

it "should show invalid values in the form" do
expected_items = page.all('select').map(&:value)
expected_quantities = page.all('input[type="number"]').map(&:value)

expect(expected_items).to eq(["", "", ""])
expect(expected_quantities).to eq(["1", "1", "1"])
end
end

context 'WHEN they create a request properly' do
Expand Down Expand Up @@ -218,6 +238,38 @@
end
end

context 'WHEN a request has invalid values' do
before do
fill_in 'Comments', with: Faker::Lorem.paragraph

# Select items
3.times do |idx|
if idx != 0
click_link 'Add Another Item'
end

last_row = find_all('tr').last
last_row.find_all('.form-control').last.fill_in(with: 1)
end

click_button 'Submit Essentials Request'
end

it 'should show an error message with the instructions ' do
expect(page).to have_content('Oops! Something went wrong with your Request')
expect(page).to have_content('Ensure each line item has a item selected AND a quantity greater than 0.')
expect(page).to have_content('Still need help? Submit a support ticket here and we will do our best to follow up with you via email.')
end

it "should show invalid values in the form" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we combine these 2? System specs are expensive, so let's get all our expectations together :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually already drafted a PR to move the input tests to request specs: #4147. So they are separate because the check for invalid values will remain a system test while the other stuff will moved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dorner do you still feel this way? or can this PR proceed? I would also like to rebase this PR on top of #4099 cause they have some overlap

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, since we're addressing it in a followup PR I'm OK with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dorner - so does that mean this one meets your approval now (sounds like it)?

expected_items = page.all('select').map(&:value)
expected_quantities = page.all('input[type="number"]').map(&:value)

expect(expected_items).to eq(["", "", ""])
expect(expected_quantities).to eq(["1", "1", "1"])
end
end

context 'WHEN they create a request with blank lines' do
before do
fill_in 'Comments', with: Faker::Lorem.paragraph
Expand Down
Loading