Browse Source

Remove flags that have been enabled for a while (#1639)

* Remove flags that have been enabled for a while

* Remove destroy from API actions
pull/1644/head
Jack 2 years ago committed by GitHub
parent
commit
5a5d0d4f79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      app/controllers/logs_controller.rb
  2. 6
      app/helpers/filters_helper.rb
  3. 26
      app/helpers/navigation_items_helper.rb
  4. 2
      app/models/organisation.rb
  5. 12
      app/services/feature_toggle.rb
  6. 2
      app/views/logs/index.html.erb
  7. 11
      spec/features/sales_log_spec.rb
  8. 335
      spec/helpers/navigation_items_helper_spec.rb

2
app/controllers/logs_controller.rb

@ -36,7 +36,7 @@ private
raise "implement in sub class"
end
API_ACTIONS = %w[create show update destroy].freeze
API_ACTIONS = %w[create show update].freeze
def json_api_request?
API_ACTIONS.include?(request["action"]) && request.format.json?

6
app/helpers/filters_helper.rb

@ -31,10 +31,6 @@ module FiltersHelper
end
def collection_year_options
if FeatureToggle.collection_2023_2024_year_enabled?
{ "2023": "2023/24", "2022": "2022/23", "2021": "2021/22" }
else
{ "2022": "2022/23", "2021": "2021/22" }
end
{ "2023": "2023/24", "2022": "2022/23", "2021": "2021/22" }
end
end

26
app/helpers/navigation_items_helper.rb

@ -7,27 +7,27 @@ module NavigationItemsHelper
NavigationItem.new("Organisations", organisations_path, organisations_current?(path)),
NavigationItem.new("Users", "/users", users_current?(path)),
NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)),
FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil,
NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)),
NavigationItem.new("Schemes", "/schemes", supported_housing_schemes_current?(path)),
].compact
elsif current_user.data_coordinator? && current_user.organisation.holds_own_stock?
[
NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)),
FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil,
NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)),
NavigationItem.new("Schemes", "/schemes", subnav_supported_housing_schemes_path?(path)),
NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)),
NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)),
(NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)) if FeatureToggle.managing_owning_enabled?),
(NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?),
NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)),
NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)),
].compact
else
[
NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)),
FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil,
NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)),
NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)),
NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)),
(NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)) if FeatureToggle.managing_owning_enabled?),
(NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?),
NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)),
NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)),
].compact
end
end
@ -36,21 +36,21 @@ module NavigationItemsHelper
if current_user.organisation.holds_own_stock?
[
NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)),
FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", subnav_sales_logs_path?(path)) : nil,
NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", subnav_sales_logs_path?(path)),
NavigationItem.new("Schemes", "/organisations/#{current_organisation_id}/schemes", subnav_supported_housing_schemes_path?(path)),
NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)),
NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)),
(NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)) if FeatureToggle.managing_owning_enabled?),
(NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?),
NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)),
NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)),
].compact
else
[
NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)),
FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(path)) : nil,
NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(path)),
NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)),
NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)),
(NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)) if FeatureToggle.managing_owning_enabled?),
(NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?),
NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)),
NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)),
].compact
end
end

2
app/models/organisation.rb

@ -106,8 +106,6 @@ class Organisation < ApplicationRecord
{ name: "Registration number", value: housing_registration_no || "", editable: false },
{ name: "Rent_periods", value: rent_period_labels, editable: false, format: :bullet },
{ name: "Owns housing stock", value: holds_own_stock ? "Yes" : "No", editable: false },
({ name: "Other stock owners", value: other_stock_owners, editable: false } unless FeatureToggle.managing_owning_enabled?),
({ name: "Managing agents", value: managing_agents_label, editable: false } unless FeatureToggle.managing_owning_enabled?),
{ name: "Data protection agreement", value: data_protection_agreement_string, editable: false },
].compact
end

12
app/services/feature_toggle.rb

@ -12,14 +12,6 @@ class FeatureToggle
Rails.env.production? || Rails.env.test? || Rails.env.staging? || Rails.env.review?
end
def self.sales_log_enabled?
true
end
def self.managing_owning_enabled?
true
end
def self.scheme_toggle_enabled?
true
end
@ -50,10 +42,6 @@ class FeatureToggle
!Rails.env.production?
end
def self.collection_2023_2024_year_enabled?
true
end
def self.merge_organisations_enabled?
!Rails.env.production?
end

2
app/views/logs/index.html.erb

@ -51,7 +51,7 @@
<%= govuk_button_to "Create a new lettings log", lettings_logs_path, class: "govuk-!-margin-right-6" %>
<% end %>
<% if FeatureToggle.sales_log_enabled? && current_page?(controller: 'sales_logs', action: 'index') %>
<% if current_page?(controller: 'sales_logs', action: 'index') %>
<%= govuk_button_to "Create a new sales log", sales_logs_path, class: "govuk-!-margin-right-6" %>
<% end %>

11
spec/features/sales_log_spec.rb

@ -52,17 +52,6 @@ RSpec.describe "Sales Log Features" do
expect(page).to have_content("PC123")
end
end
context "when the sales log feature flag is disabled" do
before do
allow(FeatureToggle).to receive(:sales_log_enabled?).and_return(false)
end
it "hides the create sales log button in production" do
visit("/sales-logs")
expect(page).not_to have_content("Create a new sales log")
end
end
end
end

335
spec/helpers/navigation_items_helper_spec.rb

@ -7,342 +7,7 @@ RSpec.describe NavigationItemsHelper do
let(:organisation_path) { "/organisations/#{current_user.organisation.id}" }
describe "#primary items" do
context "when the sales log feature flag is disabled" do
before do
allow(FeatureToggle).to receive(:sales_log_enabled?).and_return(false)
allow(FeatureToggle).to receive(:managing_owning_enabled?).and_return(false)
end
context "when the user is a data coordinator" do
context "when the user is on the logs page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the users page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", users_path, true),
NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items(users_path, current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on their organisation details page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, true),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("#{organisation_path}/details", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the account page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/account", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the individual user's page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true),
NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the individual scheme's page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
]
end
it "returns navigation items with Schemes item set as current" do
expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items)
end
end
end
context "when the user is a support user" do
let(:current_user) { FactoryBot.create(:user, :support) }
context "when the user is on the logs page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the users page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", true),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/users", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the account page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/account", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the Schemes page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/schemes", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the individual user's page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", true),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
it "returns navigation items with the users item set as current" do
expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items)
end
end
context "when the user is on the individual scheme's page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true),
]
end
let(:expected_scheme_items) do
[
NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", true),
NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", false),
]
end
it "returns navigation items with Schemes item set as current" do
expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items)
expect(scheme_items("/schemes/1", 1, "Locations")).to eq(expected_scheme_items)
end
end
context "when the user is on the scheme locations page" do
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true),
]
end
let(:expected_scheme_items) do
[
NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", false),
NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", true),
]
end
it "returns navigation items with Schemes item set as current" do
expect(primary_items("/schemes/1/locations", current_user)).to eq(expected_navigation_items)
expect(scheme_items("/schemes/1/locations", 1, "Locations")).to eq(expected_scheme_items)
end
end
context "when the user is on the specific organisation's page" do
context "when the user is on organisation logs page" do
let(:required_sub_path) { "lettings-logs" }
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
let(:expected_secondary_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/organisations/#{current_user.organisation.id}/lettings-logs", true),
NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false),
]
end
it "returns navigation items with the logs item set as current" do
expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
end
end
context "when the user is on organisation users page" do
let(:required_sub_path) { "users" }
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
let(:expected_secondary_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true),
NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false),
]
end
it "returns navigation items with the logs item set as current" do
expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
end
end
context "when the user is on organisation schemes page" do
let(:required_sub_path) { "schemes" }
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
let(:expected_secondary_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", true),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false),
]
end
it "returns navigation items with the schemes item set as current" do
expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
end
end
context "when the user is on organisation details page" do
let(:required_sub_path) { "details" }
let(:expected_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
]
end
let(:expected_secondary_navigation_items) do
[
NavigationItemsHelper::NavigationItem.new("Lettings logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false),
NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false),
NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", true),
]
end
it "returns navigation items with the logs item set as current" do
expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
end
end
end
end
end
context "when the sales log feature flag is enabled" do
before do
allow(FeatureToggle).to receive(:sales_log_enabled?).and_return(true)
allow(FeatureToggle).to receive(:managing_owning_enabled?).and_return(true)
end
context "when the user is a data coordinator" do
context "when the user is on the lettings logs page" do
let(:expected_navigation_items) do

Loading…
Cancel
Save