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

add facilities check on mobile appts #18486

Merged
merged 12 commits into from
Sep 23, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ def appointments_cache_interface
def authorize
raise_access_denied unless current_user.authorize(:vaos, :access?)
raise_access_denied_no_icn if current_user.icn.blank?
raise_access_denied_no_facilities unless current_user.authorize(:vaos, :facilities?)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kpethtel , I'm unsure if this warrants a flipper. It seems simple enough but let me know if you feel differently.

end

def raise_access_denied
Expand All @@ -147,6 +148,10 @@ def raise_access_denied_no_icn
raise Common::Exceptions::Forbidden, detail: 'No patient ICN found'
end

def raise_access_denied_no_facilities
raise Common::Exceptions::Forbidden, detail: 'No facility associated with user'
end

def staging_custom_error
if Settings.vsp_environment != 'production' && @current_user.email == 'vets.gov.user+141@gmail.com'
raise Mobile::V0::Exceptions::CustomErrors.new(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
errors:
- title: 'Forbidden'
detail: 'No facility associated with user'
code: '403'
status: '403'
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
errors:
- title: 'Forbidden'
detail: 'No patient ICN found'
code: '403'
status: '403'
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
errors:
- title: 'Forbidden'
detail: 'You do not have access to online scheduling'
code: '403'
status: '403'
11 changes: 7 additions & 4 deletions modules/mobile/docs/index.html

Large diffs are not rendered by default.

13 changes: 12 additions & 1 deletion modules/mobile/docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,18 @@ paths:
'401':
$ref: '#/components/responses/401'
'403':
$ref: '#/components/responses/403'
content:
application/json:
examples:
Missing Facilities:
value:
$ref: ./examples/appointments/forbidden_facilities_error.yml
No VAOS Access:
value:
$ref: ./examples/appointments/forbidden_no_vaos_access_error.yml
No ICN:
value:
$ref: ./examples/appointments/forbidden_no_icn_error.yml
'404':
$ref: '#/components/responses/404'
'408':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
let!(:user) { sis_user(icn: '1012846043V576341') }

before do
allow_any_instance_of(User).to receive(:va_patient?).and_return(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just adding the facilities was not working in this spec as it was transforming facilities into test facilities 984. This change is unique to testing env so not worried about effecting prod performance.

allow_any_instance_of(VAOS::UserService).to receive(:session).and_return('stubbed_token')
allow_any_instance_of(VAOS::V2::MobileFacilityService).to \
receive(:get_clinic).and_return(mock_clinic)
Expand Down Expand Up @@ -196,7 +197,6 @@
VCR.use_cassette('mobile/appointments/post_appointments_va_proposed_clinic_200',
match_requests_on: %i[method uri]) do
post '/mobile/v0/appointment', params: {}, headers: sis_headers

expect(response).to have_http_status(:created)
expect(json_body_for(response)).to match_camelized_schema('vaos/v2/appointment', { strict: false })
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.describe 'Mobile::V0::Appointments::VAOSV2', type: :request do
include JsonSchemaMatchers

let!(:user) { sis_user(icn: '1012846043V576341') }
let!(:user) { sis_user(icn: '1012846043V576341', vha_facility_ids: [402, 555]) }

before do
Flipper.enable_actor(:appointments_consolidation, user)
Expand Down Expand Up @@ -340,7 +340,7 @@
end

context 'when custom error response is injected' do
let!(:user) { sis_user(email: 'vets.gov.user+141@gmail.com') }
let!(:user) { sis_user(email: 'vets.gov.user+141@gmail.com', vha_facility_ids: [402, 555]) }

it 'raises 418 custom error' do
get '/mobile/v0/appointments', headers: sis_headers
Expand Down Expand Up @@ -698,6 +698,23 @@
'status' => '502' })
end
end

context 'appointment authorization' do
context 'when user has no facilities' do
let!(:user) { sis_user(icn: '1012846043V576341', vha_facility_ids: []) }

it 'returns forbidden error' do
VCR.use_cassette('mobile/appointments/VAOS_v2/get_appointment_200', match_requests_on: %i[method uri]) do
get '/mobile/v0/appointments', headers: sis_headers
end

expect(response.parsed_body.dig('errors', 0)).to eq({ 'title' => 'Forbidden',
'detail' => 'No facility associated with user',
'code' => '403',
'status' => '403' })
end
end
end
end
end
end
4 changes: 4 additions & 0 deletions modules/vaos/app/policies/vaos_policy.rb
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a separate check so we can return a unique error that FE can eventually use to trigger a banner saying that they don't have any facilities.

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@
def access?
Flipper.enabled?('va_online_scheduling', user) && user.loa3?
end

def facilities?
Copy link
Contributor

Choose a reason for hiding this comment

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

Would facilities_access? make sense here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah thats better. Updated.

user.va_patient?
end
end
Loading