Skip to content
Open
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
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ Only take multiple issues if they are related and you can solve all of them at t
Users that are frequent contributors and are involved in discussion (join the slack channel! :)) may be given direct Contributor access to the Repo so they can submit Pull Requests directly instead of Forking first.

## Debugging
If starting server directly, via `rail s` or `rail console`, or built-in debugger in RubyMine, or running `bundle exec rspec path/to/spec.rb:line_no`, then you can use `binding.pry` to debug. Drop the pry where you want the execution to pause.
If starting server directly, via `rails s` or `rails console`, or built-in debugger in RubyMine, or running `bundle exec rspec path/to/spec.rb:line_no`, then you can use `binding.pry` to debug. Drop the pry where you want the execution to pause.

If starting via Procfile with `bin/start`, then drop a ``binding.remote_pry`` into the line where you want execution to pause at. Then run ``pry-remote`` in the terminal to connect to it.
https://github.com/Mon-Ouie/pry-remote
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ def index
.undiscarded
.during(helpers.selected_range)
.class_filter(filter_params)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can create an issue to refactor this controller so we don't have all these variables here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Created an issue for this: #5557

if params[:include_cancelled]
@requests = @requests.with_discarded
end

@unfulfilled_requests_count = current_organization.requests.where(status: [:pending, :started]).during(helpers.selected_range).class_filter(filter_params).count
@paginated_requests = @requests.includes(:partner).page(params[:page])
@calculate_product_totals = RequestsTotalItemsService.new(requests: @requests).calculate
Expand All @@ -20,6 +25,7 @@ def index
@selected_request_item = filter_params[:by_request_item_id]
@selected_partner = filter_params[:by_partner]
@selected_status = filter_params[:by_status]
@include_cancelled = params[:include_cancelled]

respond_to do |format|
format.html
Expand Down
12 changes: 10 additions & 2 deletions app/models/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ class Request < ApplicationRecord
accepts_nested_attributes_for :item_requests, allow_destroy: true, reject_if: proc { |attributes| attributes["quantity"].blank? }
has_many :child_item_requests, through: :item_requests

enum :status, { pending: 0, started: 1, fulfilled: 2, discarded: 3 }, prefix: true
enum :status, { pending: 0, started: 1, fulfilled: 2, cancelled: 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
validate :not_completely_empty
validate :cannot_change_status_once_fulfilled, on: :update
validate :cannot_change_status_once_fulfilled,
:cannot_change_status_once_cancelled,
on: :update

after_validation :sanitize_items_data

Expand Down Expand Up @@ -93,4 +95,10 @@ def cannot_change_status_once_fulfilled
errors.add(:status, "cannot be changed once fulfilled")
end
end

def cannot_change_status_once_cancelled
if status_changed? && status_was == "cancelled"
errors.add(:status, "cannot be changed once cancelled")
end
end
end
4 changes: 2 additions & 2 deletions app/services/request_destroy_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def call

request.discarded_at = Time.current
request.discard_reason = reason
request.status = :discarded
request.status = :cancelled
request.save!

unless request.partner.deactivated?
Expand All @@ -29,7 +29,7 @@ def valid?
if request.blank?
errors.add(:base, 'request_id is invalid')
elsif request.discarded_at.present?
errors.add(:base, 'request already discarded')
errors.add(:base, 'request already cancelled')
end

errors.none?
Expand Down
6 changes: 4 additions & 2 deletions app/views/requests/_request_row.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
</td>
<td class="text-right">
<%= view_button_to request_path(request_row) %>
<%= cancel_button_to new_request_cancelation_path(request_id: request_row.id), method: :get, data: { disable_with: "Please wait..." }, size: "xs", type: "danger" %>
<% unless request_row.status_cancelled? %>
<%= cancel_button_to new_request_cancelation_path(request_id: request_row.id), method: :get, data: { disable_with: "Please wait..." }, size: "xs", type: "danger" %>
<% end %>
<%= print_button_to print_picklist_request_path(request_row), { format: :pdf, text: "Print", size: "xs" } %>
</td>
</td>
</tr>
2 changes: 1 addition & 1 deletion app/views/requests/_status.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
<% when "fulfilled" %>
<span class="badge badge-success bg-success">Fulfilled</span>
<% else %>
<span class="badge badge-danger bg-danger">Errored</span>
<span class="badge badge-danger bg-danger">Cancelled</span>
<% end %>
9 changes: 6 additions & 3 deletions app/views/requests/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@
<%= label_tag "Date Range" %>
<%= render partial: "shared/date_range_picker", locals: {css_class: "form-control"} %>
</div>
</div>
</div>
<div class="form-group col-lg-3 col-md-4 col-sm-6 col-xs-12">
<%= filter_checkbox(label: "Include Cancelled?", scope: "include_cancelled", selected: @include_cancelled) %>
</div>
</div>
</div>
<div class="card-footer">
<div class="d-flex flex-wrap justify-content-between gap-2">
<div class="d-flex flex-wrap gap-2">
Expand All @@ -73,7 +76,7 @@
text: "Print Unfulfilled Picklists (#{@unfulfilled_requests_count})",
size: "md") %>
<% end %>
<%= download_button_to(requests_path(format: :csv, filters: filter_params.merge(date_range: date_range_params)), {text: "Export Requests", size: "md"}) if @requests.any? %>
<%= download_button_to(requests_path(format: :csv, include_cancelled: @include_cancelled, filters: filter_params.merge(date_range: date_range_params)), {text: "Export Requests", size: "md"}) if @requests.any? %>
<%= modal_button_to("#newRequest", text: "New Quantity Request", icon: "plus", type: "success") if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
</div>
</div>
Expand Down
10 changes: 7 additions & 3 deletions app/views/requests/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,15 @@
</table>
</div>
<div class="card-footer flex flex-row space-x-2">
<%= submit_button_to start_request_path(@request), {text: "Fulfill request", size: "md"} unless @request.distribution %>
<% unless @request.status_cancelled? %>
<%= submit_button_to start_request_path(@request), {text: "Fulfill request", size: "md"} unless @request.distribution %>
<% end %>
<%= view_button_to(distribution_path(@request.distribution), {text: "View Associated Distribution", size: "md"}) if @request.distribution %>
<%= print_button_to print_picklist_request_path(@request), { format: :pdf, text: "Print", size: "md" } %>
<%= button_to 'Cancel', new_request_cancelation_path(request_id: @request.id),
method: :get, data: { disable_with: "Please wait..." }, form_class: 'd-inline', class: 'btn btn-danger btn-md' %>
<% unless @request.status_cancelled? %>
<%= button_to 'Cancel', new_request_cancelation_path(request_id: @request.id),
method: :get, data: { disable_with: "Please wait..." }, form_class: 'd-inline', class: 'btn btn-danger btn-md' %>
<% end %>
</div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion docs/user_guide/bank/essentials_requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ The list contains:
- pending -- haven't started fulfilling it yet
- started -- have started fulfilling, but haven't saved the resulting distribution
- fulfilled -- have created the distribution for this Request
- discarded -- have cancelled the Request
- cancelled -- have cancelled the Request
- and the actions you can take on that Request

### Filtering your Requests
Expand Down
2 changes: 1 addition & 1 deletion docs/user_guide/bank/exports.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ Click 'Requests' in the left-hand menu, then "Export Requests"
You can filter the exported Requests by the following:
- Item
- Partner
- Status (Pending, Started, Fulfilled, Discarded)
- Status (Pending, Started, Fulfilled, Cancelled)
- Date Range
Make your selection, then click 'Filter' before clicking 'Export Requests'

Expand Down
5 changes: 3 additions & 2 deletions spec/factories/requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ def random_request_items
status { 'pending' }
end

trait :discarded do
status { 'discarded' }
trait :cancelled do
status { 'cancelled' }
discarded_at { Time.current }
end

trait :with_varied_quantities do
Expand Down
26 changes: 26 additions & 0 deletions spec/models/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,32 @@
.to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once fulfilled/)
end

it "does not regress from fulfilled to cancelled" do
expect { request_fulfilled.status_cancelled! }
.to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once fulfilled/)
end

it "does not regress from cancelled to started" do
cancelled_request = create(:request, :cancelled)

expect { cancelled_request.status_started! }
.to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once cancelled/)
end

it "does not regress from cancelled to pending" do
cancelled_request = create(:request, :cancelled)

expect { cancelled_request.status_pending! }
.to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once cancelled/)
end

it "does not regress from cancelled to fulfilled" do
cancelled_request = create(:request, :cancelled)

expect { cancelled_request.status_fulfilled! }
.to raise_error(ActiveRecord::RecordInvalid, /cannot be changed once cancelled/)
end

it "allows normal transitions" do
expect { request_pending.status_started! }.not_to raise_error
expect { request_started.status_fulfilled! }.not_to raise_error
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/partners/requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@
let(:partner_user) { partner1.primary_user }
let!(:pending_request) { create(:request, :with_item_requests, :pending, partner: partner1, request_items: [{ item_id: item1.id, quantity: '100' }]) }
let!(:started_request) { create(:request, :with_item_requests, :started, partner: partner1, request_items: [{ item_id: item2.id, quantity: '50' }]) }
let!(:discarded_request) { create(:request, :with_item_requests, :discarded, partner: partner1, request_items: [{ item_id: item2.id, quantity: '30' }]) }
let!(:cancelled_request) { create(:request, :with_item_requests, :cancelled, partner: partner1, request_items: [{ item_id: item2.id, quantity: '30' }]) }
let!(:fulfilled_request) { create(:request, :with_item_requests, :fulfilled, partner: partner1, request_items: [{ item_id: item2.id, quantity: '20' }]) }

before do
Expand Down
102 changes: 92 additions & 10 deletions spec/requests/requests_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
response
end

before do
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This wasn't being used at all.

create(:request)
end

context "html" do
let(:response_format) { 'html' }

Expand All @@ -25,35 +21,103 @@
let(:response_format) { 'csv' }

it { is_expected.to be_successful }

context 'when exporting as CSV' do
it "exports only the cancelled requests CSV when 'Include Cancelled' is checked and 'Filter by Status' is 'Cancelled'" do
create(:request, :started)
create(:request, :cancelled)

get requests_path(format: :csv, params: {include_cancelled: "1", filters: { by_status: :cancelled}})

csv = CSV.parse(response.body, headers: true)

expect(csv.count).to eq(1)
expect(csv.first["Status"]).to eq("Cancelled")
end

it "exports the requests CSV including cancelled requests when 'Include Cancelled' is checked" do
create(:request, :started)
create(:request, :cancelled)

get requests_path(format: :csv, params: {include_cancelled: "1"})

csv = CSV.parse(response.body, headers: true)

expect(csv.count).to eq(2)
expect(csv[0]["Status"]).to eq("Started")
expect(csv[1]["Status"]).to eq("Cancelled")
end
end
end

context "when there are pending or started requests" do
it "shows print unfulfilled picklists button with correct quantity" do
Request.delete_all

it "shows print unfulfilled picklists button with correct quantity, excluding cancelled requests by default" do
create(:request, :pending)
create(:request, :started)
create(:request, :fulfilled)
create(:request, :discarded)
create(:request, :cancelled)

get requests_path

expect(response.body).to include('Print Unfulfilled Picklists (2)')
expect(response.body).not_to match(%r{<span class="badge badge-danger bg-danger">\s*Cancelled\s*</span>})
end
end

context "when 'include_cancelled' param is present" do
it 'does not display the Cancel button for cancelled requests' do
pending_request = create(:request, :pending)
cancelled_request = create(:request, :cancelled)

get requests_path, params: {include_cancelled: "1"}

page = Nokogiri::HTML(response.body)

cancelled_request_cancel_button = page.at_css("form[action='/requests/#{cancelled_request.id}/cancelation/new']")
expect(cancelled_request_cancel_button).to be_nil

pending_request_cancel_button = page.at_css("a[href='/requests/#{pending_request.id}/cancelation/new']")
expect(pending_request_cancel_button).to be_present
end

it "shows print unfulfilled picklists button with correct quantity including cancelled requests" do
create(:request, :pending)
create(:request, :started)
create(:request, :cancelled)

get requests_path, params: {include_cancelled: "1"}

expect(response.body).to include('Print Unfulfilled Picklists (2)')
expect(response.body).to match(%r{<span class="badge badge-danger bg-danger">\s*Cancelled\s*</span>})
end
end

context "when 'Include Cancelled?' is checked and filter by Cancelled" do
it "constrains the list for cancelled requests only" do
create(:request, :started, comments: "Need more supplies")
create(:request, :pending, comments: "Awaiting for confirmation")
create(:request, :cancelled, comments: 'Not necessary anymore')

get requests_path, params: {include_cancelled: "1", filters: { by_status: :cancelled}}

expect(response.body).to include("Not necessary anymore")
expect(response.body).not_to include("Need more supplies")
expect(response.body).not_to include("Awaiting for confirmation")
end
end

context "when there is a filter applied" do
it "shows only filtered requests, print unfulfilled picklists button with correct quantity" do
Request.delete_all

create(:request, :started, comments: "Started request - should appear")
create(:request, :pending, comments: "Pending request - should not appear")
create(:request, :cancelled, comments: 'Cancelled request - a comment')

get requests_path({ filters: { by_status: :started} })

expect(response.body).to include("Print Unfulfilled Picklists (1)")
expect(response.body).to include("Started request - should appear")
expect(response.body).not_to include("Pending request - should not appear")
expect(response.body).not_to include("Cancelled request - a comment")
end
end
end
Expand Down Expand Up @@ -156,6 +220,24 @@
expect(response.body).not_to include('Units (if applicable)')
end
end

context 'when the request has a cancelled status' do
it 'does not display the Cancel and Fulfill request buttons' do
cancelled_request = create(:request, :cancelled, organization:)

get request_path(cancelled_request)

page = Nokogiri::HTML(response.body)

cancel_button = page.at_css("form[action='/requests/#{cancelled_request.id}/cancelation/new']")
fulfill_button = page.at_css("form[action='/requests/#{cancelled_request.id}/start']")
expect(cancel_button).to be_nil
expect(fulfill_button).to be_nil

print_link = page.at_css("a[href='/requests/#{cancelled_request.id}/print_picklist']")
expect(print_link).to be_present
end
end
end

describe 'POST #start' do
Expand Down
8 changes: 4 additions & 4 deletions spec/services/request_destroy_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
end
end

context 'when the request is already discarded' do
context 'when the request is already cancelled' do
before do
request.discard!
end

it 'should not be successful and have errors' do
expect(subject.errors.full_messages).to eq(['request already discarded'])
expect(subject.errors.full_messages).to eq(['request already cancelled'])
end
end

Expand All @@ -37,7 +37,7 @@
end

it 'should update the status column on the request' do
expect { subject }.to change { request.reload.status_discarded? }.from(false).to(true)
expect { subject }.to change { request.reload.status_cancelled? }.from(false).to(true)
end

it 'should send a email notification to the partner' do
Expand All @@ -51,7 +51,7 @@
let(:request) { create(:request, partner: partner) }

it 'should update the status column on the request' do
expect { subject }.to change { request.reload.status_discarded? }.from(false).to(true)
expect { subject }.to change { request.reload.status_cancelled? }.from(false).to(true)
end

it 'should not send a email notification to the partner' do
Expand Down
4 changes: 2 additions & 2 deletions spec/system/dashboard_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@
# expect(org_dashboard_page.outstanding_requests).to be_empty
end

it "does not display a discarded request" do
create :request, :discarded
it "does not display a cancelled request" do
create :request, :cancelled
org_dashboard_page.visit
org_dashboard_page.outstanding_section # wait for the section
expect(org_dashboard_page.outstanding_requests).to be_empty
Expand Down
Loading
Loading