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.

Sign up for GitHub

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

Remove Unused Settings - Part 1 #19096

Merged
merged 3 commits into from
Nov 19, 2024
Merged

Remove Unused Settings - Part 1 #19096

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a72def
remove unused settings
stevenjcumming Oct 25, 2024
d605a38
remove unused settings
stevenjcumming Oct 25, 2024
1d80512
Merge branch 'master' into sjc-remove-unused-settings-1
stevenjcumming Nov 19, 2024
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
Next Next commit
remove unused settings
  • Loading branch information
@stevenjcumming
stevenjcumming committed Oct 25, 2024
commit 1a72deff240d46ec7e787ec977662b63eed90f9c
19 changes: 0 additions & 19 deletions config/settings.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,10 @@ sign_in:

terms_of_use:
current_version: v1
provisioner_cookie_domain: localhost
Copy link
Contributor

Choose a reason for hiding this comment

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

Riley added this last year in #14455. @rileyanderson, we don't see Settings.terms_of_use.provisioner_cookie_domain called anywhere. I just wanted to check with you before we deleted this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Talked to Riley and provisioner_cookie_domain isn't being used

enabled_clients: vaweb, mhv, myvahealth

lockbox:
master_key: "0d78eaf0e90d4e7b8910c9112e16e66d8b00ec4054a89aa426e32712a13371e9"
previous_master_key: "a798f118b400a1a7552597cc94f44c577919c76b3d26c1eed8ae0703f4240216"

binaries:
# you can specify a full path in settings.local.yml if necessary
Expand All @@ -106,8 +104,6 @@ clamav:
host: 'clamav'
port: '3310'

db_encryption_key: f01ff8ebd1a2b053ad697ae1f0d86adb

database_url: postgis:///vets-api
test_database_url: postgis:///vets-api-test

Expand Down Expand Up @@ -159,19 +155,14 @@ salesforce-gibft:
url: "https://va--rdtcddev.cs33.my.salesforce.com/"

salesforce-carma:
mock: true
url: "https://fake-carma.salesforce.com"
consumer_key: 12345ABC.QQ_1234BBfake
signing_key_path: "spec/fixtures/carma/privatekey.pem" # A dummy private key used for test suite
username: fake-user@va.gov.carmafake

salesforce:
env: dev

form_10_10cg:
carma:
mulesoft:
mock: false
host: ~
client_id: ~
client_secret: ~
Expand Down Expand Up @@ -653,12 +644,6 @@ claims_api:
content_type: 'application/vnd.api+json'
report_enabled: false
audit_enabled: false
s3:
enabled: false
aws_access_key_id: ~
aws_secret_access_key: ~
region: ~
bucket: ~
disability_claims_mock_override: false
schema_dir: config/schemas
slack:
Expand Down Expand Up @@ -838,7 +823,6 @@ iam_ssoe:
client_id: "Mobile_App_API_Server_LOWERS"
# oauth (rest) and sts (soap) services are on different hosts
oauth_url: "https://int.fed.eauth.va.gov:444"
sts_url: "https://preprod.services.eauth.va.gov:9301"
timeout: 20

statsd:
Expand Down Expand Up @@ -1622,9 +1606,6 @@ rrd:
alerts:
recipients:
- fake_email
event_tracking:
recipients:
- fake_email
mas_tracking:
recipients:
- fake_email
Expand Down
9 changes: 0 additions & 9 deletions config/settings/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ mvi:
url: http://www.example.com/
mock: false

locators:
providers_enabled: true

reports:
aws:
access_key_id: key
Expand Down Expand Up @@ -270,9 +267,6 @@ rrd:
alerts:
recipients:
- fake_email
event_tracking:
recipients:
- fake_email

sidekiq:
github_organization: "organization"
Expand Down Expand Up @@ -383,7 +377,6 @@ lgy:
form_10_10cg:
carma:
mulesoft:
mock: true
host: "https://fake-mulesoft.vapi.va.gov"
client_id: BEEFCAFE1234
client_secret: C0FFEEFACE4321
Expand All @@ -403,7 +396,6 @@ vha:

mhv:
medical_records:
app_id: 0
app_token: xyz
host: http://example.org

Expand All @@ -413,7 +405,6 @@ evss:

claims_api:
evss_container:
auth_base_name: staging-api.va.gov
client_key: fakekeyvaluehere
pdf_generator_526:
url: 'https://fake_pdf_url.com'
Expand Down
1 change: 1 addition & 0 deletions lib/gibft/configuration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

module Gibft
class Configuration < Salesforce::Configuration
# Settings.salesforce-gibft
SALESFORCE_INSTANCE_URL = Settings['salesforce-gibft'].url

def service_name
Expand Down
1 change: 1 addition & 0 deletions lib/gibft/service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Gibft
class Service < Salesforce::Service
configuration Gibft::Configuration

# Settings.salesforce-gibft
CONSUMER_KEY = Settings['salesforce-gibft'].consumer_key
SIGNING_KEY_PATH = Settings['salesforce-gibft'].signing_key_path
SALESFORCE_USERNAMES = {
Expand Down
8 changes: 3 additions & 5 deletions modules/appeals_api/spec/sidekiq/higher_level_review_upload_status_batch_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,9 @@
let(:upload) { create(:higher_level_review_v1, status: 'received') }

it 'ignores them' do
with_settings(Settings.modules_appeals_api, higher_level_review_updater_enabled: true) do
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that unlike the subsequent files, this batch version's Settings will need to be replaced with the relevant Flipper flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mathisto are you saying you would like me to leave with_settings... in the spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mathisto Can you be more specific about what changes you want here?

Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
upload.reload
expect(upload.status).to eq('received')
end
Sidekiq::Testing.inline! { AppealsApi::HigherLevelReviewUploadStatusBatch.new.perform }
upload.reload
expect(upload.status).to eq('received')
end
end

Expand Down
20 changes: 8 additions & 12 deletions modules/appeals_api/spec/sidekiq/higher_level_review_upload_status_updater_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@
in_process_element[0]['uuid'] = upload.id
expect(faraday_response).to receive(:body).at_least(:once).and_return([in_process_element].to_json)

with_settings(Settings.modules_appeals_api, higher_level_review_updater_enabled: true) do
AppealsApi::HigherLevelReviewUploadStatusUpdater.new.perform([upload])
upload.reload
expect(upload.status).to eq('processing')
end
AppealsApi::HigherLevelReviewUploadStatusUpdater.new.perform([upload])
upload.reload
expect(upload.status).to eq('processing')
end

it 'notifies sentry & slack of individual bad records without affecting good records' do
Expand All @@ -51,13 +49,11 @@
expect_any_instance_of(AppealsApi::CentralMailUpdater).to receive(:log_exception_to_sentry).once
expect_any_instance_of(AppealsApi::Slack::Messager).to receive(:notify!).once

with_settings(Settings.modules_appeals_api, higher_level_review_updater_enabled: true) do
AppealsApi::HigherLevelReviewUploadStatusUpdater.new.perform([bad_upload, upload])
upload.reload
bad_upload.reload
expect(upload.status).to eq('processing')
expect(bad_upload.status).to eq('submitting')
end
AppealsApi::HigherLevelReviewUploadStatusUpdater.new.perform([bad_upload, upload])
upload.reload
bad_upload.reload
expect(upload.status).to eq('processing')
expect(bad_upload.status).to eq('submitting')
end
end
end