Skip to content

Commit 1a59580

Browse files
committed
Enhance POA Request Filtering, Sorting, and Validation
- Refactored `index` method in `PowerOfAttorneyRequestsController` to improve parameter handling and validation. - Introduced sorting support (`sorted_by`) for `PowerOfAttorneyRequest` model. - Moved `Statuses` constants to `ParamsSchema` for centralized validation. - Updated `ParamsSchema` to enforce allowed sort fields and default sorting behavior. - Improved `decisioned` scope with `left_outer_joins` for better query handling. - Added comprehensive RSpec tests for sorting, status filtering, and schema validation. - Ensured proper handling of invalid parameters with meaningful error messages.
1 parent 9c094ff commit 1a59580

File tree

6 files changed

+342
-53
lines changed

6 files changed

+342
-53
lines changed

modules/accredited_representative_portal/app/controllers/accredited_representative_portal/v0/power_of_attorney_requests_controller.rb

+23-27
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,32 @@ class PowerOfAttorneyRequestsController < ApplicationController
1616

1717
# rubocop:disable Metrics/MethodLength
1818
def index
19-
# Validate and normalize pagination parameters
20-
validated_params = PowerOfAttorneyRequestService::ParamsSchema.validate_and_normalize!(params.to_unsafe_h)
21-
page_params = validated_params[:page]
19+
schema = PowerOfAttorneyRequestService::ParamsSchema
20+
validated_params = schema.validate_and_normalize!(params.to_unsafe_h)
21+
page_params = validated_params.fetch(:page, {})
22+
sort_params = validated_params.fetch(:sort, {})
23+
status = validated_params.fetch(:status, nil)
2224

2325
relation = policy_scope(PowerOfAttorneyRequest)
24-
status = params[:status].presence
25-
relation =
26-
case status
27-
when Statuses::PENDING
28-
pending(relation)
29-
when Statuses::PROCESSED
30-
processed(relation)
31-
when NilClass
32-
relation
33-
else
34-
raise ActionController::BadRequest, <<~MSG.squish
35-
Invalid status parameter.
36-
Must be one of (#{Statuses::ALL.join(', ')})
37-
MSG
38-
end
3926

40-
poa_requests = relation.includes(scope_includes)
41-
.paginate(page: page_params[:number], per_page: page_params[:size])
27+
relation = case status
28+
when schema::Statuses::PENDING
29+
pending(relation)
30+
when schema::Statuses::PROCESSED
31+
processed(relation)
32+
when NilClass
33+
relation
34+
else
35+
raise ActionController::BadRequest, <<~MSG.squish
36+
Invalid status parameter.
37+
Must be one of (#{Statuses::ALL.join(', ')})
38+
MSG
39+
end
40+
41+
poa_requests = relation
42+
.then { |it| sort_params.present? ? it.sorted_by(sort_params[:by], sort_params[:order]) : it }
43+
.includes(scope_includes)
44+
.paginate(page: page_params[:number], per_page: page_params[:size])
4245

4346
serializer = PowerOfAttorneyRequestSerializer.new(poa_requests)
4447
render json: {
@@ -55,13 +58,6 @@ def show
5558

5659
private
5760

58-
module Statuses
59-
ALL = [
60-
PENDING = 'pending',
61-
PROCESSED = 'processed'
62-
].freeze
63-
end
64-
6561
def pending(relation)
6662
relation
6763
.not_processed

modules/accredited_representative_portal/app/models/accredited_representative_portal/power_of_attorney_request.rb

+19-5
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,25 @@ def mark_replaced!(superseding_power_of_attorney_request)
104104
scope :resolved, -> { joins(:resolution) }
105105

106106
scope :decisioned, lambda {
107-
where(
108-
resolution: {
109-
resolving_type: PowerOfAttorneyRequestDecision.to_s
110-
}
111-
)
107+
left_outer_joins(:resolution)
108+
.where(
109+
resolution: {
110+
resolving_type: PowerOfAttorneyRequestDecision.to_s
111+
}
112+
)
113+
}
114+
115+
scope :sorted_by, lambda { |sort_column, direction = :asc|
116+
case sort_column
117+
when 'created_at'
118+
order(created_at: direction)
119+
when 'resolved_at'
120+
includes(:resolution)
121+
.references(:resolution)
122+
.order(Arel.sql("ar_power_of_attorney_request_resolutions.created_at #{direction} NULLS LAST"))
123+
else
124+
raise ArgumentError, "Invalid sort column: #{sort_column}"
125+
end
112126
}
113127

114128
concerning :ProcessedScopes do

modules/accredited_representative_portal/app/services/accredited_representative_portal/power_of_attorney_request_service/params_schema.rb

+26-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ module ParamsSchema
77
Dry::Schema.load_extensions(:json_schema)
88
Dry::Schema.load_extensions(:hints)
99

10+
ALLOWED_SORT_FIELDS = %w[created_at updated_at].freeze
11+
1012
module Page
1113
module Size
1214
MIN = 10
@@ -15,6 +17,19 @@ module Size
1517
end
1618
end
1719

20+
module Sort
21+
ALLOWED_FIELDS = %w[created_at].freeze
22+
ALLOWED_ORDERS = %w[asc desc].freeze
23+
DEFAULT_ORDER = 'desc'
24+
end
25+
26+
module Statuses
27+
ALL = [
28+
PENDING = 'pending',
29+
PROCESSED = 'processed'
30+
].freeze
31+
end
32+
1833
Schema = Dry::Schema.Params do
1934
optional(:page).hash do
2035
optional(:number).value(:integer, gteq?: 1)
@@ -24,7 +39,13 @@ module Size
2439
lteq?: Page::Size::MAX
2540
)
2641
end
27-
# Future PR: Add filter and sort schemas
42+
43+
optional(:sort).hash do
44+
optional(:by).value(:string, included_in?: Sort::ALLOWED_FIELDS)
45+
optional(:order).value(:string, included_in?: Sort::ALLOWED_ORDERS)
46+
end
47+
48+
optional(:status).value(:string, included_in?: Statuses::ALL)
2849
end
2950

3051
class << self
@@ -46,6 +67,10 @@ def apply_defaults(validated_params)
4667
validated_params[:page] ||= {}
4768
validated_params[:page][:number] ||= 1
4869
validated_params[:page][:size] ||= Page::Size::DEFAULT
70+
71+
if validated_params[:sort].present? && validated_params[:sort][:by].present?
72+
validated_params[:sort][:order] ||= Sort::DEFAULT_ORDER
73+
end
4974
end
5075
end
5176
end

modules/accredited_representative_portal/spec/models/accredited_representative_portal/power_of_attorney_request_spec.rb

+102-15
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,109 @@
33
require 'rails_helper'
44

55
RSpec.describe AccreditedRepresentativePortal::PowerOfAttorneyRequest, type: :model do
6-
it 'validates its form and claimant type' do
7-
poa_request =
8-
build(
9-
:power_of_attorney_request,
10-
power_of_attorney_form: build(
11-
:power_of_attorney_form,
12-
data: {}.to_json
13-
),
14-
power_of_attorney_holder_type: 'abc'
6+
describe 'associations' do
7+
it 'validates its form and claimant type' do
8+
poa_request =
9+
build(
10+
:power_of_attorney_request,
11+
power_of_attorney_form: build(
12+
:power_of_attorney_form,
13+
data: {}.to_json
14+
),
15+
power_of_attorney_holder_type: 'abc'
16+
)
17+
18+
expect(poa_request).not_to be_valid
19+
expect(poa_request.errors.full_messages).to contain_exactly(
20+
'Claimant type is not included in the list',
21+
'Power of attorney holder type is not included in the list',
22+
'Power of attorney form data does not comply with schema'
1523
)
24+
end
25+
end
26+
27+
describe 'scopes' do
28+
let(:time) { Time.zone.parse('2024-12-21T04:45:37.000Z') }
29+
30+
let(:poa_code) { 'x23' }
31+
32+
let(:pending1) { create(:power_of_attorney_request, created_at: time, poa_code:) }
33+
let(:pending2) { create(:power_of_attorney_request, created_at: time + 1.day, poa_code:) }
34+
let(:pending3) { create(:power_of_attorney_request, created_at: time + 2.days, poa_code:) }
35+
36+
let(:accepted_request) do
37+
create(:power_of_attorney_request, :with_acceptance,
38+
resolution_created_at: time,
39+
created_at: time,
40+
poa_code:)
41+
end
42+
43+
let(:declined_request) do
44+
create(:power_of_attorney_request, :with_declination,
45+
resolution_created_at: time + 1.day,
46+
created_at: time + 1.day,
47+
poa_code:)
48+
end
49+
50+
let(:expired_request) do
51+
create(:power_of_attorney_request, :with_expiration,
52+
resolution_created_at: time + 2.days,
53+
created_at: time + 2.days,
54+
poa_code:)
55+
end
56+
57+
describe '.sorted_by' do
58+
context 'using created_at column' do
59+
before do
60+
pending1
61+
pending2
62+
pending3
63+
end
64+
65+
it 'sorts by creation date ascending' do
66+
result = described_class.sorted_by('created_at', :asc)
67+
68+
expect(result.first).to eq(pending1)
69+
expect(result.last).to eq(pending3)
70+
end
71+
72+
it 'sorts by creation date descending' do
73+
result = described_class.sorted_by('created_at', :desc)
74+
75+
expect(result.first).to eq(pending3)
76+
expect(result.last).to eq(pending1)
77+
end
78+
end
79+
80+
context 'using resolution date' do
81+
before do
82+
accepted_request
83+
declined_request
84+
expired_request
85+
end
86+
87+
it 'sorts by resolution date ascending' do
88+
result = described_class.where.not(resolution: nil).sorted_by('resolved_at', :asc)
89+
90+
expect(result.first).to eq(accepted_request)
91+
expect(result.second).to eq(declined_request)
92+
expect(result.third).to eq(expired_request)
93+
end
94+
95+
it 'sorts by resolution date descending' do
96+
result = described_class.where.not(resolution: nil).sorted_by('resolved_at', :desc)
97+
98+
expect(result.first).to eq(expired_request)
99+
expect(result.second).to eq(declined_request)
100+
expect(result.third).to eq(accepted_request)
101+
end
102+
end
16103

17-
expect(poa_request).not_to be_valid
18-
expect(poa_request.errors.full_messages).to contain_exactly(
19-
'Claimant type is not included in the list',
20-
'Power of attorney holder type is not included in the list',
21-
'Power of attorney form data does not comply with schema'
22-
)
104+
context 'with invalid column' do
105+
it 'raises argument error' do
106+
expect { described_class.sorted_by('invalid_column') }.to raise_error(ArgumentError)
107+
end
108+
end
109+
end
23110
end
24111
end

modules/accredited_representative_portal/spec/requests/accredited_representative_portal/v0/power_of_attorney_requests_spec.rb

+55-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,61 @@ def load_response_fixture(path_suffix)
6969
expect(parsed_response['data'].map { |p| p['id'] }).not_to include(other_poa_request.id)
7070
end
7171

72-
describe 'a variety of POA request configurations' do
72+
describe 'sorting' do
73+
let!(:poa_requests) do
74+
[
75+
create(:power_of_attorney_request, :with_veteran_claimant,
76+
created_at: time.to_time - 2.days,
77+
poa_code:),
78+
create(:power_of_attorney_request, :with_veteran_claimant,
79+
created_at: time.to_time - 1.day,
80+
poa_code:),
81+
create(:power_of_attorney_request, :with_veteran_claimant,
82+
created_at: time.to_time - 3.days,
83+
poa_code:)
84+
]
85+
end
86+
87+
it 'sorts by created_at in ascending order' do
88+
get('/accredited_representative_portal/v0/power_of_attorney_requests',
89+
params: { sort: { by: 'created_at', order: 'asc' } })
90+
91+
expect(response).to have_http_status(:ok)
92+
93+
# check that they're sorted by created_at in ascending order
94+
ids = parsed_response.to_h['data'].map { |item| item['id'] }[0..2]
95+
expect(ids).to eq([poa_requests[2].id, poa_requests[0].id, poa_requests[1].id])
96+
end
97+
98+
it 'sorts by created_at in descending order' do
99+
get('/accredited_representative_portal/v0/power_of_attorney_requests',
100+
params: { sort: { by: 'created_at', order: 'desc' } })
101+
102+
expect(response).to have_http_status(:ok)
103+
104+
# check that they're sorted by created_at in descending order
105+
ids = parsed_response.to_h['data'].map { |item| item['id'] }[1..3]
106+
expect(ids).to eq([poa_requests[1].id, poa_requests[0].id, poa_requests[2].id])
107+
end
108+
109+
it 'returns error for invalid sort field' do
110+
get('/accredited_representative_portal/v0/power_of_attorney_requests',
111+
params: { sort: { by: 'invalid_field' } })
112+
113+
expect(response).to have_http_status(:bad_request)
114+
expect(parsed_response.to_h['errors']).to include(/Invalid parameters/)
115+
end
116+
117+
it 'returns error for invalid sort order' do
118+
get('/accredited_representative_portal/v0/power_of_attorney_requests',
119+
params: { sort: { by: 'created_at', order: 'invalid' } })
120+
121+
expect(response).to have_http_status(:bad_request)
122+
expect(parsed_response.to_h['errors']).to include(/Invalid parameters/)
123+
end
124+
end
125+
126+
describe 'a variety of poa request configurations' do
73127
let(:poa_requests) do
74128
[].tap do |memo|
75129
memo <<

0 commit comments

Comments
 (0)