From 1a1d51b77247e01d3475b01420430a1143347973 Mon Sep 17 00:00:00 2001 From: Adam Lee <32270711+Gubbsy@users.noreply.github.com> Date: Fri, 4 Aug 2023 12:11:33 +0100 Subject: [PATCH 01/14] Fix/app memory allocation (#1133) * Bump research memory allocation to 1GB * Bump mem to 2GB on UATT & PROD, restore research to 256MB --- manifest-production.yml | 2 +- manifest-uat.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifest-production.yml b/manifest-production.yml index 4cd9d10e0..4b365b3a0 100644 --- a/manifest-production.yml +++ b/manifest-production.yml @@ -13,7 +13,7 @@ applications: processes: - type: web command: bundle exec rake cf:on_first_instance db:migrate && rails s -p $PORT - memory: 256M + memory: 2GB instances: 2 health-check-type: http health-check-http-endpoint: /health diff --git a/manifest-uat.yml b/manifest-uat.yml index ec8b89ef5..7276a9785 100644 --- a/manifest-uat.yml +++ b/manifest-uat.yml @@ -13,7 +13,7 @@ applications: processes: - type: web command: bundle exec rake cf:on_first_instance db:migrate && rails s -p $PORT - memory: 256M + memory: 2GB instances: 2 health-check-type: http health-check-http-endpoint: /health From b5da4f3b16859e648b01095a681face630586c4d Mon Sep 17 00:00:00 2001 From: Jack <91466216+JJD1990@users.noreply.github.com> Date: Mon, 7 Aug 2023 10:13:33 +0100 Subject: [PATCH 02/14] reordered reconnection report data (#1132) --- app/helpers/import_helper.rb | 2 +- .../admin_portal/reconnection_report/show.html.erb | 14 ++++++-------- .../salesforce/import/import_salesforce_api.rb | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb index 780d3efa7..ccb20792b 100644 --- a/app/helpers/import_helper.rb +++ b/app/helpers/import_helper.rb @@ -373,7 +373,7 @@ def populate_temporary_table_and_run_report(projects_for_reconnection) project.Project_Reference_Number__c) pop_temp_table_sql << "INSERT INTO reconnection_projects VALUES (" \ - "#{name}, #{title}, #{ref}, #{area});" + "#{name}, #{title}, #{area}, #{ref} );" end diff --git a/app/views/admin_portal/reconnection_report/show.html.erb b/app/views/admin_portal/reconnection_report/show.html.erb index 3a622f587..4d5e0c73f 100644 --- a/app/views/admin_portal/reconnection_report/show.html.erb +++ b/app/views/admin_portal/reconnection_report/show.html.erb @@ -11,9 +11,9 @@ Project Owner - Project Reference Number Project Title Project Area/Country + Project Reference Number Reconnection Date @@ -22,17 +22,15 @@ <%# This could be a nested loop, but to be explicit: %> <%# row[0] - Project Owner%> - <%# row[2] - Project Reference Number%> - <%# row[3] - Project Title%> - <%# row[1] - Project area/country%> + <%# row[1] - Project Title%> + <%# row[2] - Project area/country%> + <%# row[3] - Project Reference Number%> <%# row[4] - Project Reconnection Date%> <%=row[0]%> - <%=row[2]%> + <%=row[1]%> + <%=row[2].present? ? row[2]: 'Not specified'%> <%=row[3]%> - <%=row[1].present? ? row[1]: 'Not specified'%> <%=row[4].present? ? row[4].strftime('%d-%m-%Y').to_s : 'Not reconnected' %> - - <% end %> diff --git a/lib/apis/salesforce/import/import_salesforce_api.rb b/lib/apis/salesforce/import/import_salesforce_api.rb index 4d1e2e23f..6bbc464d0 100644 --- a/lib/apis/salesforce/import/import_salesforce_api.rb +++ b/lib/apis/salesforce/import/import_salesforce_api.rb @@ -180,7 +180,7 @@ def retrieve_existing_account_info(name, postcode, org_id) def get_projects_selected_for_reconnection query = "SELECT Owner.Name, Project_Title__c, " \ - "Project_Reference_Number__c, Region__c " \ + "Region__c, Project_Reference_Number__c " \ "FROM Case where Export_to_IMS_Portal__c = true " restforce_response = run_salesforce_query( From 2fc89d06e4e6d8bc4dd00a7f968956dec078679c Mon Sep 17 00:00:00 2001 From: Paul Trelease Date: Mon, 7 Aug 2023 14:03:50 +0100 Subject: [PATCH 03/14] Feature/manifest to increase disk for uat prod (#1134) * increased disk storage values for UAT and prod from 1 to 3GB --- manifest-production.yml | 1 + manifest-uat.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/manifest-production.yml b/manifest-production.yml index 4b365b3a0..8437d5db4 100644 --- a/manifest-production.yml +++ b/manifest-production.yml @@ -14,6 +14,7 @@ applications: - type: web command: bundle exec rake cf:on_first_instance db:migrate && rails s -p $PORT memory: 2GB + disk_quota: 3GB instances: 2 health-check-type: http health-check-http-endpoint: /health diff --git a/manifest-uat.yml b/manifest-uat.yml index 7276a9785..a00ebd841 100644 --- a/manifest-uat.yml +++ b/manifest-uat.yml @@ -14,6 +14,7 @@ applications: - type: web command: bundle exec rake cf:on_first_instance db:migrate && rails s -p $PORT memory: 2GB + disk_quota: 3GB instances: 2 health-check-type: http health-check-http-endpoint: /health From 63081996f8d078bc61347235c96a0685751afcca Mon Sep 17 00:00:00 2001 From: Jack <91466216+JJD1990@users.noreply.github.com> Date: Tue, 8 Aug 2023 08:50:37 +0100 Subject: [PATCH 04/14] Fix/org name length 255 (#1135) * validation for organisation name set to 255, spec written * error message added to en & cy yml files, validade_length/too_long method added to organisation model. * better rspec test written for organisation name validation * deleted redundant validate_length method in organisation.rb --- app/models/organisation.rb | 1 + config/locales/cy.yml | 1 + config/locales/en.yml | 1 + spec/models/organisation_spec.rb | 17 +++++++++++++++++ 4 files changed, 20 insertions(+) create mode 100644 spec/models/organisation_spec.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb index a540db638..eda2024a1 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -46,6 +46,7 @@ class Organisation < ApplicationRecord validates :custom_org_type, presence: true, if: :validate_custom_org_type? validate :validate_mission_array, if: :validate_mission? validates :name, presence: true, if: :validate_name? + validates :name, length: { maximum: 255 } validates :name, presence: true, if: :validate_address? validates :line1, presence: true, if: :validate_address? validates :townCity, presence: true, if: :validate_address? diff --git a/config/locales/cy.yml b/config/locales/cy.yml index ca78fb019..83db170c0 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -204,6 +204,7 @@ cy: not_a_number: "Mae'n rhaid i rif cwmni fod yn rhif, fel 12345678" name: blank: "Rhowch enw eich sefydliad" + too_long: "Rhaid i Enw Sefydliad fod yn 225 nod neu lai" line1: blank: "Rhowch linell gyntaf cyfeiriad eich sefydliad" townCity: diff --git a/config/locales/en.yml b/config/locales/en.yml index 7ac41400b..eec7484d5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -204,6 +204,7 @@ en-GB: not_a_number: "Company number must be a number, like 12345678" name: blank: "Enter the name of your organisation" + too_long: "Organisation name must be 255 characters or fewer" line1: blank: "Enter the first line of your organisation's address" townCity: diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb new file mode 100644 index 000000000..5a741cda0 --- /dev/null +++ b/spec/models/organisation_spec.rb @@ -0,0 +1,17 @@ +require 'rails_helper' + +RSpec.describe Organisation, type: :model do + let(:valid_organisation_1) { Organisation.new(name: 'A' * 255) } + let(:valid_organisation_2) { Organisation.new(name: 'A' * 100) } + let(:invalid_organisation) { Organisation.new(name: 'A' * 256) } + + it 'validates length of name to be less than or equal to 255 characters' do + expect(invalid_organisation.valid?).to be(false) + expect(invalid_organisation.errors[:name]).to include("Organisation name must be 255 characters or fewer") + end + + it 'is valid when organisation name is equal to or below 255 characters' do + expect(valid_organisation_1.valid?).to be(true) + expect(valid_organisation_2.valid?).to be(true) + end +end \ No newline at end of file From 0728d2c4c709a161bd141020de3f247852f67fe4 Mon Sep 17 00:00:00 2001 From: Jack <91466216+JJD1990@users.noreply.github.com> Date: Wed, 9 Aug 2023 09:10:14 +0100 Subject: [PATCH 05/14] Fix/unticked mission (#1136) * conditionals added to update action in mission_controller which sets mission to empty string if no mission chosen for bugfix * created ensure_mission_params method in organisation.rb, added not to explain method * mission_controller_spec added both indirectly and directly testing the ensure_mission_params method --- .../organisation/mission_controller.rb | 18 ++++++ .../organisation/mission_controller_spec.rb | 62 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/app/controllers/organisation/mission_controller.rb b/app/controllers/organisation/mission_controller.rb index d6e4b30e0..8dd83437b 100644 --- a/app/controllers/organisation/mission_controller.rb +++ b/app/controllers/organisation/mission_controller.rb @@ -10,6 +10,8 @@ def update logger.info "Updating mission for organisation ID: #{@organisation.id}" + ensure_mission_params + @organisation.validate_mission = true @organisation.update(organisation_params) @@ -40,4 +42,20 @@ def organisation_params end + # This method ensures that if no mission is chosen by the user + # the mission array is set back to empty. + def ensure_mission_params + + if params[:organisation] + + params[:organisation][:mission] ||= [] + + else + + params[:organisation] = { mission: [] } + + end + + end + end diff --git a/spec/controllers/organisation/mission_controller_spec.rb b/spec/controllers/organisation/mission_controller_spec.rb index d9cd985a1..7c6775e6d 100644 --- a/spec/controllers/organisation/mission_controller_spec.rb +++ b/spec/controllers/organisation/mission_controller_spec.rb @@ -110,6 +110,68 @@ end + it "should successfully update if no mission params are passed" do + + put :update, params: { + organisation_id: subject.current_user.organisations.first.id, + organisation: { + mission: [] + } + } + + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(:organisation_summary) + + expect(assigns(:organisation).errors.empty?).to eq(true) + expect(assigns(:organisation) + .mission).to eq([]) + + end + + end + + # These tests specifically test the ensure_mission_params method + describe '#ensure_mission_params' do + + before do + controller.class.send(:public, :ensure_mission_params) # This makes the method available for testing as it is a private method + end + + context 'when :organisation is present' do + context 'when :mission is already set' do + it 'does not change the mission' do + params = { + organisation: { + mission: ["female_led"], + } + } + allow(controller).to receive(:params).and_return(params) + + controller.ensure_mission_params + + expect(params[:organisation][:mission]).to eq(['female_led']) + + end + + end + + end + + context 'when :mission is not set' do + it 'sets mission to an empty array' do + params = { + organisation: {} + } + allow(controller).to receive(:params).and_return(params) + + controller.ensure_mission_params + + expect(params[:organisation][:mission]).to eq([]) + + end + + end + end end From 9561121f7dc281b5e37d141809cc192ff26155c9 Mon Sep 17 00:00:00 2001 From: Eithel Anderson <48526057+etelish@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:44:11 +0100 Subject: [PATCH 06/14] added project title conditional to get main contact apps method (#1137) * added project title conditional to get main contact apps method * added in a method to migrate and move application medium over 100k --- app/helpers/admin_portal_helper.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/app/helpers/admin_portal_helper.rb b/app/helpers/admin_portal_helper.rb index fb49f2a36..11800eda7 100644 --- a/app/helpers/admin_portal_helper.rb +++ b/app/helpers/admin_portal_helper.rb @@ -43,6 +43,7 @@ module AdminPortalHelper PEF = 4 EOI = 5 UNKNOWN = 6 + MIGRATED_MEDIUM_OVER_100k = 7 # Creates an array of hashes for the applications and # pre-applications belonging to a main applicant. @@ -74,6 +75,16 @@ def get_main_contact_apps(org_id, user_id) type = MEDIUM end + if fa.migrated_medium_over_100k? + type = MIGRATED_MEDIUM_OVER_100k + + salesforce_api_client= SalesforceApiClient.new + + title = salesforce_api_client + .get_project_title(fa.salesforce_case_id) + .Project_Title__c + end + if fa.project.present? title = fa.project.project_title type = SMALL @@ -163,6 +174,8 @@ def move_app_to_new_user(chosen_app_hash, new_contact_id, new_org_id) move_3_to_10k(chosen_app_hash, new_contact_id, new_org_id) when MEDIUM move_10_to_250k(chosen_app_hash, new_contact_id, new_org_id) + when MIGRATED_MEDIUM_OVER_100k + move_migrated_medium_over_100k(chosen_app_hash, new_org_id) when LARGE move_large(chosen_app_hash, new_org_id) when PEF @@ -354,6 +367,16 @@ def move_large(chosen_app_hash, new_org_id) end + # uses exactly the same as move_large for migrating medium over 100k + # @param [Hash] chosen_app_hash App that we are moving: example + # {:id=>"", :ref_no=>"", :type=>1, :title=>"", salesforce_id => ""} + # @param [String] new_org_id FFE GUID for new organisation + def move_migrated_medium_over_100k(chosen_app_hash, new_org_id) + + move_large(chosen_app_hash, new_org_id) + + end + # Moves a pre_application to a new user # Amends pre_applications rows # Writes audit row of changes From 485fe9730fc6af5351d78965ea25de38f4eec7e8 Mon Sep 17 00:00:00 2001 From: Paul Trelease Date: Thu, 10 Aug 2023 15:00:09 +0100 Subject: [PATCH 07/14] Enabled session timeout (#1138) --- app/models/user.rb | 3 ++- config/initializers/devise.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 698034762..7aca2fb87 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -8,7 +8,8 @@ class User < ApplicationRecord :recoverable, :rememberable, :validatable, - :confirmable + :confirmable, + :timeoutable enum role: [:user, :admin] after_initialize :set_default_role, :if => :new_record? diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 54103ffa1..7bcbb6883 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -175,8 +175,8 @@ # ==> Configuration for :timeoutable # The time you want to timeout the user session without activity. After this - # time the user will be asked for credentials again. Default is 30 minutes. - # config.timeout_in = 30.minutes + # time the user will be asked for credentials again. + config.timeout_in = 60.minutes # ==> Configuration for :lockable # Defines which strategy will be used to lock an account. From add0a9c80fff568fcdab2d74b1389a06b80932f9 Mon Sep 17 00:00:00 2001 From: Paul Trelease Date: Fri, 11 Aug 2023 11:42:41 +0100 Subject: [PATCH 08/14] Feature/amend longer session (#1140) * increased timeout criteria to 20 hours --- config/initializers/devise.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 7bcbb6883..87f400fa0 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -176,7 +176,9 @@ # ==> Configuration for :timeoutable # The time you want to timeout the user session without activity. After this # time the user will be asked for credentials again. - config.timeout_in = 60.minutes + # Gone for WCAG 20 hour exception to meet level A criteria. + # https://www.w3.org/WAI/WCAG21/Understanding/timing-adjustable.html + config.timeout_in = 20.hours # ==> Configuration for :lockable # Defines which strategy will be used to lock an account. From 27f53a02ca6bb277d149bef868073aef7051a50c Mon Sep 17 00:00:00 2001 From: Eithel Anderson <48526057+etelish@users.noreply.github.com> Date: Fri, 11 Aug 2023 12:28:35 +0100 Subject: [PATCH 09/14] replaced email input field with label and override update_resource method to prevent users email being updated (#1139) --- app/controllers/user/registrations_controller.rb | 9 +++++++++ app/views/user/registrations/edit.html.erb | 7 +++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/controllers/user/registrations_controller.rb b/app/controllers/user/registrations_controller.rb index 96ed1650f..a6cd284e9 100644 --- a/app/controllers/user/registrations_controller.rb +++ b/app/controllers/user/registrations_controller.rb @@ -33,5 +33,14 @@ def create_person(resource) NotifyMailer.confirmation_instructions_copy(resource).deliver_later end + + # Override the Devise::RegistrationsController update_resource method + # Ensures the email is not provided as a param to prevent it being updated + def update_resource(resource, params) + + params.delete(:email) + + super + end end diff --git a/app/views/user/registrations/edit.html.erb b/app/views/user/registrations/edit.html.erb index cfa83dd9a..1b67bdf41 100644 --- a/app/views/user/registrations/edit.html.erb +++ b/app/views/user/registrations/edit.html.erb @@ -45,10 +45,9 @@ <%= - f.text_field :email, - autofocus: true, - autocomplete: "email", - class: "govuk-input govuk-input--width-20" + f.label :email, + @user.email, + class: "govuk-label govuk-!-margin-top-2 govuk-!-font-weight-bold" %> From 5734e941dcbe59212b1a7809a35e8f36edfb6bf3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 21 Aug 2023 09:52:07 +0100 Subject: [PATCH 10/14] Bump semver from 5.7.1 to 5.7.2 (#1120) Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2. - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md) - [Commits](https://github.com/npm/node-semver/compare/v5.7.1...v5.7.2) --- updated-dependencies: - dependency-name: semver dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/yarn.lock b/yarn.lock index ba5efb06b..86fa6394e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7612,19 +7612,19 @@ selfsigned@^1.10.8: node-forge "^0.10.0" "semver@2 || 3 || 4 || 5", semver@^5.5.0, semver@^5.6.0: - version "5.7.1" - resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.1.tgz#a954f931aeba508d307bbf069eff0c01c96116f7" - integrity sha512-sauaDf/PZdVgrLTNYHRtpXa1iRiKcaebiKQ1BJdpQlWH2lCvexQdX55snPFyK7QzpudqbCI0qXFfOasHdyNDGQ== + version "5.7.2" + resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.2.tgz#48d55db737c3287cd4835e17fa13feace1c41ef8" + integrity sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g== semver@^6.0.0, semver@^6.1.1, semver@^6.1.2, semver@^6.3.0: - version "6.3.0" - resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d" - integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== + version "6.3.1" + resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.1.tgz#556d2ef8689146e46dcea4bfdd095f3434dffcb4" + integrity sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA== semver@^7.3.2, semver@^7.3.5: - version "7.5.3" - resolved "https://registry.yarnpkg.com/semver/-/semver-7.5.3.tgz#161ce8c2c6b4b3bdca6caadc9fa3317a4c4fe88e" - integrity sha512-QBlUtyVk/5EeHbi7X0fw6liDZc7BBmEaSYn01fMU1OUYbf6GPsbTtd8WmnqbI20SeycoHSeiybkE/q1Q+qlThQ== + version "7.5.4" + resolved "https://registry.yarnpkg.com/semver/-/semver-7.5.4.tgz#483986ec4ed38e1c6c48c34894a9182dbff68a6e" + integrity sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA== dependencies: lru-cache "^6.0.0" From a244c9b47db07697c32533676bc8b279fcd38e9a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Aug 2023 11:07:28 +0100 Subject: [PATCH 11/14] Bump puma from 4.3.12 to 5.6.7 (#1141) Bumps [puma](https://github.com/puma/puma) from 4.3.12 to 5.6.7. - [Release notes](https://github.com/puma/puma/releases) - [Changelog](https://github.com/puma/puma/blob/master/History.md) - [Commits](https://github.com/puma/puma/compare/v4.3.12...v5.6.7) --- updated-dependencies: - dependency-name: puma dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 190c3f2e3..e3336b943 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,7 @@ gem 'lograge', '~> 0.11.2' gem 'mail-notify', '~> 1.1.0 ' gem 'nilify_blanks', '~> 1.3' gem 'pg', '~> 1.1' -gem 'puma', '~> 4.3' +gem 'puma', '~> 5.6' gem "rails", "~> 7.0.0" gem 'rails-i18n', '~> 7.0.5' gem 'redis', '~> 4.1.3' diff --git a/Gemfile.lock b/Gemfile.lock index 0c71c1029..5f82a41c5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -257,7 +257,7 @@ GEM method_source (~> 1.0) psych (3.3.4) public_suffix (5.0.1) - puma (4.3.12) + puma (5.6.7) nio4r (~> 2.0) racc (1.7.1) rack (2.2.7) @@ -473,7 +473,7 @@ DEPENDENCIES pg (~> 1.1) pry (~> 0.14.1) psych (< 4) - puma (~> 4.3) + puma (~> 5.6) rails (~> 7.0.0) rails-controller-testing (~> 1.0.4) rails-i18n (~> 7.0.5) From 42fda8232bb04a737a3ca39f60cbde8bae4055fa Mon Sep 17 00:00:00 2001 From: Jack <91466216+JJD1990@users.noreply.github.com> Date: Thu, 24 Aug 2023 09:39:37 +0100 Subject: [PATCH 12/14] Fix/cc no not sure (#1142) * method created in the project model to format the cash contribution secured value * dash taken away from not sure in format secured for salesforce method, dasherize is needed for other values to work * added cc build and further expect to spec for x_not_sure fix * context added to spec to test project with no cc, better formatted method description * formatting comments and line at eof --- app/models/project.rb | 23 ++- spec/models/project_spec.rb | 318 ++++++++++++++++++++---------------- 2 files changed, 195 insertions(+), 146 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index ce0f20a1d..28c2b24a3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -417,7 +417,7 @@ def to_salesforce_json json.cashContributions self.cash_contributions do |cash_contribution| json.description cash_contribution.description json.amount cash_contribution.amount - json.secured cash_contribution.secured&.dasherize + json.secured format_secured_for_salesforce(cash_contribution) json.id cash_contribution.id end json.set!('organisationSalesforceAccountId', @@ -470,4 +470,23 @@ def get_organisation_type_for_salesforce_json end -end + # Formats the secured value of a cash contribution for Salesforce. + # + # Given a cash contribution with a particular secured value, this method + # will either return the "not sure" string or a dasherized version of the + # value, depending on the original value. + # + # @param [object] cash contribution object + # @param [string] :secured for the answer/value of the 'is your cash + # contribution secured?' question + # + # @return [string] - A formatted string value. Either 'not sure' or + # a dasherized value + def format_secured_for_salesforce(cash_contribution) + if cash_contribution.secured == 'x_not_sure' + 'not sure' + else + cash_contribution.secured&.dasherize + end + end +end \ No newline at end of file diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2107d6c14..cb338d65c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1,40 +1,38 @@ require "rails_helper" RSpec.describe Project, type: :model do - describe "Project model" do - it "should serialise Salesforce JSON successfully" do - - @project = build( - :project, - id: "2c660111-ab15-4221-98e0-cf0e02748a9b", - project_title: "Test Project", start_date: "1/1/2025", - end_date: "1/10/2025", line1: "10 Downing Street", - line2: "Westminster", townCity: "London", county: "LONDON", - postcode: "SW1A 2AA", description: "A description of my project...", - difference: "The difference my project will make to...", - matter: "My project matters because...", - best_placed_description: "My organisation is best placed to...", - heritage_description: "The heritage of my project...", - involvement_description: "My project will involve a wider range of " \ - "people...", - outcome_2: true, outcome_3: false, outcome_4: true, outcome_5: false, - outcome_6: true, outcome_7: false, outcome_8: true, outcome_9: false, - outcome_2_description: "Description of outcome 2", - outcome_3_description: "", - outcome_4_description: "Description of outcome 4", - outcome_5_description: "", - outcome_6_description: "Description of outcome 6", - outcome_7_description: "", - outcome_8_description: "Description of outcome 8", - outcome_9_description: "", permission_type: 2, - permission_description: "permission description", - partnership_details: "partnership details", - declaration_reasons_description: "something" - ) + before do + @project = build( + :project, + id: "2c660111-ab15-4221-98e0-cf0e02748a9b", + project_title: "Test Project", start_date: "1/1/2025", + end_date: "1/10/2025", line1: "10 Downing Street", + line2: "Westminster", townCity: "London", county: "LONDON", + postcode: "SW1A 2AA", description: "A description of my project...", + difference: "The difference my project will make to...", + matter: "My project matters because...", + best_placed_description: "My organisation is best placed to...", + heritage_description: "The heritage of my project...", + involvement_description: "My project will involve a wider range of " \ + "people...", + outcome_2: true, outcome_3: false, outcome_4: true, outcome_5: false, + outcome_6: true, outcome_7: false, outcome_8: true, outcome_9: false, + outcome_2_description: "Description of outcome 2", + outcome_3_description: "", + outcome_4_description: "Description of outcome 4", + outcome_5_description: "", + outcome_6_description: "Description of outcome 6", + outcome_7_description: "", + outcome_8_description: "Description of outcome 8", + outcome_9_description: "", permission_type: 2, + permission_description: "permission description", + partnership_details: "partnership details", + declaration_reasons_description: "something" + ) - organisation = build( + organisation = build( :organisation, name: "Test Organisation", org_type: 5, @@ -50,120 +48,152 @@ @project.user.organisations.append(organisation) - project_salesforce_json = JSON.parse(@project.to_salesforce_json) - - # Assert metadata parameters - expect(project_salesforce_json['meta']['applicationId']) - .to eq("2c660111-ab15-4221-98e0-cf0e02748a9b") - expect(project_salesforce_json['meta']['username']) - .to eq(@project.user.email) - - # Assert main contact parameters - expect(project_salesforce_json['application']['mainContactName']) - .to eq("Joe Bloggs") - expect(project_salesforce_json['application']['mainContactDateOfBirth']) - .to eq("1980-01-01") - expect(project_salesforce_json['application']['mainContactPhone']) - .to eq("07123456789") - expect(project_salesforce_json['application']['mainContactEmail']) - .to eq(@project.user.email) - expect(project_salesforce_json['application']['mainContactAddress']['line1']) - .to eq("10 Downing Street, Westminster") - expect(project_salesforce_json['application']['mainContactAddress']['townCity']) - .to eq("London") - expect(project_salesforce_json['application']['mainContactAddress']['county']) - .to eq("LONDON") - expect(project_salesforce_json['application']['mainContactAddress']['postcode']) - .to eq("SW1A 2AA") - - # Assert organisation parameters - expect(project_salesforce_json['application']['organisationName']) - .to eq("Test Organisation") - expect(project_salesforce_json['application']['organisationType']) - .to eq("faith-based-or-church-organisation") - expect(project_salesforce_json['application']['organisationMission']) - .to eq(%w(young-people-led disability-led)) - expect(project_salesforce_json['application']['charityNumber']) - .to eq("12345") - expect(project_salesforce_json['application']['companyNumber']) - .to eq("54321") - expect(project_salesforce_json['application']['organisationAddress']['line1']) - .to eq("10 Downing Street, Westminster") - expect(project_salesforce_json['application']['organisationAddress']['townCity']) - .to eq("London") - expect(project_salesforce_json['application']['organisationAddress']['county']) - .to eq("LONDON") - expect(project_salesforce_json['application']['organisationAddress']['postcode']) - .to eq("SW1A 2AA") - - # Assert project parameters - expect(project_salesforce_json['application']['projectName']) - .to eq("Test Project") - expect(project_salesforce_json['application']['projectDateRange']['startDate']) - .to eq("2025-01-01") - expect(project_salesforce_json['application']['projectDateRange']['endDate']) - .to eq("2025-10-01") - expect(project_salesforce_json['application']['projectAddress']['line1']) - .to eq("10 Downing Street, Westminster") - expect(project_salesforce_json['application']['projectAddress']['townCity']) - .to eq("London") - expect(project_salesforce_json['application']['projectAddress']['county']) - .to eq("LONDON") - expect(project_salesforce_json['application']['projectAddress']['projectPostcode']) - .to eq("SW1A 2AA") - expect(project_salesforce_json['application']['yourIdeaProject']) - .to eq("A description of my project...") - expect(project_salesforce_json['application']['projectDifference']) - .to eq("The difference my project will make to...") - expect(project_salesforce_json['application']['projectOrgBestPlace']) - .to eq("My organisation is best placed to...") - expect(project_salesforce_json['application']['projectAvailable']) - .to eq("The heritage of my project...") - expect(project_salesforce_json['application']['projectOutcome1']) - .to eq("My project will involve a wider range of people...") - expect(project_salesforce_json['application']['projectOutcome2']) - .to eq("Description of outcome 2") - expect(project_salesforce_json['application']['projectOutcome3']) - .to eq("") - expect(project_salesforce_json['application']['projectOutcome4']) - .to eq("Description of outcome 4") - expect(project_salesforce_json['application']['projectOutcome5']) - .to eq("") - expect(project_salesforce_json['application']['projectOutcome6']) - .to eq("Description of outcome 6") - expect(project_salesforce_json['application']['projectOutcome7']) - .to eq("") - expect(project_salesforce_json['application']['projectOutcome8']) - .to eq("Description of outcome 8") - expect(project_salesforce_json['application']['projectOutcome9']) - .to eq("") - expect(project_salesforce_json['application']['projectOutcome2Checked']) - .to eq(true) - expect(project_salesforce_json['application']['projectOutcome3Checked']) - .to eq(false) - expect(project_salesforce_json['application']['projectOutcome4Checked']) - .to eq(true) - expect(project_salesforce_json['application']['projectOutcome5Checked']) - .to eq(false) - expect(project_salesforce_json['application']['projectOutcome6Checked']) - .to eq(true) - expect(project_salesforce_json['application']['projectOutcome7Checked']) - .to eq(false) - expect(project_salesforce_json['application']['projectOutcome8Checked']) - .to eq(true) - expect(project_salesforce_json['application']['projectOutcome9Checked']) - .to eq(false) - expect(project_salesforce_json['application']['projectNeedsPermission']) - .to eq("not-sure") - expect(project_salesforce_json['application']['projectNeedsPermissionDetails']) - .to eq("permission description") - expect(project_salesforce_json['application']['partnershipDetails']) - .to eq("partnership details") - expect(project_salesforce_json['application']['informationNotPubliclyAvailableRequest']) - .to eq("something") + end + + context "with a cash contribution" do + + before do + @cash_contribution = build( + :cash_contribution, + description: "Test Contribution", + amount: 1000, + secured: 'x_not_sure' + ) + + @project.cash_contributions << @cash_contribution + + @project_salesforce_json = JSON.parse(@project.to_salesforce_json) + + end + it "should serialize Salesforce JSON successfully" do + # Assert metadata parameters + expect(@project_salesforce_json['meta']['applicationId']) + .to eq("2c660111-ab15-4221-98e0-cf0e02748a9b") + expect(@project_salesforce_json['meta']['username']) + .to eq(@project.user.email) + + # Assert main contact parameters + expect(@project_salesforce_json['application']['mainContactName']) + .to eq("Joe Bloggs") + expect(@project_salesforce_json['application']['mainContactDateOfBirth']) + .to eq("1980-01-01") + expect(@project_salesforce_json['application']['mainContactPhone']) + .to eq("07123456789") + expect(@project_salesforce_json['application']['mainContactEmail']) + .to eq(@project.user.email) + expect(@project_salesforce_json['application']['mainContactAddress']['line1']) + .to eq("10 Downing Street, Westminster") + expect(@project_salesforce_json['application']['mainContactAddress']['townCity']) + .to eq("London") + expect(@project_salesforce_json['application']['mainContactAddress']['county']) + .to eq("LONDON") + expect(@project_salesforce_json['application']['mainContactAddress']['postcode']) + .to eq("SW1A 2AA") + + # Assert organisation parameters + expect(@project_salesforce_json['application']['organisationName']) + .to eq("Test Organisation") + expect(@project_salesforce_json['application']['organisationType']) + .to eq("faith-based-or-church-organisation") + expect(@project_salesforce_json['application']['organisationMission']) + .to eq(%w(young-people-led disability-led)) + expect(@project_salesforce_json['application']['charityNumber']) + .to eq("12345") + expect(@project_salesforce_json['application']['companyNumber']) + .to eq("54321") + expect(@project_salesforce_json['application']['organisationAddress']['line1']) + .to eq("10 Downing Street, Westminster") + expect(@project_salesforce_json['application']['organisationAddress']['townCity']) + .to eq("London") + expect(@project_salesforce_json['application']['organisationAddress']['county']) + .to eq("LONDON") + expect(@project_salesforce_json['application']['organisationAddress']['postcode']) + .to eq("SW1A 2AA") + + # Assert project parameters + expect(@project_salesforce_json['application']['projectName']) + .to eq("Test Project") + expect(@project_salesforce_json['application']['projectDateRange']['startDate']) + .to eq("2025-01-01") + expect(@project_salesforce_json['application']['projectDateRange']['endDate']) + .to eq("2025-10-01") + expect(@project_salesforce_json['application']['projectAddress']['line1']) + .to eq("10 Downing Street, Westminster") + expect(@project_salesforce_json['application']['projectAddress']['townCity']) + .to eq("London") + expect(@project_salesforce_json['application']['projectAddress']['county']) + .to eq("LONDON") + expect(@project_salesforce_json['application']['projectAddress']['projectPostcode']) + .to eq("SW1A 2AA") + expect(@project_salesforce_json['application']['yourIdeaProject']) + .to eq("A description of my project...") + expect(@project_salesforce_json['application']['projectDifference']) + .to eq("The difference my project will make to...") + expect(@project_salesforce_json['application']['projectOrgBestPlace']) + .to eq("My organisation is best placed to...") + expect(@project_salesforce_json['application']['projectAvailable']) + .to eq("The heritage of my project...") + expect(@project_salesforce_json['application']['projectOutcome1']) + .to eq("My project will involve a wider range of people...") + expect(@project_salesforce_json['application']['projectOutcome2']) + .to eq("Description of outcome 2") + expect(@project_salesforce_json['application']['projectOutcome3']) + .to eq("") + expect(@project_salesforce_json['application']['projectOutcome4']) + .to eq("Description of outcome 4") + expect(@project_salesforce_json['application']['projectOutcome5']) + .to eq("") + expect(@project_salesforce_json['application']['projectOutcome6']) + .to eq("Description of outcome 6") + expect(@project_salesforce_json['application']['projectOutcome7']) + .to eq("") + expect(@project_salesforce_json['application']['projectOutcome8']) + .to eq("Description of outcome 8") + expect(@project_salesforce_json['application']['projectOutcome9']) + .to eq("") + expect(@project_salesforce_json['application']['projectOutcome2Checked']) + .to eq(true) + expect(@project_salesforce_json['application']['projectOutcome3Checked']) + .to eq(false) + expect(@project_salesforce_json['application']['projectOutcome4Checked']) + .to eq(true) + expect(@project_salesforce_json['application']['projectOutcome5Checked']) + .to eq(false) + expect(@project_salesforce_json['application']['projectOutcome6Checked']) + .to eq(true) + expect(@project_salesforce_json['application']['projectOutcome7Checked']) + .to eq(false) + expect(@project_salesforce_json['application']['projectOutcome8Checked']) + .to eq(true) + expect(@project_salesforce_json['application']['projectOutcome9Checked']) + .to eq(false) + expect(@project_salesforce_json['application']['projectNeedsPermission']) + .to eq("not-sure") + expect(@project_salesforce_json['application']['projectNeedsPermissionDetails']) + .to eq("permission description") + expect(@project_salesforce_json['application']['partnershipDetails']) + .to eq("partnership details") + expect(@project_salesforce_json['application']['informationNotPubliclyAvailableRequest']) + .to eq("something") + expect(@project_salesforce_json['application']['cashContributions'][0]['description']) + .to eq("Test Contribution") + expect(@project_salesforce_json['application']['cashContributions'][0]['amount']) + .to eq(1000) + expect(@project_salesforce_json['application']['cashContributions'][0]['secured']) + .to eq('not sure') + end end - end + context "without a cash contribution" do + before do + @project_salesforce_json = JSON.parse(@project.to_salesforce_json) + end -end + it "should serialise a project without cash contributions Salesforce JSON successfully" do + expect(@project_salesforce_json['application']['cashContributions']).to eq([]) + end + end + + end +end \ No newline at end of file From 6ef4cc86d5f97123d207510b1021a9831ddc0d41 Mon Sep 17 00:00:00 2001 From: Jack <91466216+JJD1990@users.noreply.github.com> Date: Wed, 20 Sep 2023 11:51:48 +0100 Subject: [PATCH 13/14] inital regex alteration to enforce domains in email validation (#1146) * inital regex alteration to enforce domains in email validation * add tests for legal sig regex and registration * tests written for email validation, legal sig model and user model * comments added to explain regex in devise.rb and legalsig model * deleted end of file extra lines * misunderstanding, added EOF lines back in! * eof line to user_spec * took out debugging comments in user_spec --------- Co-authored-by: eithel --- app/models/legal_signatory.rb | 4 +- config/initializers/devise.rb | 7 +- .../second_signatory_controller_spec.rb | 26 ++++++ .../users/registrations_controller_spec.rb | 1 + spec/models/legal_signatory_spec.rb | 86 +++++++++++++++++++ spec/models/user_spec.rb | 25 +++++- 6 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 spec/models/legal_signatory_spec.rb diff --git a/app/models/legal_signatory.rb b/app/models/legal_signatory.rb index 663808aab..b2b368574 100644 --- a/app/models/legal_signatory.rb +++ b/app/models/legal_signatory.rb @@ -14,8 +14,10 @@ class LegalSignatory < ApplicationRecord validates :name, length: { minimum: 1, maximum: 80 } + # the custom regex below ensures that a domain + # is present and also allows tags. validates :email_address, - format: { with: URI::MailTo::EMAIL_REGEXP } + format: { with: /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i } def validate_role? validate_role == true diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 87f400fa0..62660d6a6 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -171,7 +171,12 @@ # Email regex used to validate email formats. It simply asserts that # one (and only one) @ exists in the given string. This is mainly # to give user feedback and not to assert the e-mail validity. - config.email_regexp = /\A[^@\s]+@[^@\s]+\z/ + # original email_regexp /\A[^@\s]+@[^@\s]+\z/ + + # a custom regex has now been added below, this ensures that a domain + # is present and also allows tags. + config.email_regexp =/\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i + # ==> Configuration for :timeoutable # The time you want to timeout the user session without activity. After this diff --git a/spec/controllers/funding_application/legal_agreements/second_signatory_controller_spec.rb b/spec/controllers/funding_application/legal_agreements/second_signatory_controller_spec.rb index 840ab04ad..c4bbb46ef 100644 --- a/spec/controllers/funding_application/legal_agreements/second_signatory_controller_spec.rb +++ b/spec/controllers/funding_application/legal_agreements/second_signatory_controller_spec.rb @@ -157,6 +157,32 @@ end + it "should raise email error based on invalid email validation " \ + "when email without a domain is passed" do + + put :update, + params: { + application_id: @funding_application.id, + legal_signatory:{ + name: "John Smith", + email_address: "john@smith", + role: "Trustee" + } + } + + expect(response).to have_http_status(:success) + expect(response).to render_template(:show) + + expect(assigns(:funding_application).errors.empty?).to eq(false) + + expect(assigns(:funding_application).errors.count) + .to eq(1) + + expect(assigns(:funding_application).errors[:"legal_signatories.email_address"][0]) + .to eq("Enter a valid email address") + + end + it "should raise email error based matching email address of " \ "legal signatory 1 and legal signatory 2" do diff --git a/spec/controllers/users/registrations_controller_spec.rb b/spec/controllers/users/registrations_controller_spec.rb index 42751beed..9d2c42108 100644 --- a/spec/controllers/users/registrations_controller_spec.rb +++ b/spec/controllers/users/registrations_controller_spec.rb @@ -30,4 +30,5 @@ subject.create_person(resource) end end + end diff --git a/spec/models/legal_signatory_spec.rb b/spec/models/legal_signatory_spec.rb new file mode 100644 index 000000000..e02272938 --- /dev/null +++ b/spec/models/legal_signatory_spec.rb @@ -0,0 +1,86 @@ +require "rails_helper" + +RSpec.describe LegalSignatory, type: :model do + subject { build(:legal_signatory) } + + context "Validations" do + it "validates the length of role" do + subject.role = 'a' * 81 + expect(subject.valid?).to be_falsey + expect(subject.errors[:role]).to include("The role of the legal signatory must be fewer than 80 characters") + + subject.role = '' + expect(subject.valid?).to be_falsey + expect(subject.errors[:role]).to include("Enter the role of a legal signatory") + + subject.role = 'Valid Role' + expect(subject.valid?).to be_truthy + end + + it "validates the length of name" do + subject.name = 'a' * 81 + expect(subject.valid?).to be_falsey + expect(subject.errors[:name]).to include("The name of the legal signatory must be fewer than 80 characters") + + subject.name = '' + expect(subject.valid?).to be_falsey + expect(subject.errors[:name]).to include("Enter the name of a legal signatory") + + subject.name = 'Valid Name' + expect(subject.valid?).to be_truthy + end + end + + describe "Legal model" do + + let (:resource) { + create( + :legal_signatory, + id: 1, + email_address: 'a@f.com', + role: 'role' + ) + } + + context "when email is invalid" do + let(:invalid_emails) { ['invalid', 'invalid@', 'invalid@.com', '@invalid.com', 'invalid@invalid'] } + + it "should be invalid" do + invalid_emails.each do |email| + resource.email_address = email + expect(resource.valid?).to eq(false) + end + end + end + + context "when email is valid" do + let(:valid_emails) { ['valid@example.com', 'valid.name@example.com', 'valid.name+tag@example.co.uk', 'valid-name@example.co.uk'] } + + it "should be valid" do + valid_emails.each do |email| + resource.email_address = email + unless resource.valid? + puts "Validation failed for email #{email}" + puts resource.errors.full_messages + end + expect(resource.valid?).to eq(true) + end + end + end + + describe "Conditionally validating email_address" do + it "should validate email_address when validate_email_address is set to true" do + subject.validate_email_address = true + expect(subject.validate_email_address?).to eq(true) + end + end + + describe "Conditionally validating phone number" do + it "should validate phone number when validate_phone_number is set to true" do + subject.validate_phone_number = true + expect(subject.validate_phone_number?).to eq(true) + end + end + + end +end \ No newline at end of file diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 002e853f8..44d277813 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -92,9 +92,30 @@ expect(resource.send_english_mails?).to eq(true) expect(resource.send_welsh_mails?).to eq(false) expect(resource.send_bilingual_mails?).to eq(false) + end - end - end + context "when email is invalid" do + let(:invalid_emails) { ['invalid', 'invalid@', 'invalid@.com', '@invalid.com', 'invalid@invalid'] } + + it "should be invalid" do + invalid_emails.each do |email| + resource.email = email + expect(resource.valid?).to eq(false) + end + end + end + context "when email is valid" do + let(:valid_emails) { ['valid@example.com', 'valid.name@example.com', 'valid.name+tag@example.co.uk', 'valid-name@example.co.uk'] } + + it "should be valid" do + valid_emails.each do |email| + resource.email = email + expect(resource.valid?).to eq(true) + end + end + end + + end end From 82020dcd2e68034673b28d31ee707ab88da77b1a Mon Sep 17 00:00:00 2001 From: Jack <91466216+JJD1990@users.noreply.github.com> Date: Mon, 25 Sep 2023 12:31:47 +0100 Subject: [PATCH 14/14] Feature/create ogranisation model spec (#1148) * feat/create org spec * context for empty address fields with error tests added * add org type tests * rearranged tests in oragnisation spec, added invalid org type test * taken contexts out, used factory organisations * test org mission validation * refactored spec, configured org factory to allow test suite to run properly, added further organisation tests * all organisation tests added to file, need to refactor further * refactored organisation factory and references * organisation_spec refactored * association tests added to organisation spec * comment added about shoulda gem * added EOF lines --------- Co-authored-by: eithel --- spec/factories/organisations.rb | 68 ++++- spec/models/organisation_spec.rb | 426 ++++++++++++++++++++++++++++++- 2 files changed, 481 insertions(+), 13 deletions(-) diff --git a/spec/factories/organisations.rb b/spec/factories/organisations.rb index 6e52c882b..e9a3722d4 100644 --- a/spec/factories/organisations.rb +++ b/spec/factories/organisations.rb @@ -1,7 +1,69 @@ FactoryBot.define do - factory :organisation do |f| - + # This blank :organisation is used throughout the test suite + # best not to change it without knowing where its used. + factory :organisation do end -end + # Everything below and including this organisation model is + # used within the organisation_spec.rb. + trait :organisation_model do + id { SecureRandom.uuid } + created_at { Time.current } + updated_at { Time.current } + line1 { "123 Main Street" } + line2 { "Flat 3" } + line3 { "Third Floor" } + townCity { "Plymouth" } + county { "Devon" } + postcode { "PL1 3TT" } + org_type { 0 } + company_number { "COMP12345" } + charity_number { "CHAR12345" } + charity_number_ni { 7890 } + mission { ["black_or_minority_ethnic_led"] } + salesforce_account_id { "sf-123456789" } + custom_org_type { "CustomType" } + main_purpose_and_activities { "Main purpose and activities text" } + spend_in_last_financial_year { 1000.00 } + unrestricted_funds { 500.00 } + board_members_or_trustees { 5 } + vat_registered { true } + vat_number { "GB123456789" } + social_media_info { "Follow us on Twitter @test_org" } + end + + # A trait to allow testing of blank attributes + #that must be present. + trait :blank_organisation do + after(:build) do |org| + org.validate_name = true + org.validate_address = true + org.validate_org_type = true + end + + org_type { nil } + custom_org_type { nil } + name { nil } + line1 { nil } + townCity { nil } + county { nil } + postcode { nil } + main_purpose_and_activities { nil } + end + + trait :valid_organisation do + name { 'A' * 255 } + end + + trait :invalid_organisation do + name { 'A' * 256 } + end + + trait :invalid_mission do + mission { ["invalid_value1", "invalid_value2", "black_or_minority_ethnic_led" ] } + validate_mission {true} + end + + end + \ No newline at end of file diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 5a741cda0..f386fc110 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -1,17 +1,423 @@ require 'rails_helper' RSpec.describe Organisation, type: :model do - let(:valid_organisation_1) { Organisation.new(name: 'A' * 255) } - let(:valid_organisation_2) { Organisation.new(name: 'A' * 100) } - let(:invalid_organisation) { Organisation.new(name: 'A' * 256) } + subject {build(:organisation)} + let(:valid_organisation) { build(:organisation, :organisation_model, :valid_organisation) } + let(:invalid_mission_organisation) { build(:organisation, :organisation_model, :invalid_organisation, :invalid_mission) } + let(:blank_organisation) { build(:organisation, :organisation_model, :blank_organisation) } + let(:not_vat_registered_org) { build(:organisation, :organisation_model, vat_registered: false, validate_vat_registered: true) } + let(:invalid_vat_registered_org) { build(:organisation, :organisation_model, vat_registered: nil, validate_vat_registered: true) } + let(:custom_org_type_blank) { build(:organisation, :organisation_model, custom_org_type: nil, validate_custom_org_type: true) } - it 'validates length of name to be less than or equal to 255 characters' do - expect(invalid_organisation.valid?).to be(false) - expect(invalid_organisation.errors[:name]).to include("Organisation name must be 255 characters or fewer") + # Set the state of the organisations to ensure any error + # messages are there to be seen in the tests. + before do + blank_organisation.valid? end - it 'is valid when organisation name is equal to or below 255 characters' do - expect(valid_organisation_1.valid?).to be(true) - expect(valid_organisation_2.valid?).to be(true) + # create a hash of attributes/fields that should have presence + # of errors. + describe "Validation of mandatory fields" do + fields_with_presence_errors = { + name: 'Enter the name of your organisation', + line1: "Enter the first line of your organisation's address", + townCity: "Enter the town or city where your organisation is located", + county: "Enter the county where your organisation is located", + postcode: "Enter the postcode where your organisation is located", + org_type: "Select the type of organisation that will be running your project" + } + + # Loop through each field to check they have an error + # and that the error matches what it should be. + fields_with_presence_errors.each do |field, message| + it "is invalid without a #{field}" do + blank_organisation[field] = nil + expect(blank_organisation.valid?).to be(false) + expect(blank_organisation.errors[field]).to include(message) + end + end + end + + # create a hash of attributes/fields that should have length limits + # with their error message. + describe "Validation of length for relevant fields" do + length_fields = { + name: [255, "Organisation name must be 255 characters or fewer"], + company_number: [20, "Company number must be 20 characters or fewer"], + charity_number: [20, "Charity number must be 20 characters or fewer. For example 1234567 in England and Wales, SC000123 in Scotland, or 10000-0 in Northern Ireland"], + vat_number: [[9, 12], "Enter the VAT number of your organisation in the correct format"] + } + + # Loop through each field to check they have an error + # and that the error matches what it should be. + length_fields.each do |field, details| + max_length, message = details + + it "validates length of #{field} to be within valid constraints" do + expect(valid_organisation.valid?).to be(true) + + if max_length.is_a?(Array) + # For VAT number, we have a range. + min_len, max_len = max_length + too_long = build(:organisation, field => 'A' * (max_len + 1)) + too_short = build(:organisation, field => 'A' * (min_len - 1)) + + if field == :vat_number + too_long.validate_vat_number = true + too_short.validate_vat_number = true + end + + expect(too_long.valid?).to be(false) + expect(too_short.valid?).to be(false) + expect(too_long.errors[field]).to include(message) + expect(too_short.errors[field]).to include(message) + + else + too_long = build(:organisation, field => 'A' * (max_length + 1)) + + if field == :company_number + too_long.validate_company_number = true + elsif field == :charity_number + too_long.validate_charity_number = true + end + + expect(too_long.valid?).to be(false) + expect(too_long.errors[field]).to include(message) + end + end + end + end + + # org_type tests + describe "validation or org_type" do + it 'has a valid org type' do + expect(valid_organisation.valid?).to be(true) + expect(blank_organisation.errors[:org_type]).to include("Select the type of organisation that will be running your project") + end + + it 'validates the presence of org_type when org_type is blank' do + expect(blank_organisation.valid?).to be(false) + expect(blank_organisation.errors[:org_type]).to include("Select the type of organisation that will be running your project") + end + + it 'validates the org_type with the correct enum' do + valid_org_type = build(:organisation, org_type: 3) + expect(valid_org_type.org_type).to eq("community_interest_company") + end + + it 'should allow organization types within the range 0 to 11' do + (0..11).each do |org_type| + valid_org = build(:organisation, org_type: org_type) + expect(valid_org.valid?).to be(true), "Expected organization type #{org_type} to be valid, but got errors: #{valid_org.errors[:org_type].join(', ')}" + end + end + + # We are testing an enum, so should recieve an ArgumentError. + it 'should raise an ArgumentError for invalid organization types' do + invalid_org_types = [-1, 12, 200, "invalid"] + invalid_org_types.each do |org_type| + expect { subject.org_type = org_type }.to raise_error(ArgumentError), "Expected an ArgumentError to be raised for org_type #{org_type.inspect}, but it wasn't." + end + end + end + + # testing custom_org_type + describe "Validation of custom_org_type" do + it 'passes validation if custom_org_type is present when validate_custom_org_type is true' do + expect(valid_organisation.valid?).to be(true) + end + + it 'fails validation if custom_org_type is blank when validate_custom_org_type is true' do + blank_organisation.validate_custom_org_type = true + expect(blank_organisation.valid?).to be(false) + expect(blank_organisation.errors[:custom_org_type]).to include("Specify your organisation type") + end + + it 'passes validation regardless of custom_org_type value when validate_custom_org_type is false' do + custom_org_type_blank.validate_custom_org_type = false + expect(custom_org_type_blank.valid?).to be(true) + end + end + + # mission tests - here we test the validate_mission_array method + describe "Validation of mission and mission_array" do + it 'validates the mission with the correct value ' do + expect(valid_organisation.mission).to eq(["black_or_minority_ethnic_led"]) + expect(invalid_mission_organisation.valid?).to be(false) + end + + it 'adds no error when mission contains only valid values' do + valid_organisation.mission = ["black_or_minority_ethnic_led", "female_led"] + valid_organisation.valid? + expect(valid_organisation.errors[:mission]).to be_empty + end + + it "adds an error when mission contains an invalid value" do + invalid_mission_organisation.valid? + expect(invalid_mission_organisation.errors[:mission]).to include("invalid_value1 is not a valid selection") + end + + it "adds multiple errors when mission contains multiple invalid values" do + invalid_mission_organisation = Organisation.new( + mission: ["invalid_value1", "invalid_value2"], + validate_mission: true + ) + invalid_mission_organisation.valid? + expect(invalid_mission_organisation.errors[:mission]).to include("invalid_value1 is not a valid selection", "invalid_value2 is not a valid selection") + end + + it 'adds no errors when mission is nil' do + expect(blank_organisation.errors[:mission]).to be_empty + end + + it 'adds no errors when mission is an empty array' do + blank_organisation.mission = [] + blank_organisation.valid? + expect(blank_organisation.errors[:mission]).to be_empty + end + end + + # More complex tests to assert the validate_length methods work + # via a loop + describe "Test the validate_length methods" do + [ + [:main_purpose_and_activities, 'activerecord.errors.models.organisation.attributes.main_purpose_and_activities.too_long'], + [:social_media_info, 'activerecord.errors.models.organisation.attributes.social_media_info.too_long'] + ].each do |attribute, translation_key| + it "validates the length of #{attribute}, must be 500 characters or fewer" do + subject.send("validate_#{attribute}=", true) + subject.send("#{attribute}=", "A " * 501) + subject.valid? + + expect(subject.errors[attribute]).to include( + I18n.t( + translation_key, + word_count: 500 + ) + ) + end + end + end + + # tests for board_members_or_trustees, main_purpose_and_activities + # spend_in_last_financial_year and unrestricted_funds + # Iterate through each set of test data for different attributes. + # Each set of test data consists of an attribute and an array of test cases. + describe "More complex validations for attributes" do + [ + { + attribute: :board_members_or_trustees, # The attribute to be tested + cases: [ + # Array of test cases, each containing a value to test and the expected error message. + { value: -1, error: "Enter an amount greater than -1" }, + { value: "Twenty One", error: "Number of board members or trustees must be a number" }, + { value: 2147483648, error: "Enter an amount less than 2147483648" }, + { value: nil, error: nil } + ] + }, + { + attribute: :main_purpose_and_activities, + cases: [ + { value: nil, error: "Enter your organisation's main purpose or activities" }, + { value: "Some Activities", error: nil } + ] + }, + { + attribute: :spend_in_last_financial_year, + cases: [ + { value: 0, error: "Enter an amount greater than 0" }, + { value: "Ninety Pound", error: "Must be a number, like 500" }, + { value: nil, error: nil }, + { value: 900000, error: nil } + ] + }, + { + attribute: :unrestricted_funds, + cases: [ + { value: 0, error: "Enter an amount greater than 0" }, + { value: "Ninety Thousand Pounds", error: "Level of unrestricted funds must be a number" }, + { value: nil, error: nil }, + { value: 900000, error: nil } + ] + } + ].each do |test_data| + attribute = test_data[:attribute] + cases = test_data[:cases] + + # Testing when the corresponding validate flag for the attribute is true + context "when validate_#{attribute} is true" do + before { subject.send("validate_#{attribute}=", true) } + + # Iterate through each case and apply the test + cases.each do |test_case| + it "handles value: #{test_case[:value]}" do + subject.send("#{attribute}=", test_case[:value]) + + # Validate the subject and compare with the expected outcome + expect(subject.valid?).to eq(test_case[:error].nil?) + + # Check for error messages if any are expected + if test_case[:error] + expect(subject.errors[attribute]).to include(test_case[:error]) + else + expect(subject.errors[attribute]).to be_empty + end + end + end + end + + describe "Conditional Validation of Attributes" do + # Testing when the corresponding validate flag for the attribute is false + context "when validate_#{attribute} is false" do + before { subject.send("validate_#{attribute}=", false) } + + cases.each do |test_case| + it "skips validation for value: #{test_case[:value]}" do + subject.send("#{attribute}=", test_case[:value]) + expect(subject.valid?).to be(true) + expect(subject.errors[attribute]).to be_empty + end + end + end + end + end + + end + + # Tests inclusion of vat_registered + describe "VAT Registered Validations" do + it 'fails validation if vat_registered is neither true or false when validate_vat_registered is true' do + expect(invalid_vat_registered_org.valid?).to be(false) + expect(invalid_vat_registered_org.errors[:vat_registered]).to include("Select an option to tell us whether your organisation is VAT registered") + end + + it 'passes validation if vat_registered is true when validate_vat_registered is true' do + expect(valid_organisation.valid?).to be(true) + end + + it 'passes validation if vat_registered is false when validate_vat_registered is true' do + expect(not_vat_registered_org.valid?).to be(true) + end + + it 'passes validation regardless of vat_registered value when validate_vat_registered is false' do + invalid_vat_registered_org.validate_vat_registered = false + expect(invalid_vat_registered_org.valid?).to be(true) + end + end + + # Tests that the validate_xyz? methods work + describe "Conditionally validating fields" do + fields_to_validate = [ + :name, + :org_type, + :custom_org_type, + :address, + :mission, + :main_purpose_and_activities, + :board_members_or_trustees, + :vat_registered, + :vat_number, + :company_number, + :charity_number, + :social_media_info, + :spend_in_last_financial_year, + :unrestricted_funds + ] + + fields_to_validate.each do |field| + it "should validate #{field} when validate_#{field} is set to true" do + subject.public_send("validate_#{field}=", true) + expect(subject.public_send("validate_#{field}?")).to eq(true) + end + end end -end \ No newline at end of file + + # Tests for Organisation associations + # We could use the 'shoulda' gem which tests associations + describe 'Associations' do + + it 'can exist without pre_applications' do + expect(valid_organisation.pre_applications).to be_empty + end + + it 'can have many pre_applications' do + pre_application1 = create(:pre_application, organisation: valid_organisation) + pre_application2 = create(:pre_application, organisation: valid_organisation) + + expect(valid_organisation.pre_applications).to include(pre_application1, pre_application2) + end + + it 'can exist without a funding_applications' do + expect(valid_organisation.funding_applications).to be_empty + end + + it 'can have many funding_applications' do + funding_application1 = create(:funding_application, organisation: valid_organisation) + funding_application2 = create(:funding_application, organisation: valid_organisation) + + expect(valid_organisation.funding_applications).to include(funding_application1, funding_application2) + end + + it 'can exist without organisations_org_types' do + expect(valid_organisation.organisations_org_types).to be_empty + end + + it 'can have many organisations_org_types' do + organisation = Organisation.create!() + + org_type_1 = OrgType.create!(id: SecureRandom.uuid, created_at: DateTime.now, updated_at: DateTime.now) + org_type_2 = OrgType.create!(id: SecureRandom.uuid, created_at: DateTime.now, updated_at: DateTime.now) + + organisations_org_type_1 = OrganisationsOrgType.create!(id: SecureRandom.uuid, organisation: organisation, org_type: org_type_1, created_at: DateTime.now, updated_at: DateTime.now) + organisations_org_type_2 = OrganisationsOrgType.create!(id: SecureRandom.uuid, organisation: organisation, org_type: org_type_2, created_at: DateTime.now, updated_at: DateTime.now) + + expect(organisation.organisations_org_types).to include(organisations_org_type_1, organisations_org_type_2) + end + + it 'can have many org_types through organisations_org_types' do + organisation = Organisation.create!() + + org_type1 = OrgType.create!() + org_type2 = OrgType.create!() + + OrganisationsOrgType.create!(organisation: organisation, org_type: org_type1) + OrganisationsOrgType.create!(organisation: organisation, org_type: org_type2) + + expect(organisation.org_types).to include(org_type1, org_type2) + end + + it 'can exist without org_types through organisations_org_types' do + expect(valid_organisation.org_types).to be_empty + end + + it 'can exist without users_organisations' do + expect(valid_organisation.users_organisations).to be_empty + end + + it 'can have many users_organisations' do + user1 = create(:user) + user2 = create(:user) + + user_org1 = create(:users_organisation, organisation: valid_organisation, user: user1) + user_org2 = create(:users_organisation, organisation: valid_organisation, user: user2) + + expect(valid_organisation.users_organisations).to include(user_org1, user_org2) + end + + + it 'can exist without users through users_organisations' do + expect(valid_organisation.users).to be_empty + end + + it 'can have many users through users_organisations' do + user1 = create(:user) + user2 = create(:user) + + create(:users_organisation, organisation: valid_organisation, user: user1) + create(:users_organisation, organisation: valid_organisation, user: user2) + + expect(valid_organisation.users).to include(user1, user2) + end + end + +end