Skip to content

Commit

Permalink
chore: Hide cross-deck notifications behind feature flag
Browse files Browse the repository at this point in the history
FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS should be set to a
comma-separated list, e.g. "geoamey,serco"

[MAP-322]
  • Loading branch information
jimbali committed Jun 12, 2024
1 parent 8c5a674 commit fabb58a
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 19 deletions.
9 changes: 8 additions & 1 deletion app/jobs/prepare_base_notifications_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,17 @@ def subscriptions(move, action_name:, only_supplier_id: nil)
end

def build_and_send_notifications(subscription, type_id, topic, action_name, queue_as)
type = event_type(action_name, topic, type_id, subscription)

if type.starts_with?('cross_supplier_')
enabled_suppliers = ENV.fetch('FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS', '').split(',')
return unless enabled_suppliers.include?(subscription.supplier.key)
end

notification = subscription.notifications.create!(
notification_type_id: type_id,
topic:,
event_type: event_type(action_name, topic, type_id, subscription),
event_type: type,
)
notify_job(type_id).perform_later(notification_id: notification.id, queue_as:)
end
Expand Down
2 changes: 1 addition & 1 deletion helm_deploy/values-staging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ generic-service:
SENTRY_ENVIRONMENT: "staging"
SERVER_FQDN: "hmpps-book-secure-move-api-staging.apps.cloud-platform.service.justice.gov.uk"
WEB_CONCURRENCY: "3"

FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: "geoamey,serco"

# Pre-existing kubernetes secrets to load as environment variables in the deployment.
# namespace_secrets:
Expand Down
1 change: 0 additions & 1 deletion helm_deploy/values-uat.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ generic-service:
SENTRY_ENVIRONMENT: "uat"
SERVER_FQDN: "hmpps-book-secure-move-api-uat.apps.cloud-platform.service.justice.gov.uk"


# Pre-existing kubernetes secrets to load as environment variables in the deployment.
# namespace_secrets:
# [name of kubernetes secret]:
Expand Down
84 changes: 73 additions & 11 deletions spec/jobs/prepare_move_notifications_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
let(:send_webhooks) { true }
let(:send_emails) { true }
let(:only_supplier_id) { nil }
let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: 'geoamey,serco' } }

before do
create(:notification_type, :webhook)
Expand All @@ -24,6 +25,12 @@
allow(NotifyEmailJob).to receive(:perform_later)
end

around do |example|
ClimateControl.modify(**envs) do
example.run
end
end

shared_examples_for 'it creates a webhook notification record' do
it do
expect { perform }.to change(Notification.webhooks, :count).by(1)
Expand Down Expand Up @@ -99,8 +106,8 @@
end

context 'when it is a cross-deck move' do
let!(:initial_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier) }
let!(:initial_supplier) { create(:supplier, :serco) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier: initial_supplier) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand All @@ -113,6 +120,17 @@
[subscription2.id, 'cross_supplier_move_add'],
)
end

context 'when the feature flag is not set' do
let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: '' } }

it 'only sends the create_move notification to the initial supplier' do
perform
expect(Notification.webhooks.order(:created_at).pluck(:subscription_id, :event_type)).to contain_exactly(
[subscription.id, 'create_move'],
)
end
end
end
end

Expand Down Expand Up @@ -145,8 +163,8 @@
end

context 'when it is updated to become a cross-deck move' do
let!(:initial_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier) }
let!(:initial_supplier) { create(:supplier, :serco) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier: initial_supplier) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand All @@ -159,11 +177,22 @@
[subscription2.id, 'cross_supplier_move_add'],
)
end

context 'when the feature flag is not set' do
let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: '' } }

it 'only sends the update_move notification to the initial supplier' do
perform
expect(Notification.webhooks.order(:created_at).pluck(:subscription_id, :event_type)).to contain_exactly(
[subscription.id, 'update_move'],
)
end
end
end

context 'when it is a cross-deck move that has already been notified' do
let!(:initial_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier) }
let!(:initial_supplier) { create(:supplier, :serco) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier: initial_supplier) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand All @@ -172,11 +201,26 @@

it 'sends the update_move and cross_supplier_move_update notifications to the suppliers' do
perform
expect(Notification.webhooks.order(:created_at).last(2).pluck(:subscription_id, :event_type)).to contain_exactly(
expect(
Notification.webhooks.order(:created_at).where.not(event_type: 'cross_supplier_move_add').pluck(:subscription_id, :event_type),
).to contain_exactly(
[subscription.id, 'update_move'],
[subscription2.id, 'cross_supplier_move_update'],
)
end

context 'when the feature flag is not set' do
let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: '' } }

it 'only sends the update_move notification to the initial supplier' do
perform
expect(
Notification.webhooks.order(:created_at).where.not(event_type: 'cross_supplier_move_add').pluck(:subscription_id, :event_type),
).to contain_exactly(
[subscription.id, 'update_move'],
)
end
end
end
end

Expand Down Expand Up @@ -242,8 +286,8 @@

context 'when explicitly notifying a cross-deck move' do
let(:action_name) { 'cross_supplier_add' }
let!(:initial_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier) }
let!(:initial_supplier) { create(:supplier, :serco) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier: initial_supplier) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand All @@ -255,12 +299,21 @@
[subscription2.id, 'cross_supplier_move_add'],
)
end

context 'when the feature flag is not set' do
let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: '' } }

it 'does not notify the receiving supplier' do
perform
expect(Notification.webhooks.order(:created_at).pluck(:subscription_id, :event_type)).to be_empty
end
end
end

context 'when explicitly notifying that a move is no longer cross-deck' do
let(:action_name) { 'cross_supplier_remove' }
let!(:initial_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier) }
let!(:initial_supplier) { create(:supplier, :serco) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier: initial_supplier) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand All @@ -272,6 +325,15 @@
expect(Notification.webhooks.where.not(event_type: 'cross_supplier_move_add').pluck(:subscription_id, :event_type))
.to contain_exactly([subscription2.id, 'cross_supplier_move_remove'])
end

context 'when the feature flag is not set' do
let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: '' } }

it 'does not notify the receiving supplier' do
perform
expect(Notification.webhooks.where.not(event_type: 'cross_supplier_move_add').pluck(:subscription_id, :event_type)).to be_empty
end
end
end

context 'when confirming a person escort record' do
Expand Down
10 changes: 9 additions & 1 deletion spec/requests/api/allocations_controller_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
resource
end

let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: 'geoamey,serco' } }

around do |example|
ClimateControl.modify(**envs) do
example.run
end
end

describe 'PATCH /allocations' do
subject(:patch_allocations) do
patch "/api/allocations/#{allocation.id}", params: { data: }, headers:, as: :json
Expand All @@ -25,7 +33,7 @@
let(:headers) { { 'Authorization' => "Bearer #{access_token}", 'CONTENT_TYPE': content_type } }
let(:existing_date) { Date.new(2023, 1, 1) }
let(:new_date) { existing_date.tomorrow }
let(:supplier) { create(:supplier) }
let(:supplier) { create(:supplier, :serco) }
let!(:allocation) { create(:allocation, date: existing_date, moves_count:) }
let!(:moves) { create_list(:move, moves_count, allocation:, date: existing_date, person: create(:person), supplier:) }

Expand Down
12 changes: 10 additions & 2 deletions spec/requests/api/moves_controller_create_v2_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

let(:response_json) { JSON.parse(response.body) }
let(:schema) { load_yaml_schema('post_moves_responses.yaml', version: 'v2') }
let(:supplier) { create(:supplier) }
let(:supplier) { create(:supplier, :serco) }
let(:access_token) { 'spoofed-token' }
let(:content_type) { ApiController::CONTENT_TYPE }

Expand All @@ -25,6 +25,14 @@
}
end

let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: 'geoamey,serco' } }

around do |example|
ClimateControl.modify(**envs) do
example.run
end
end

describe 'POST /moves' do
let(:move_attributes) do
{
Expand Down Expand Up @@ -737,7 +745,7 @@
allow(Faraday).to receive(:new).and_return(faraday_client)
end

let!(:receiving_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier: another_supplier) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand Down
12 changes: 10 additions & 2 deletions spec/requests/api/moves_controller_update_v2_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
let(:response_json) { JSON.parse(response.body) }
let(:schema) { load_yaml_schema('patch_move_responses.yaml', version: 'v2') }
let(:access_token) { 'spoofed-token' }
let(:supplier) { create(:supplier) }
let(:supplier) { create(:supplier, :serco) }

let(:resource_to_json) do
JSON.parse(V2::MoveSerializer.new(move.reload, serializer: V2::MoveSerializer).serializable_hash.to_json)
Expand All @@ -24,6 +24,14 @@
}
end

let(:envs) { { FEATURE_FLAG_CROSS_DECK_NOTIFICATIONS_SUPPLIERS: 'geoamey,serco' } }

around do |example|
ClimateControl.modify(**envs) do
example.run
end
end

describe 'PATCH /moves' do
let!(:move) { create :move, :proposed, :prison_recall, from_location:, profile:, supplier: }

Expand Down Expand Up @@ -341,7 +349,7 @@
end

context 'when it is a cross-deck move' do
let!(:receiving_supplier) { create(:supplier) }
let!(:receiving_supplier) { create(:supplier, :geoamey) }
let!(:subscription) { create(:subscription, :no_email_address, supplier:) }
let!(:subscription2) { create(:subscription, :no_email_address, supplier: receiving_supplier) }
let(:to_location) { create :location, :court, suppliers: [receiving_supplier] }
Expand Down

0 comments on commit fabb58a

Please sign in to comment.