From b761496244d89c93c84eba5c7f0e341c9ba9e963 Mon Sep 17 00:00:00 2001 From: Cory Streiff <90390502+coalest@users.noreply.github.com> Date: Tue, 10 Dec 2024 02:45:00 +0100 Subject: [PATCH] 4175 add request type flag (#4827) * Adds type enum to request model. * Adds request type enum to request model. * Adds request_type to request. * Adds request_type to request. * Passes request_type to FamilyRequestCreateService.new * Passes request_type to FamilyRequestCreateService.new * Passes request_type to RequestCreateService.new * Passes request_type to Request.new. * Adds hidden field in order to pass request_type param on submit. * Adds request type flag to index and show pages as well as the requests export. Adds filter by request type to index. * Adds by_request_type scope for filtering by request type. * Adds request type flag to index and show pages as well as the requests export. Adds filter by request type to index. * Adds by_request_type filter. * Updates flag names and removes flag styles based on feedback from ruby for good. * Adds test for request_type enum. * Removes unnecessary line. * Adds Type to expected_headers. * Adds request_type traits to request factory. * Adds request_type to family_request_create_service_spec args. * Adds request_type to args. * Pass request_type directly. * Remove request_type partial and enter text directly into reqest_row * Revert changes to database.yml * Revert changes to request_spec.rb * Remove request_type * Remove hidden request_type fields. * Change request_type data_type to string, replace request_type string in view to display only the first letter, pull request_type from service instead of defining with an instance var. * Remove request_type from tests * Add request_type_label method * Undo changes to versions * Add request_type_label method * Add request_type to initialize * Fix request_type bug * Fix linting * Fix request_type_label method model spec * Remove unused for_families attribute from RequestCreateService * Remove leftover print debugging statement * Fix/add specs for new request_type attribute * Revert submit button color change for family requests * Hover over request type abbreviation to see full text * Show full request type when viewing individual request * Fix linting --------- Co-authored-by: nathan Co-authored-by: Nathan Thomas <47466067+nathangthomas@users.noreply.github.com> --- .../partners/requests_controller.rb | 2 ++ app/controllers/requests_controller.rb | 4 ++- app/models/request.rb | 7 ++++ .../exports/export_request_service.rb | 3 ++ .../partners/family_request_create_service.rb | 8 +++-- .../partners/request_create_service.rb | 14 +++++--- app/views/requests/_request_row.html.erb | 5 +++ app/views/requests/index.html.erb | 4 +++ app/views/requests/show.html.erb | 2 ++ .../20240315190152_add_type_to_requests.rb | 5 +++ db/schema.rb | 1 + spec/factories/requests.rb | 13 +++++++ spec/models/request_spec.rb | 9 +++++ spec/requests/items_requests_spec.rb | 2 +- .../exports/export_request_service_spec.rb | 19 +++++++++++ .../family_request_create_service_spec.rb | 24 ++++++++++++- .../partners/request_create_service_spec.rb | 34 +++++++++++++++++++ 17 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20240315190152_add_type_to_requests.rb diff --git a/app/controllers/partners/requests_controller.rb b/app/controllers/partners/requests_controller.rb index 2c01b28991..48b7516cd3 100644 --- a/app/controllers/partners/requests_controller.rb +++ b/app/controllers/partners/requests_controller.rb @@ -20,6 +20,7 @@ def show def create create_service = Partners::RequestCreateService.new( + request_type: "quantity", partner_user_id: current_user.id, comments: partner_request_params[:comments], item_requests_attributes: partner_request_params[:item_requests_attributes]&.values || [] @@ -43,6 +44,7 @@ def create def validate @partner_request = Partners::RequestCreateService.new( + request_type: "quantity", partner_user_id: current_user.id, comments: partner_request_params[:comments], item_requests_attributes: partner_request_params[:item_requests_attributes]&.values || [] diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index db073afc15..f1d5a10e13 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -15,6 +15,8 @@ def index @partners = current_organization.partners.order(:name) @statuses = Request.statuses.transform_keys(&:humanize) @partner_users = User.where(id: @paginated_requests.pluck(:partner_user_id)) + @request_types = Request.request_types.transform_keys(&:humanize) + @selected_request_type = filter_params[:by_request_type] @selected_request_item = filter_params[:by_request_item_id] @selected_partner = filter_params[:by_partner] @selected_status = filter_params[:by_status] @@ -71,6 +73,6 @@ def load_items def filter_params return {} unless params.key?(:filters) - params.require(:filters).permit(:by_request_item_id, :by_partner, :by_status) + params.require(:filters).permit(:by_request_item_id, :by_partner, :by_status, :by_request_type) end end diff --git a/app/models/request.rb b/app/models/request.rb index 799d2f5331..0c2647ccc1 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -7,6 +7,7 @@ # discard_reason :text # discarded_at :datetime # request_items :jsonb +# request_type :string # status :integer default("pending") # created_at :datetime not null # updated_at :datetime not null @@ -31,6 +32,7 @@ class Request < ApplicationRecord has_many :child_item_requests, through: :item_requests enum status: { pending: 0, started: 1, fulfilled: 2, discarded: 3 }, _prefix: true + enum request_type: %w[quantity individual child].map { |v| [v, v] }.to_h validates :distribution_id, uniqueness: true, allow_nil: true validate :item_requests_uniqueness_by_item_id @@ -45,6 +47,7 @@ class Request < ApplicationRecord scope :by_partner, ->(partner_id) { where(partner_id: partner_id) } # status scope to allow filtering by status scope :by_status, ->(status) { where(status: status) } + scope :by_request_type, ->(request_type) { where(request_type: request_type) } scope :during, ->(range) { where(created_at: range) } scope :for_csv_export, ->(organization, *) { where(organization: organization) @@ -60,6 +63,10 @@ def user_email partner_user_id ? User.find_by(id: partner_user_id).email : Partner.find_by(id: partner_id).email end + def request_type_label + request_type&.first&.capitalize + end + private def item_requests_uniqueness_by_item_id diff --git a/app/services/exports/export_request_service.rb b/app/services/exports/export_request_service.rb index 94501fae1c..ae7a224eaa 100644 --- a/app/services/exports/export_request_service.rb +++ b/app/services/exports/export_request_service.rb @@ -46,6 +46,9 @@ def base_table "Requestor" => ->(request) { request.partner.name }, + "Type" => ->(request) { + request.request_type&.humanize + }, "Status" => ->(request) { request.status.humanize } diff --git a/app/services/partners/family_request_create_service.rb b/app/services/partners/family_request_create_service.rb index e585da9a13..8e018530db 100644 --- a/app/services/partners/family_request_create_service.rb +++ b/app/services/partners/family_request_create_service.rb @@ -21,7 +21,7 @@ def call request_create_svc = Partners::RequestCreateService.new( partner_user_id: partner_user_id, comments: comments, - for_families: @for_families, + request_type: request_type, item_requests_attributes: item_requests_attributes ) @@ -43,7 +43,7 @@ def initialize_only Partners::RequestCreateService.new( partner_user_id: partner_user_id, comments: comments, - for_families: @for_families, + request_type: request_type, item_requests_attributes: item_requests_attributes ).initialize_only end @@ -81,5 +81,9 @@ def convert_person_count_to_item_quantity(item_id:, person_count:) def included_items_by_id @included_items_by_id ||= Item.where(id: family_requests_attributes.pluck(:item_id)).index_by(&:id) end + + def request_type + @for_families ? "child" : "individual" + end end end diff --git a/app/services/partners/request_create_service.rb b/app/services/partners/request_create_service.rb index a9875fbf2c..b80321d1f7 100644 --- a/app/services/partners/request_create_service.rb +++ b/app/services/partners/request_create_service.rb @@ -4,19 +4,22 @@ class RequestCreateService attr_reader :partner_request - def initialize(partner_user_id:, comments: nil, for_families: false, item_requests_attributes: [], additional_attrs: {}) + def initialize(request_type:, partner_user_id:, comments: nil, item_requests_attributes: [], additional_attrs: {}) @partner_user_id = partner_user_id @comments = comments - @for_families = for_families + @request_type = request_type @item_requests_attributes = item_requests_attributes @additional_attrs = additional_attrs end def call - @partner_request = ::Request.new(partner_id: partner.id, + @partner_request = ::Request.new( + partner_id: partner.id, organization_id: organization_id, comments: comments, - partner_user_id: partner_user_id) + request_type: request_type, + partner_user_id: partner_user_id + ) @partner_request = populate_item_request(@partner_request) @partner_request.assign_attributes(additional_attrs) @@ -44,6 +47,7 @@ def initialize_only partner_request = ::Request.new(partner_id: partner.id, organization_id: organization_id, comments: comments, + request_type: request_type, partner_user_id: partner_user_id) partner_request = populate_item_request(partner_request) partner_request.assign_attributes(additional_attrs) @@ -52,7 +56,7 @@ def initialize_only private - attr_reader :partner_user_id, :comments, :item_requests_attributes, :additional_attrs + attr_reader :partner_user_id, :comments, :item_requests_attributes, :additional_attrs, :request_type def populate_item_request(partner_request) # Exclude any line item that is completely empty diff --git a/app/views/requests/_request_row.html.erb b/app/views/requests/_request_row.html.erb index dbdbceb467..86523947ed 100644 --- a/app/views/requests/_request_row.html.erb +++ b/app/views/requests/_request_row.html.erb @@ -12,6 +12,11 @@ <%= request_row.comments %> + + + <%= request_row.request_type_label %> + + <%= render partial: "status", locals: {status: request_row.status} %> diff --git a/app/views/requests/index.html.erb b/app/views/requests/index.html.erb index ef44ea0d3f..0fa0872137 100644 --- a/app/views/requests/index.html.erb +++ b/app/views/requests/index.html.erb @@ -47,6 +47,9 @@ <%= filter_select(scope: :by_partner, collection: @partners, selected: @selected_partner) %> <% end %> +
+ <%= filter_select(scope: :by_request_type, collection: @request_types, key: :last, value: :first, selected: @selected_request_type) %> +
<%= filter_select(scope: :by_status, collection: @statuses, key: :last, value: :first, selected: @selected_status) %>
@@ -98,6 +101,7 @@ Request Sender # of Items (Request Limit) Comments + Type Status Actions diff --git a/app/views/requests/show.html.erb b/app/views/requests/show.html.erb index 89478380fa..acb2a0635f 100644 --- a/app/views/requests/show.html.erb +++ b/app/views/requests/show.html.erb @@ -37,6 +37,7 @@ Request was sent by: Request Sender: + Request Type: Request Status: Comments: @@ -45,6 +46,7 @@ <%= @request.partner.name %> <%= @request.partner_user&.formatted_email %> + <%= @request.request_type&.humanize %> <%= render partial: "status", locals: {status: @request.status} %> <%= @request.comments || 'None' %> diff --git a/db/migrate/20240315190152_add_type_to_requests.rb b/db/migrate/20240315190152_add_type_to_requests.rb new file mode 100644 index 0000000000..335f3908bc --- /dev/null +++ b/db/migrate/20240315190152_add_type_to_requests.rb @@ -0,0 +1,5 @@ +class AddTypeToRequests < ActiveRecord::Migration[7.0] + def change + add_column :requests, :request_type, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 0b1ab87950..7f403f94e8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -712,6 +712,7 @@ t.datetime "discarded_at", precision: nil t.text "discard_reason" t.integer "partner_user_id" + t.string "request_type" t.index ["discarded_at"], name: "index_requests_on_discarded_at" t.index ["distribution_id"], name: "index_requests_on_distribution_id", unique: true t.index ["organization_id"], name: "index_requests_on_organization_id" diff --git a/spec/factories/requests.rb b/spec/factories/requests.rb index 987527a7da..017a36b787 100644 --- a/spec/factories/requests.rb +++ b/spec/factories/requests.rb @@ -7,6 +7,7 @@ # discard_reason :text # discarded_at :datetime # request_items :jsonb +# request_type :string # status :integer default("pending") # created_at :datetime not null # updated_at :datetime not null @@ -57,6 +58,18 @@ def random_request_items status { 'fulfilled' } end + trait :quantity do + request_type { 'quantity' } + end + + trait :individual do + request_type { 'individual' } + end + + trait :child do + request_type { 'child' } + end + trait :pending do status { 'pending' } end diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index 6a794b651b..473447797c 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -7,6 +7,7 @@ # discard_reason :text # discarded_at :datetime # request_items :jsonb +# request_type :string # status :integer default("pending") # created_at :datetime not null # updated_at :datetime not null @@ -127,6 +128,14 @@ end end + describe "request_type_label" do + let(:request) { build(:request, request_type: "individual") } + + it "returns the the first letter of the request_type capitalized" do + expect(request.request_type_label).to eq("I") + end + end + describe "versioning" do it { is_expected.to be_versioned } end diff --git a/spec/requests/items_requests_spec.rb b/spec/requests/items_requests_spec.rb index 8a2110645d..0e1f0adc65 100644 --- a/spec/requests/items_requests_spec.rb +++ b/spec/requests/items_requests_spec.rb @@ -222,7 +222,7 @@ it 'shows custom request units when flipper enabled' do Flipper.enable(:enable_packs) get item_path(id: item.id) - print(response.body) + expect(response.body).to include('Custom Units') expect(response.body).to include("ITEM1; ITEM2") end diff --git a/spec/services/exports/export_request_service_spec.rb b/spec/services/exports/export_request_service_spec.rb index 0c5bfa8849..fb91b5e5d1 100644 --- a/spec/services/exports/export_request_service_spec.rb +++ b/spec/services/exports/export_request_service_spec.rb @@ -16,6 +16,7 @@ let!(:request_3t) do create(:request, :started, + :child, :with_item_requests, organization: org, partner: partner, @@ -25,6 +26,7 @@ let!(:request_2t) do create(:request, :fulfilled, + :individual, :with_item_requests, organization: org, partner: partner, @@ -61,6 +63,7 @@ let!(:request_4t) do create(:request, :started, + :quantity, :with_item_requests, organization: org, partner: partner, @@ -72,6 +75,7 @@ let!(:request_4t_pack) do create(:request, :started, + :quantity, :with_item_requests, organization: org, partner: partner, @@ -94,6 +98,7 @@ expect(subject.first).to eq([ "Date", "Requestor", + "Type", "Status", "2T Diapers", "3T Diapers", @@ -111,6 +116,7 @@ expect(subject[1]).to eq([ request_3t.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Child", "Started", 0, # 2T Diapers 150, # 3T Diapers @@ -124,6 +130,7 @@ expect(subject[2]).to eq([ request_2t.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Individual", "Fulfilled", 100, # 2T Diapers 0, # 3T Diapers @@ -137,6 +144,7 @@ expect(subject[3]).to eq([ request_with_deleted_items.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + nil, "Fulfilled", 0, # 2T Diapers 0, # 3T Diapers @@ -150,6 +158,7 @@ expect(subject[4]).to eq([ request_with_multiple_items.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + nil, "Started", 3, # 2T Diapers 2, # 3T Diapers @@ -163,6 +172,7 @@ expect(subject[5]).to eq([ request_4t.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Quantity", "Started", 0, # 2T Diapers 0, # 3T Diapers @@ -176,6 +186,7 @@ expect(subject[6]).to eq([ request_4t_pack.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Quantity", "Started", 0, # 2T Diapers 0, # 3T Diapers @@ -190,6 +201,7 @@ expect(subject[6]).to eq([ request_4t_pack.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Quantity", "Started", 0, # 2T Diapers 0, # 3T Diapers @@ -211,6 +223,7 @@ expect(subject.first).to eq([ "Date", "Requestor", + "Type", "Status", "2T Diapers", "3T Diapers", @@ -227,6 +240,7 @@ expect(subject[1]).to eq([ request_3t.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Child", request_3t.status.humanize, 0, # 2T Diapers 150, # 3T Diapers @@ -239,6 +253,7 @@ expect(subject[2]).to eq([ request_2t.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Individual", "Fulfilled", 100, # 2T Diapers 0, # 3T Diapers @@ -251,6 +266,7 @@ expect(subject[3]).to eq([ request_with_deleted_items.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + nil, "Fulfilled", 0, # 2T Diapers 0, # 3T Diapers @@ -263,6 +279,7 @@ expect(subject[4]).to eq([ request_with_multiple_items.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + nil, "Started", 3, # 2T Diapers 2, # 3T Diapers @@ -275,6 +292,7 @@ expect(subject[5]).to eq([ request_4t.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Quantity", "Started", 0, # 2T Diapers 0, # 3T Diapers @@ -287,6 +305,7 @@ expect(subject[6]).to eq([ request_4t_pack.created_at.strftime("%m/%d/%Y").to_s, "Howdy Partner", + "Quantity", "Started", 0, # 2T Diapers 0, # 3T Diapers diff --git a/spec/services/partners/family_request_create_service_spec.rb b/spec/services/partners/family_request_create_service_spec.rb index a51e5ff9a7..1cbe16d5bf 100644 --- a/spec/services/partners/family_request_create_service_spec.rb +++ b/spec/services/partners/family_request_create_service_spec.rb @@ -96,6 +96,28 @@ expect(first_item_request.quantity.to_i).to eq(first_item_request.item.default_quantity) expect(second_item_request.quantity.to_i).to eq(second_item_request.item.default_quantity * 4) end + + context "with for_families false" do + let(:for_families) { false } + + it "creates a request of type individual" do + expect { subject }.to change { Request.count }.by(1) + + partner_request = Request.last + expect(partner_request.request_type).to eq("individual") + end + end + + context "with for_families true" do + let(:for_families) { true } + + it "creates a request of type child" do + expect { subject }.to change { Request.count }.by(1) + + partner_request = Request.last + expect(partner_request.request_type).to eq("child") + end + end end context 'without children' do @@ -120,7 +142,7 @@ let(:fake_request_create_service) { instance_double(Partners::RequestCreateService, call: -> {}, errors: [], partner_request: -> {}) } before do - allow(Partners::RequestCreateService).to receive(:new).with(partner_user_id: partner_user.id, comments: comments, for_families: false, item_requests_attributes: contain_exactly(*expected_item_request_attributes)).and_return(fake_request_create_service) + allow(Partners::RequestCreateService).to receive(:new).with(partner_user_id: partner_user.id, request_type: "individual", comments: comments, item_requests_attributes: contain_exactly(*expected_item_request_attributes)).and_return(fake_request_create_service) end it 'should send the correct request payload to the Partners::RequestCreateService and call it' do diff --git a/spec/services/partners/request_create_service_spec.rb b/spec/services/partners/request_create_service_spec.rb index 6da6d9be68..fbc9be8fb0 100644 --- a/spec/services/partners/request_create_service_spec.rb +++ b/spec/services/partners/request_create_service_spec.rb @@ -4,11 +4,13 @@ let(:args) do { partner_user_id: partner_user.id, + request_type: request_type, comments: comments, item_requests_attributes: item_requests_attributes } end let(:partner_user) { partner.primary_user } + let(:request_type) { nil } let(:partner) { create(:partner) } let(:comments) { Faker::Lorem.paragraph } let(:item_requests_attributes) do @@ -89,6 +91,36 @@ expect(Partners::ItemRequest.count).to eq(item_requests_attributes.count) end + context "when request_type is child" do + let(:request_type) { "child" } + + it "creates a request with of that type" do + expect { subject }.to change { Request.count }.by(1) + + expect(Request.last.request_type).to eq("child") + end + end + + context "when request_type is individual" do + let(:request_type) { "individual" } + + it "creates a request with of that type" do + expect { subject }.to change { Request.count }.by(1) + + expect(Request.last.request_type).to eq("individual") + end + end + + context "when request_type is quantity" do + let(:request_type) { "quantity" } + + it "creates a request with of that type" do + expect { subject }.to change { Request.count }.by(1) + + expect(Request.last.request_type).to eq("quantity") + end + end + context 'when we have duplicate item as part of request' do let(:duplicate_item) { FactoryBot.create(:item) } let(:unique_item) { FactoryBot.create(:item) } @@ -149,12 +181,14 @@ let(:args) do { partner_user_id: partner_user.id, + request_type: request_type, comments: comments, item_requests_attributes: item_requests_attributes } end let(:partner_user) { partner.primary_user } let(:partner) { create(:partner) } + let(:request_type) { "child" } let(:comments) { Faker::Lorem.paragraph } let(:item) { FactoryBot.create(:item) } let(:item_requests_attributes) do