From 18b3e2fc9df291249c9f2cd296505e1a16fe660e Mon Sep 17 00:00:00 2001 From: cintamani Date: Mon, 30 Dec 2019 09:00:09 +0000 Subject: [PATCH 1/5] Add permission checks files --- ..._registration_permission_checks_service.rb | 28 +++++++++++++++++++ .../flow_permission_checks_service.rb | 2 ++ .../flow_permission_checks_service_spec.rb | 10 +++++++ 3 files changed, 40 insertions(+) create mode 100644 app/services/waste_carriers_engine/ceased_or_revoked_registration_permission_checks_service.rb diff --git a/app/services/waste_carriers_engine/ceased_or_revoked_registration_permission_checks_service.rb b/app/services/waste_carriers_engine/ceased_or_revoked_registration_permission_checks_service.rb new file mode 100644 index 000000000..ff98c7585 --- /dev/null +++ b/app/services/waste_carriers_engine/ceased_or_revoked_registration_permission_checks_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module WasteCarriersEngine + class CeasedOrRevokedRegistrationPermissionChecksService < BaseRegistrationPermissionChecksService + + private + + def all_checks_pass? + transient_registration_is_valid? && user_has_permission? && registation_is_active? + end + + def user_has_permission? + return true if can?(:revoke, registration) && can?(:cease, registration) + + permission_check_result.needs_permissions! + + false + end + + def registation_is_active? + return true if registration.active? + + permission_check_result.invalid! + + false + end + end +end diff --git a/app/services/waste_carriers_engine/flow_permission_checks_service.rb b/app/services/waste_carriers_engine/flow_permission_checks_service.rb index 3e739fda1..6463e5ac7 100644 --- a/app/services/waste_carriers_engine/flow_permission_checks_service.rb +++ b/app/services/waste_carriers_engine/flow_permission_checks_service.rb @@ -26,6 +26,8 @@ def run_flow_setup_checks RenewingRegistrationPermissionChecksService.run(params) when OrderCopyCardsRegistration OrderCopyCardsRegistrationPermissionChecksService.run(params) + when CeasedOrRevokedRegistration + CeasedOrRevokedRegistrationPermissionChecksService.run(params) else raise MissingFlowPermissionChecksService, "No permission service found for #{transient_registration.class}" end diff --git a/spec/services/waste_carriers_engine/flow_permission_checks_service_spec.rb b/spec/services/waste_carriers_engine/flow_permission_checks_service_spec.rb index 89380ad8c..ab58f1bb6 100644 --- a/spec/services/waste_carriers_engine/flow_permission_checks_service_spec.rb +++ b/spec/services/waste_carriers_engine/flow_permission_checks_service_spec.rb @@ -29,6 +29,16 @@ module WasteCarriersEngine end end + context "when the transient object is an CeasedOrRevokedRegistration" do + let(:transient_registration) { CeasedOrRevokedRegistration.new } + + it "runs the correct permission check service and return a result" do + expect(CeasedOrRevokedRegistrationPermissionChecksService).to receive(:run).with(params).and_return(result) + + expect(described_class.run(params)).to eq(result) + end + end + context "when there is no permission check service for the given transient object" do let(:transient_registration) { double(:transient_registration) } From 583e179ebc5a4d268f0a5a573b4e949c2f33c756 Mon Sep 17 00:00:00 2001 From: cintamani Date: Mon, 30 Dec 2019 09:01:03 +0000 Subject: [PATCH 2/5] Fix routes and add abikity to dummy app --- config/routes.rb | 8 ++++---- spec/dummy/app/models/ability.rb | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index c1eed8ed4..885b75926 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,12 +38,12 @@ # End of order copy cards flow # Ceased or revoked flow - resources :cease_or_revoke_form, + resources :cease_or_revoke_forms, only: %i[new create], - path: "ceased-or-revoked", + path: "cease-or-revoke", path_names: { new: "" } - resources :ceased_or_revoked_confirm_form, + resources :ceased_or_revoked_confirm_forms, only: %i[new create], path: "ceased-or-revoked-confirm", path_names: { new: "" } do @@ -53,7 +53,7 @@ on: :collection end - resources :ceased_or_revoked_completed_form, + resources :ceased_or_revoked_completed_forms, only: %i[create], path: "ceased-or-revoked-complete" # End of ceased or revoked flow diff --git a/spec/dummy/app/models/ability.rb b/spec/dummy/app/models/ability.rb index 7809c5dee..8c019ad7c 100644 --- a/spec/dummy/app/models/ability.rb +++ b/spec/dummy/app/models/ability.rb @@ -7,5 +7,7 @@ def initialize(user) can :read, WasteCarriersEngine::Registration, account_email: user.email can :manage, WasteCarriersEngine::RenewingRegistration, account_email: user.email can :order_copy_cards, WasteCarriersEngine::Registration + can :cease, WasteCarriersEngine::Registration + can :revoke, WasteCarriersEngine::Registration end end From 456d3106af47e7f2c75919d46a742d439effeee5 Mon Sep 17 00:00:00 2001 From: cintamani Date: Mon, 30 Dec 2019 09:01:35 +0000 Subject: [PATCH 3/5] Delegate more params to registration --- .../waste_carriers_engine/ceased_or_revoked_registration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/waste_carriers_engine/ceased_or_revoked_registration.rb b/app/models/waste_carriers_engine/ceased_or_revoked_registration.rb index 256025b32..7fc5f52eb 100644 --- a/app/models/waste_carriers_engine/ceased_or_revoked_registration.rb +++ b/app/models/waste_carriers_engine/ceased_or_revoked_registration.rb @@ -6,7 +6,7 @@ class CeasedOrRevokedRegistration < TransientRegistration validates :reg_identifier, "waste_carriers_engine/reg_identifier": true - delegate :status, to: :registration + delegate :company_name, :registration_type, :tier, :contact_address, to: :registration def registration @_registration ||= Registration.find_by(reg_identifier: reg_identifier) From 457c95bae98174381c91f3bce352e61d83903ae6 Mon Sep 17 00:00:00 2001 From: cintamani Date: Mon, 30 Dec 2019 09:03:46 +0000 Subject: [PATCH 4/5] Add controller, views an all necessary code for the first part of the flow to work --- .../cease_or_revoke_forms_controller.rb | 27 +++ .../cease_or_revoke_form.rb | 40 ++++ .../ceased_or_revoked_confirm_form.rb | 7 + .../cease_or_revoke_forms/new.html.erb | 62 +++++++ .../forms/cease_or_revoke_forms/en.yml | 35 ++++ .../ceased_or_revoked_registrations/en.yml | 10 + .../ceased_or_revoked_registration.rb | 2 +- .../cease_or_revoke_forms_spec.rb | 174 ++++++++++++++++++ 8 files changed, 356 insertions(+), 1 deletion(-) create mode 100644 app/controllers/waste_carriers_engine/cease_or_revoke_forms_controller.rb create mode 100644 app/forms/waste_carriers_engine/cease_or_revoke_form.rb create mode 100644 app/forms/waste_carriers_engine/ceased_or_revoked_confirm_form.rb create mode 100644 app/views/waste_carriers_engine/cease_or_revoke_forms/new.html.erb create mode 100644 config/locales/forms/cease_or_revoke_forms/en.yml create mode 100644 config/locales/models/ceased_or_revoked_registrations/en.yml create mode 100644 spec/requests/waste_carriers_engine/cease_or_revoke_forms_spec.rb diff --git a/app/controllers/waste_carriers_engine/cease_or_revoke_forms_controller.rb b/app/controllers/waste_carriers_engine/cease_or_revoke_forms_controller.rb new file mode 100644 index 000000000..e75420e5c --- /dev/null +++ b/app/controllers/waste_carriers_engine/cease_or_revoke_forms_controller.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module WasteCarriersEngine + class CeaseOrRevokeFormsController < FormsController + def new + super(CeaseOrRevokeForm, "cease_or_revoke_form") + end + + def create + super(CeaseOrRevokeForm, "cease_or_revoke_form") + end + + private + + def transient_registration_attributes + params.fetch(:cease_or_revoke_form).permit(metaData: %i[status revoked_reason]) + end + + # rubocop:disable Naming/MemoizedInstanceVariableName + def find_or_initialize_transient_registration(token) + @transient_registration ||= CeasedOrRevokedRegistration.where(reg_identifier: token).first || + CeasedOrRevokedRegistration.where(token: token).first || + CeasedOrRevokedRegistration.new(reg_identifier: token) + end + # rubocop:enable Naming/MemoizedInstanceVariableName + end +end diff --git a/app/forms/waste_carriers_engine/cease_or_revoke_form.rb b/app/forms/waste_carriers_engine/cease_or_revoke_form.rb new file mode 100644 index 000000000..1b9ecd045 --- /dev/null +++ b/app/forms/waste_carriers_engine/cease_or_revoke_form.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module WasteCarriersEngine + class CeaseOrRevokeForm < BaseForm + def self.can_navigate_flexibly? + false + end + + delegate :metaData, to: :transient_registration + delegate :status, :revoked_reason, to: :metaData, allow_nil: true + delegate :contact_address, :company_name, :registration_type, :tier, to: :transient_registration + + validate :validate_status + validate :validate_revoked_reason + + private + + def validate_status + return true if %w[INACTIVE REVOKED].include?(status) + + errors.add(:status, :presence) + + false + end + + def validate_revoked_reason + if revoked_reason.blank? + errors.add(:revoked_reason, :presence) + + false + elsif revoked_reason.size > 500 + errors.add(:revoked_reason, :length) + + false + end + + true + end + end +end diff --git a/app/forms/waste_carriers_engine/ceased_or_revoked_confirm_form.rb b/app/forms/waste_carriers_engine/ceased_or_revoked_confirm_form.rb new file mode 100644 index 000000000..2f123a5af --- /dev/null +++ b/app/forms/waste_carriers_engine/ceased_or_revoked_confirm_form.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module WasteCarriersEngine + class CeasedOrRevokedConfirmForm < BaseForm + # TODO + end +end diff --git a/app/views/waste_carriers_engine/cease_or_revoke_forms/new.html.erb b/app/views/waste_carriers_engine/cease_or_revoke_forms/new.html.erb new file mode 100644 index 000000000..2a696bc5f --- /dev/null +++ b/app/views/waste_carriers_engine/cease_or_revoke_forms/new.html.erb @@ -0,0 +1,62 @@ +<%= render("waste_carriers_engine/shared/back", back_path: Rails.application.routes.url_helpers.registration_path(@transient_registration.reg_identifier)) %> + +
+ <%= form_for(@cease_or_revoke_form) do |f| %> + <%= f.fields_for :metaData do |f| %> + <%= render("waste_carriers_engine/shared/errors", object: @cease_or_revoke_form) %> + +

<%= t(".heading", reg_identifier: @transient_registration.reg_identifier) %>

+ +
+

<%= @cease_or_revoke_form.company_name %>

+ +

+ <%= t(".tier.#{@cease_or_revoke_form.tier.downcase}") %> - + <%= t(".registration_type.#{@cease_or_revoke_form.registration_type}") %> +

+

+ <%= displayable_address(@cease_or_revoke_form.contact_address).join(", ") %> +

+
+ +
"> +
+ +

+ <%= t(".cease_or_revoke.legend") %> +

+ + + <% if @cease_or_revoke_form.errors[:status].any? %> + <%= @cease_or_revoke_form.errors[:status].join(", ") %> + <% end %> + +
+ <%= f.radio_button :status, "REVOKED", checked: @transient_registration.metaData&.status == "REVOKED" %> + <%= f.label :status, t(".cease_or_revoke.options.revoke"), value: "REVOKED" %> +
+ +
+ <%= f.radio_button :status, "INACTIVE", checked: @transient_registration.metaData&.status == "INACTIVE" %> + <%= f.label :status, t(".cease_or_revoke.options.cease"), value: "INACTIVE" %> +
+
+
+ +
"> +
+ <% if @cease_or_revoke_form.errors[:revoked_reason].any? %> + <%= @cease_or_revoke_form.errors[:revoked_reason].join(", ") %> + <% end %> + + <%= f.label :revoked_reason, t(".cease_or_revoke.reason"), class: "form-label" %> + <%= f.text_area :revoked_reason, class: "form-control form-control-3-4", rows: 5, value: @transient_registration.metaData&.revoked_reason %> +
+
+ +
+ <%= f.submit t(".next_button"), class: "button" %> +
+ <% end %> + <% end %> +
diff --git a/config/locales/forms/cease_or_revoke_forms/en.yml b/config/locales/forms/cease_or_revoke_forms/en.yml new file mode 100644 index 000000000..40d27e45c --- /dev/null +++ b/config/locales/forms/cease_or_revoke_forms/en.yml @@ -0,0 +1,35 @@ +en: + waste_carriers_engine: + cease_or_revoke_forms: + new: + heading: "Revoke or cease registration %{reg_identifier}" + tier: + upper: "Upper tier" + lower: "Lower tier" + registration_type: + carrier_dealer: "Carrier and dealer" + broker_dealer: "Broker and dealer" + carrier_broker_dealer: "Carrier, broker and dealer" + reason: + label: "Reason" + help_text: "Maximum 500 characters" + cease_or_revoke: + legend: "What do you want to do?" + options: + revoke: "Revoke" + cease: "De-register" + reason: "Reason" + total_cost_info: "The total cost will be shown on the next page" + contact_address: + heading: "Contact address:" + next_button: "Confirm" + activemodel: + errors: + models: + waste_carriers_engine/cease_or_revoke_form: + attributes: + status: + presence: "Select revoke or cease" + revoked_reason: + presence: "Enter a reason" + length: "Reason must be 500 characters or fewer" diff --git a/config/locales/models/ceased_or_revoked_registrations/en.yml b/config/locales/models/ceased_or_revoked_registrations/en.yml new file mode 100644 index 000000000..ec8e7342f --- /dev/null +++ b/config/locales/models/ceased_or_revoked_registrations/en.yml @@ -0,0 +1,10 @@ +en: + activemodel: + errors: + models: + waste_carriers_engine/ceased_or_revoked_registration: + attributes: + reg_identifier: + invalid_format: "The registration ID is not in a valid format" + no_registration: "There is no registration matching this ID" + renewal_in_progress: "This renewal is already in progress" diff --git a/spec/factories/ceased_or_revoked_registration.rb b/spec/factories/ceased_or_revoked_registration.rb index 313982f84..3cf20c36e 100644 --- a/spec/factories/ceased_or_revoked_registration.rb +++ b/spec/factories/ceased_or_revoked_registration.rb @@ -2,6 +2,6 @@ FactoryBot.define do factory :ceased_or_revoked_registration, class: WasteCarriersEngine::CeasedOrRevokedRegistration do - initialize_with { new(reg_identifier: build(:registration, :has_required_data, :is_active).reg_identifier) } + initialize_with { new(reg_identifier: create(:registration, :has_required_data, :is_active).reg_identifier) } end end diff --git a/spec/requests/waste_carriers_engine/cease_or_revoke_forms_spec.rb b/spec/requests/waste_carriers_engine/cease_or_revoke_forms_spec.rb new file mode 100644 index 000000000..1fef28a56 --- /dev/null +++ b/spec/requests/waste_carriers_engine/cease_or_revoke_forms_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require "rails_helper" + +module WasteCarriersEngine + RSpec.describe "CeaseOrRevokeForms", type: :request do + describe "GET new_cease_or_revoke_form_path" do + context "when a user is signed in" do + let(:user) { create(:user) } + + before(:each) do + sign_in(user) + end + + context "when no matching registration exists" do + it "redirects to the invalid token error page" do + get new_cease_or_revoke_form_path("CBDU999999999") + + expect(response).to redirect_to(page_path("invalid")) + end + end + + context "when the token doesn't match the format" do + it "redirects to the invalid token error page" do + get new_cease_or_revoke_form_path("foo") + + expect(response).to redirect_to(page_path("invalid")) + end + end + + context "when a matching registration exists" do + context "when the given registration is not active" do + let(:registration) { create(:registration, :has_required_data, :is_pending) } + + it "redirects to the page" do + get new_cease_or_revoke_form_path(registration.reg_identifier) + + expect(response).to redirect_to(page_path("invalid")) + end + end + + context "when the given registration is active" do + let(:registration) { create(:registration, :has_required_data, :is_active) } + + it "responds to the GET request with a 200 status code and renders the appropriate template" do + get new_cease_or_revoke_form_path(registration.reg_identifier) + + expect(response).to render_template("waste_carriers_engine/cease_or_revoke_forms/new") + expect(response.code).to eq("200") + end + + context "when an order is in progress" do + let!(:transient_registration) { create(:ceased_or_revoked_registration, workflow_state: "ceased_or_revoked_confirm_form") } + + context "when the token is a reg_identifier" do + it "redirects to the correct workflow state form" do + get new_cease_or_revoke_form_path(transient_registration.registration.reg_identifier) + + expect(response).to redirect_to(new_ceased_or_revoked_confirm_form_path(transient_registration.token)) + end + end + + it "redirects to the correct workflow state form" do + get new_cease_or_revoke_form_path(transient_registration.token) + + expect(response).to redirect_to(new_ceased_or_revoked_confirm_form_path(transient_registration.token)) + end + end + end + end + end + + context "when a user is not signed in" do + before(:each) do + user = create(:user) + sign_out(user) + end + + it "returns a 302 response and redirects to the sign in page" do + get new_cease_or_revoke_form_path("foo") + + expect(response).to have_http_status(302) + expect(response).to redirect_to(new_user_session_path) + end + end + end + + describe "POST cease_or_revoke_forms_path" do + context "when a user is signed in" do + let(:user) { create(:user) } + + before(:each) do + sign_in(user) + end + + context "when no matching registration exists" do + it "redirects to the invalid token error page and does not create a new transient registration" do + original_tr_count = CeasedOrRevokedRegistration.count + + post cease_or_revoke_forms_path("CBDU99999") + + expect(response).to redirect_to(page_path("invalid")) + expect(CeasedOrRevokedRegistration.count).to eq(original_tr_count) + end + end + + context "when the token doesn't match the format" do + it "redirects to the invalid token error page and does not create a new transient registration" do + original_tr_count = CeasedOrRevokedRegistration.count + + post cease_or_revoke_forms_path("foo") + + expect(CeasedOrRevokedRegistration.count).to eq(original_tr_count) + expect(response).to redirect_to(page_path("invalid")) + end + end + + context "when a matching registration exists" do + let(:registration) { create(:registration, :has_required_data, :is_active) } + + context "when valid params are submitted" do + let(:valid_params) { { token: registration.reg_identifier, metaData: { status: "REVOKED", revoked_reason: "Some reason" } } } + + it "creates a transient registration with correct data, returns a 302 response and redirects to the " do + expected_tr_count = CeasedOrRevokedRegistration.count + 1 + + post cease_or_revoke_forms_path(registration.reg_identifier), cease_or_revoke_form: valid_params + + transient_registration = CeasedOrRevokedRegistration.find_by(reg_identifier: registration.reg_identifier) + + expect(expected_tr_count).to eq(CeasedOrRevokedRegistration.count) + expect(transient_registration.metaData.status).to eq("REVOKED") + expect(transient_registration.metaData.revoked_reason).to eq("Some reason") + + expect(response).to have_http_status(302) + expect(response).to redirect_to(new_ceased_or_revoked_confirm_form_path(transient_registration.token)) + end + end + + context "when invalid params are submitted" do + let(:invalid_params) { { token: registration.reg_identifier, metaData: {} } } + + it "returns a 200 response and render the new form" do + post cease_or_revoke_forms_path(registration.reg_identifier), cease_or_revoke_form: invalid_params + + expect(response).to have_http_status(200) + expect(response).to render_template("waste_carriers_engine/cease_or_revoke_forms/new") + end + end + end + end + + context "when a user is not signed in" do + let(:registration) { create(:registration, :has_required_data) } + let(:valid_params) { { token: registration.reg_identifier } } + + before(:each) do + user = create(:user) + sign_out(user) + end + + it "returns a 302 response, redirects to the login page and does not create a new transient registration" do + original_tr_count = CeasedOrRevokedRegistration.count + + post cease_or_revoke_forms_path(registration.reg_identifier), renewal_start_form: valid_params + + expect(response).to redirect_to(new_user_session_path) + expect(response).to have_http_status(302) + expect(CeasedOrRevokedRegistration.count).to eq(original_tr_count) + end + end + end + end +end From 6d6a5d495023e69ad124640eaadbc99d8bc79eeb Mon Sep 17 00:00:00 2001 From: cintamani Date: Mon, 30 Dec 2019 09:46:37 +0000 Subject: [PATCH 5/5] Remove unused translation and change De-Register to Cease --- config/locales/forms/cease_or_revoke_forms/en.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/locales/forms/cease_or_revoke_forms/en.yml b/config/locales/forms/cease_or_revoke_forms/en.yml index 40d27e45c..bfb8293f0 100644 --- a/config/locales/forms/cease_or_revoke_forms/en.yml +++ b/config/locales/forms/cease_or_revoke_forms/en.yml @@ -17,9 +17,8 @@ en: legend: "What do you want to do?" options: revoke: "Revoke" - cease: "De-register" + cease: "Cease" reason: "Reason" - total_cost_info: "The total cost will be shown on the next page" contact_address: heading: "Contact address:" next_button: "Confirm"