diff --git a/.github/workflows/aws_deploy.yml b/.github/workflows/aws_deploy.yml index 8fa267a69..75b63d6dd 100644 --- a/.github/workflows/aws_deploy.yml +++ b/.github/workflows/aws_deploy.yml @@ -6,7 +6,10 @@ on: aws_account_id: required: true type: string - aws_resource_prefix: + aws_role_prefix: + required: true + type: string + aws_task_prefix: required: true type: string environment: @@ -104,12 +107,12 @@ jobs: uses: aws-actions/configure-aws-credentials@v3 with: aws-region: ${{ env.aws_region }} - role-to-assume: arn:aws:iam::${{ inputs.aws_account_id }}:role/${{ inputs.aws_resource_prefix }}-deployment + role-to-assume: arn:aws:iam::${{ inputs.aws_account_id }}:role/${{ inputs.aws_role_prefix }}-deployment role-chaining: true - name: Download ad hoc task definition env: - ad_hoc_task_definition: ${{ inputs.aws_resource_prefix }}-ad-hoc + ad_hoc_task_definition: ${{ inputs.aws_task_prefix }}-ad-hoc run: | aws ecs describe-task-definition --task-definition $ad_hoc_task_definition --query taskDefinition > ad-hoc-task-definition.json @@ -126,11 +129,28 @@ jobs: with: task-definition: ${{ steps.ad-hoc-task-def.outputs.task-definition }} + - name: Setup Database + if: ${{ inputs.environment == 'review' }} + env: + ad_hoc_task_definition: ${{ inputs.aws_task_prefix }}-ad-hoc + cluster: ${{ inputs.aws_task_prefix }}-app + service: ${{ inputs.aws_task_prefix }}-app + run: | + network=$(aws ecs describe-services --cluster $cluster --services $service --query services[0].networkConfiguration) + overrides='{ "containerOverrides" : [{ "name" : "app", "command" : ["bundle", "exec", "rake", "db:prepare"]}]}' + arn=$(aws ecs run-task --cluster $cluster --task-definition $ad_hoc_task_definition --network-configuration "$network" --overrides "$overrides" --group migrations --launch-type FARGATE --query tasks[0].taskArn) + echo "Waiting for db prepare task to complete" + temp=${arn##*/} + id=${temp%*\"} + aws ecs wait tasks-stopped --cluster $cluster --tasks $id + succeeded=$(aws ecs describe-tasks --cluster $cluster --tasks $id --query "tasks[0].stopCode == 'EssentialContainerExited' && to_string(tasks[0].containers[0].exitCode) == '0'") + if [ $succeeded == true ]; then exit 0; else exit 1; fi + - name: Run migrations task env: - ad_hoc_task_definition: ${{ inputs.aws_resource_prefix }}-ad-hoc - cluster: ${{ inputs.aws_resource_prefix }}-app - service: ${{ inputs.aws_resource_prefix }}-app + ad_hoc_task_definition: ${{ inputs.aws_task_prefix }}-ad-hoc + cluster: ${{ inputs.aws_task_prefix }}-app + service: ${{ inputs.aws_task_prefix }}-app run: | network=$(aws ecs describe-services --cluster $cluster --services $service --query services[0].networkConfiguration) overrides='{ "containerOverrides" : [{ "name" : "app", "command" : ["bundle", "exec", "rake", "db:migrate"]}]}' @@ -144,7 +164,7 @@ jobs: - name: Download app service task definition env: - app_task_definition: ${{ inputs.aws_resource_prefix }}-app + app_task_definition: ${{ inputs.aws_task_prefix }}-app run: | aws ecs describe-task-definition --task-definition $app_task_definition --query taskDefinition > app-task-definition.json @@ -159,14 +179,14 @@ jobs: - name: Deploy updated application uses: aws-actions/amazon-ecs-deploy-task-definition@v1 with: - cluster: ${{ inputs.aws_resource_prefix }}-app - service: ${{ inputs.aws_resource_prefix }}-app + cluster: ${{ inputs.aws_task_prefix }}-app + service: ${{ inputs.aws_task_prefix }}-app task-definition: ${{ steps.app-task-def.outputs.task-definition }} wait-for-service-stability: true - name: Download sidekiq service task definition env: - sidekiq_task_definition: ${{ inputs.aws_resource_prefix }}-sidekiq + sidekiq_task_definition: ${{ inputs.aws_task_prefix }}-sidekiq run: | aws ecs describe-task-definition --task-definition $sidekiq_task_definition --query taskDefinition > sidekiq-task-definition.json @@ -181,7 +201,7 @@ jobs: - name: Deploy updated sidekiq uses: aws-actions/amazon-ecs-deploy-task-definition@v1 with: - cluster: ${{ inputs.aws_resource_prefix }}-app - service: ${{ inputs.aws_resource_prefix }}-sidekiq + cluster: ${{ inputs.aws_task_prefix }}-app + service: ${{ inputs.aws_task_prefix }}-sidekiq task-definition: ${{ steps.sidekiq-task-def.outputs.task-definition }} wait-for-service-stability: true diff --git a/.github/workflows/production_pipeline.yml b/.github/workflows/production_pipeline.yml index 52540e92b..cb79038ef 100644 --- a/.github/workflows/production_pipeline.yml +++ b/.github/workflows/production_pipeline.yml @@ -215,7 +215,8 @@ jobs: uses: ./.github/workflows/aws_deploy.yml with: aws_account_id: 977287343304 - aws_resource_prefix: core-prod + aws_task_prefix: core-prod + aws_role_prefix: core-prod environment: production release_tag: ${{ needs.test.outputs.releasetag }} permissions: diff --git a/.github/workflows/review_pipeline.yml b/.github/workflows/review_pipeline.yml index d5921e049..bf2d1b0dc 100644 --- a/.github/workflows/review_pipeline.yml +++ b/.github/workflows/review_pipeline.yml @@ -1,6 +1,7 @@ name: Review app pipeline -concurrency: ${{ github.workflow }}-${{ github.event.pull_request.number }} +concurrency: + group: review-${{ github.event.pull_request.number }} on: pull_request: @@ -15,160 +16,41 @@ defaults: shell: bash jobs: - postgres: - name: Provision postgres - runs-on: ubuntu-latest - environment: review - - steps: - - name: Install Cloud Foundry CLI - run: | - wget --user-agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15" -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo apt-key add - - echo "deb https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list - sudo apt-get update - sudo apt-get install cf8-cli - - - name: Provision postgres - env: - CF_USERNAME: ${{ secrets.CF_USERNAME }} - CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - CF_API_ENDPOINT: ${{ secrets.CF_API_ENDPOINT }} - CF_SPACE: dev - CF_ORG: ${{ secrets.CF_ORG }} - run: | - cf api $CF_API_ENDPOINT - cf auth - cf target -o $CF_ORG -s $CF_SPACE - cf create-service postgres tiny-unencrypted-13 dluhc-core-review-${{ github.event.pull_request.number }}-postgres --wait - - redis: - name: Provision redis - runs-on: ubuntu-latest - environment: review - - steps: - - name: Install Cloud Foundry CLI - run: | - wget --user-agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15" -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo apt-key add - - echo "deb https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list - sudo apt-get update - sudo apt-get install cf8-cli - - - name: Provision redis - env: - CF_USERNAME: ${{ secrets.CF_USERNAME }} - CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - CF_API_ENDPOINT: ${{ secrets.CF_API_ENDPOINT }} - CF_SPACE: dev - CF_ORG: ${{ secrets.CF_ORG }} - run: | - cf api $CF_API_ENDPOINT - cf auth - cf target -o $CF_ORG -s $CF_SPACE - cf create-service redis micro-6.x dluhc-core-review-${{ github.event.pull_request.number }}-redis --wait + infra: + name: Deploy review app infrastructure + uses: communitiesuk/submit-social-housing-lettings-and-sales-data-infrastructure/.github/workflows/create_review_app_infra.yml@main + with: + key: ${{ github.event.pull_request.number }} + app_repo_role: arn:aws:iam::815624722760:role/core-application-repo + permissions: + id-token: write + + code: + name: Deploy review app code + needs: [infra] + uses: ./.github/workflows/aws_deploy.yml + with: + aws_account_id: 837698168072 + aws_role_prefix: core-dev + aws_task_prefix: core-review-${{ github.event.pull_request.number }} + environment: review + permissions: + id-token: write - deploy: - name: Deploy review app + comment: + name: Add link to PR + needs: [code] runs-on: ubuntu-latest - environment: review - needs: [postgres, redis] permissions: issues: write pull-requests: write steps: - - name: Checkout code - uses: actions/checkout@v3 - - - name: Install Cloud Foundry CLI - run: | - wget --user-agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15" -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo apt-key add - - echo "deb https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list - sudo apt-get update - sudo apt-get install cf8-cli - - - name: Setup review app without starting - env: - CF_USERNAME: ${{ secrets.CF_USERNAME }} - CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - CF_API_ENDPOINT: ${{ secrets.CF_API_ENDPOINT }} - CF_SPACE: dev - CF_ORG: ${{ secrets.CF_ORG }} - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - run: | - cf api $CF_API_ENDPOINT - cf auth - cf target -o $CF_ORG -s $CF_SPACE - cf push $APP_NAME \ - --manifest ./config/cloud_foundry/review_manifest.yml \ - --no-start - - - name: Set environment variables - env: - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - API_USER: ${{ secrets.API_USER }} - API_KEY: ${{ secrets.API_KEY }} - GOVUK_NOTIFY_API_KEY: ${{ secrets.GOVUK_NOTIFY_API_KEY }} - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} - OS_DATA_KEY: ${{ secrets.OS_DATA_KEY }} - IMPORT_PAAS_INSTANCE: ${{ secrets.IMPORT_PAAS_INSTANCE }} - EXPORT_PAAS_INSTANCE: ${{ secrets.EXPORT_PAAS_INSTANCE }} - S3_CONFIG: ${{ secrets.S3_CONFIG }} - CSV_DOWNLOAD_PAAS_INSTANCE: ${{ secrets.CSV_DOWNLOAD_PAAS_INSTANCE }} - SENTRY_DSN: ${{ secrets.SENTRY_DSN }} - run: | - cf set-env $APP_NAME API_USER $API_USER - cf set-env $APP_NAME API_KEY $API_KEY - cf set-env $APP_NAME GOVUK_NOTIFY_API_KEY $GOVUK_NOTIFY_API_KEY - cf set-env $APP_NAME RAILS_MASTER_KEY $RAILS_MASTER_KEY - cf set-env $APP_NAME OS_DATA_KEY $OS_DATA_KEY - cf set-env $APP_NAME IMPORT_PAAS_INSTANCE $IMPORT_PAAS_INSTANCE - cf set-env $APP_NAME EXPORT_PAAS_INSTANCE "dluhc-core-review-export-bucket" - cf set-env $APP_NAME S3_CONFIG $S3_CONFIG - cf set-env $APP_NAME CSV_DOWNLOAD_PAAS_INSTANCE "dluhc-core-review-csv-bucket" - cf set-env $APP_NAME SENTRY_DSN $SENTRY_DSN - cf set-env $APP_NAME APP_HOST "https://dluhc-core-review-${{ github.event.pull_request.number }}.london.cloudapps.digital" - - - name: Bind postgres service - env: - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - SERVICE_NAME: dluhc-core-review-${{ github.event.pull_request.number }}-postgres - run: | - cf bind-service $APP_NAME $SERVICE_NAME --wait - - - name: Bind redis service - env: - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - SERVICE_NAME: dluhc-core-review-${{ github.event.pull_request.number }}-redis - run: | - cf bind-service $APP_NAME $SERVICE_NAME --wait - - - name: Bind logit drain service - env: - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - SERVICE_NAME: logit-ssl-drain - run: | - cf bind-service $APP_NAME $SERVICE_NAME --wait - - - name: Bind S3 buckets services - env: - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - run: | - cf bind-service $APP_NAME dluhc-core-review-csv-bucket --wait - cf bind-service $APP_NAME dluhc-core-review-export-bucket --wait - cf bind-service $APP_NAME dluhc-core-review-import-bucket --wait - - - name: Start review app - env: - APP_NAME: dluhc-core-review-${{ github.event.pull_request.number }} - run: | - cf restage $APP_NAME - - name: Comment on PR with URL uses: unsplash/comment-on-pr@v1.3.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - msg: "Created review app at https://dluhc-core-review-${{ github.event.pull_request.number }}.london.cloudapps.digital" + msg: "Created review app at https://review.submit-social-housing-data.levellingup.gov.uk/${{ github.event.pull_request.number }}" check_for_duplicate_msg: true duplicate_msg_pattern: Created review app at* diff --git a/.github/workflows/review_teardown_pipeline.yml b/.github/workflows/review_teardown_pipeline.yml index 3962243c4..4303e4063 100644 --- a/.github/workflows/review_teardown_pipeline.yml +++ b/.github/workflows/review_teardown_pipeline.yml @@ -1,92 +1,64 @@ name: Review app teardown pipeline +concurrency: + group: review-${{ github.event.pull_request.number }} + on: pull_request: types: - closed workflow_dispatch: -defaults: - run: - shell: bash +env: + app_repo_role: arn:aws:iam::815624722760:role/core-application-repo + aws_account_id: 837698168072 + aws_region: eu-west-2 + aws_role_prefix: core-dev + aws_task_prefix: core-review-${{ github.event.pull_request.number }} jobs: - app: - name: Teardown app + database: + name: Drop database runs-on: ubuntu-latest - environment: review + permissions: + id-token: write steps: - - name: Install Cloud Foundry CLI - run: | - wget --user-agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15" -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo apt-key add - - echo "deb https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list - sudo apt-get update - sudo apt-get install cf8-cli - - - name: Teardown app - env: - CF_USERNAME: ${{ secrets.CF_USERNAME }} - CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - CF_API_ENDPOINT: ${{ secrets.CF_API_ENDPOINT }} - CF_SPACE: dev - CF_ORG: ${{ secrets.CF_ORG }} - run: | - cf api $CF_API_ENDPOINT - cf auth - cf target -o $CF_ORG -s $CF_SPACE - cf delete dluhc-core-review-${{ github.event.pull_request.number }} -f -r - - postgres: - name: Teardown postgres - runs-on: ubuntu-latest - environment: review - needs: [app] + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@v3 + with: + aws-region: ${{ env.aws_region }} + role-to-assume: ${{ env.app_repo_role }} - steps: - - name: Install Cloud Foundry CLI - run: | - wget --user-agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15" -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo apt-key add - - echo "deb https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list - sudo apt-get update - sudo apt-get install cf8-cli + - name: Configure AWS credentials for review environment + uses: aws-actions/configure-aws-credentials@v3 + with: + aws-region: ${{ env.aws_region }} + role-to-assume: arn:aws:iam::${{ env.aws_account_id }}:role/${{ env.aws_role_prefix }}-deployment + role-chaining: true - - name: Teardown postgres + - name: Drop Database env: - CF_USERNAME: ${{ secrets.CF_USERNAME }} - CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - CF_API_ENDPOINT: ${{ secrets.CF_API_ENDPOINT }} - CF_SPACE: dev - CF_ORG: ${{ secrets.CF_ORG }} - run: | - cf api $CF_API_ENDPOINT - cf auth - cf target -o $CF_ORG -s $CF_SPACE - cf delete-service dluhc-core-review-${{ github.event.pull_request.number }}-postgres --wait -f - - redis: - name: Teardown redis - runs-on: ubuntu-latest - environment: review - needs: [app] - - steps: - - name: Install Cloud Foundry CLI + ad_hoc_task_definition: ${{ env.aws_task_prefix }}-ad-hoc + cluster: ${{ env.aws_task_prefix }}-app + service: ${{ env.aws_task_prefix }}-app run: | - wget --user-agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15" -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo apt-key add - - echo "deb https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list - sudo apt-get update - sudo apt-get install cf8-cli + network=$(aws ecs describe-services --cluster $cluster --services $service --query services[0].networkConfiguration) + overrides='{ "containerOverrides" : [{ "name" : "app", "command" : ["bundle", "exec", "rake", "db:drop"]}]}' + arn=$(aws ecs run-task --cluster $cluster --task-definition $ad_hoc_task_definition --network-configuration "$network" --overrides "$overrides" --group migrations --launch-type FARGATE --query tasks[0].taskArn) + echo "Waiting for db prepare task to complete" + temp=${arn##*/} + id=${temp%*\"} + aws ecs wait tasks-stopped --cluster $cluster --tasks $id + succeeded=$(aws ecs describe-tasks --cluster $cluster --tasks $id --query "tasks[0].stopCode == 'EssentialContainerExited' && to_string(tasks[0].containers[0].exitCode) == '0'") + if [ $succeeded == true ]; then exit 0; else exit 1; fi - - name: Teardown redis - env: - CF_USERNAME: ${{ secrets.CF_USERNAME }} - CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - CF_API_ENDPOINT: ${{ secrets.CF_API_ENDPOINT }} - CF_SPACE: dev - CF_ORG: ${{ secrets.CF_ORG }} - run: | - cf api $CF_API_ENDPOINT - cf auth - cf target -o $CF_ORG -s $CF_SPACE - cf delete-service dluhc-core-review-${{ github.event.pull_request.number }}-redis --wait -f + infra: + name: Teardown review app + needs: [database] + uses: communitiesuk/submit-social-housing-lettings-and-sales-data-infrastructure/.github/workflows/destroy_review_app_infra.yml@main + with: + key: ${{ github.event.pull_request.number }} + app_repo_role: arn:aws:iam::815624722760:role/core-application-repo + permissions: + id-token: write diff --git a/.github/workflows/staging_pipeline.yml b/.github/workflows/staging_pipeline.yml index ee9a45d40..883c5c816 100644 --- a/.github/workflows/staging_pipeline.yml +++ b/.github/workflows/staging_pipeline.yml @@ -238,7 +238,8 @@ jobs: uses: ./.github/workflows/aws_deploy.yml with: aws_account_id: 107155005276 - aws_resource_prefix: core-staging + aws_role_prefix: core-staging + aws_task_prefix: core-staging environment: staging permissions: id-token: write diff --git a/README.md b/README.md index cccb080f2..1e5bb10f8 100644 --- a/README.md +++ b/README.md @@ -13,10 +13,6 @@ Ruby on Rails app that handles the submission of lettings and sales of social ho * [API browser](https://communitiesuk.github.io/submit-social-housing-lettings-and-sales-data/api) (using this [OpenAPI specification](docs/api/v1.json)) * [Design history](https://core-design-history.herokuapp.com) -## System architecture - -![View of system architecture](docs/images/architecture.drawio.png) - ## User interface ![View of the logs list](docs/images/service.png) diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 2df2d21b1..8c5f45db5 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -132,7 +132,7 @@ module FiltersHelper end def user_lettings_path? - request.path == "/lettings-logs" + request.path == lettings_logs_path end def user_or_org_lettings_path? diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index adf109f15..13462eb2b 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -5,18 +5,18 @@ module NavigationItemsHelper if current_user.support? [ NavigationItem.new("Organisations", organisations_path, organisations_current?(path)), - NavigationItem.new("Users", "/users", users_current?(path)), + NavigationItem.new("Users", users_path, users_current?(path)), NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)), NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)), - NavigationItem.new("Schemes", "/schemes", supported_housing_schemes_current?(path)), + NavigationItem.new("Schemes", schemes_path, supported_housing_schemes_current?(path)), ].compact else [ NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)), NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)), - (NavigationItem.new("Schemes", "/schemes", supported_housing_schemes_current?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), + (NavigationItem.new("Schemes", schemes_path, non_support_supported_housing_schemes_current?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), 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("About your organisation", organisation_path(current_user.organisation.id), subnav_details_path?(path)), 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 @@ -25,11 +25,11 @@ module NavigationItemsHelper def secondary_items(path, current_organisation_id) [ - NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)), - 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)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), - 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("Lettings logs", lettings_logs_organisation_path(current_organisation_id), subnav_lettings_logs_path?(path)), + NavigationItem.new("Sales logs", sales_logs_organisation_path(current_organisation_id), subnav_sales_logs_path?(path)), + (NavigationItem.new("Schemes", schemes_organisation_path(current_organisation_id), subnav_supported_housing_schemes_path?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), + NavigationItem.new("Users", users_organisation_path(current_organisation_id), subnav_users_path?(path)), + NavigationItem.new("About this organisation", organisation_path(current_organisation_id), subnav_details_path?(path)), 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 @@ -37,31 +37,35 @@ module NavigationItemsHelper def scheme_items(path, current_scheme_id, title) [ - NavigationItem.new("Scheme", "/schemes/#{current_scheme_id}", !path.include?("locations")), - NavigationItem.new(title, "/schemes/#{current_scheme_id}/locations", path.include?("locations")), + NavigationItem.new("Scheme", scheme_path(current_scheme_id), !path.include?("locations")), + NavigationItem.new(title, scheme_locations_path(current_scheme_id), path.include?("locations")), ] end private def lettings_logs_current?(path) - path.starts_with?("/lettings-logs") + path.starts_with?(lettings_logs_path) end def sales_logs_current?(path) - path.starts_with?("/sales-logs") + path.starts_with?(sales_logs_path) end def users_current?(path) - path == "/users" || path.include?("/users/") + path == users_path || path.include?("/users/") end def supported_housing_schemes_current?(path) - path == "/schemes" || path.include?("/schemes/") + path == schemes_path || path.include?("/schemes/") + end + + def non_support_supported_housing_schemes_current?(path) + path.starts_with?(organisations_path) && path.include?("/schemes") || path.include?("/schemes/") end def organisations_current?(path) - path == "/organisations" || path.include?("/organisations/") + path == organisations_path || path.include?("/organisations/") end def subnav_supported_housing_schemes_path?(path) diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index 745350aa4..c78c4a080 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/app/models/form/lettings/questions/stock_owner.rb @@ -32,8 +32,8 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question Organisation.where(holds_own_stock: true).find_each do |org| if org.merge_date.present? answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period - elsif org.absorbed_organisations.merged_during_open_collection_period.exists? - answer_opts[org.id] = "#{org.name} (active as of #{org.created_at.to_fs(:govuk_date)})" + elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present? + answer_opts[org.id] = "#{org.name} (active as of #{org.available_from.to_fs(:govuk_date)})" else answer_opts[org.id] = org.name end diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index 529d14f66..a53f99dc3 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -33,8 +33,8 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question Organisation.where(holds_own_stock: true).find_each do |org| if org.merge_date.present? answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period - elsif org.absorbed_organisations.merged_during_open_collection_period.exists? - answer_opts[org.id] = "#{org.name} (active as of #{org.created_at.to_fs(:govuk_date)})" + elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present? + answer_opts[org.id] = "#{org.name} (active as of #{org.available_from.to_fs(:govuk_date)})" else answer_opts[org.id] = org.name end diff --git a/app/models/log.rb b/app/models/log.rb index 7870076da..ca10b94e5 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -50,7 +50,7 @@ class Log < ApplicationRecord scope :imported_2023_with_old_form_id, -> { imported.filter_by_year(2023).has_old_form_id } scope :imported_2023, -> { imported.filter_by_year(2023) } - attr_accessor :skip_update_status, :skip_update_uprn_confirmed + attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :skip_dpo_validation def process_uprn_change! if uprn.present? diff --git a/app/models/validations/date_validations.rb b/app/models/validations/date_validations.rb index 3757f1cfb..47c5d9638 100644 --- a/app/models/validations/date_validations.rb +++ b/app/models/validations/date_validations.rb @@ -53,9 +53,6 @@ module Validations::DateValidations if record["mrcdate"].present? && record["startdate"].to_date - record["mrcdate"].to_date > 3650 record.errors.add :startdate, I18n.t("validations.setup.startdate.ten_years_after_mrc_date") end - - location_during_startdate_validation(record) - scheme_during_startdate_validation(record) end private diff --git a/app/models/validations/setup_validations.rb b/app/models/validations/setup_validations.rb index 3fd760c30..744cc5ade 100644 --- a/app/models/validations/setup_validations.rb +++ b/app/models/validations/setup_validations.rb @@ -18,34 +18,6 @@ module Validations::SetupValidations validate_merged_organisations_start_date(record) end - def validate_irproduct_other(record) - if intermediate_product_rent_type?(record) && record.irproduct_other.blank? - record.errors.add :irproduct_other, I18n.t("validations.setup.intermediate_rent_product_name.blank") - end - end - - def validate_location(record) - location_during_startdate_validation(record) - - if record.location&.status == :incomplete - record.errors.add :location_id, :incomplete, message: I18n.t("validations.setup.location.incomplete") - record.errors.add :scheme_id, :incomplete, message: I18n.t("validations.setup.location.incomplete") - end - end - - def validate_scheme_has_confirmed_locations_validation(record) - return unless record.scheme - - unless record.scheme.locations.confirmed.any? - record.errors.add :scheme_id, :no_completed_locations, message: I18n.t("validations.scheme.no_completed_locations") - end - end - - def validate_scheme(record) - location_during_startdate_validation(record) - scheme_during_startdate_validation(record) - end - def validate_organisation(record) created_by, managing_organisation, owning_organisation = record.values_at("created_by", "managing_organisation", "owning_organisation") unless [created_by, managing_organisation, owning_organisation].any?(&:blank?) || ((created_by.organisation.absorbed_organisations + [created_by.organisation]) & [managing_organisation, owning_organisation]).present? @@ -82,7 +54,36 @@ module Validations::SetupValidations end end + def validate_irproduct_other(record) + if intermediate_product_rent_type?(record) && record.irproduct_other.blank? + record.errors.add :irproduct_other, I18n.t("validations.setup.intermediate_rent_product_name.blank") + end + end + + def validate_location(record) + location_during_startdate_validation(record) + + if record.location&.status == :incomplete + record.errors.add :location_id, :incomplete, message: I18n.t("validations.setup.location.incomplete") + record.errors.add :scheme_id, :incomplete, message: I18n.t("validations.setup.location.incomplete") + end + end + + def validate_scheme_has_confirmed_locations_validation(record) + return unless record.scheme + + unless record.scheme.locations.confirmed.any? + record.errors.add :scheme_id, :no_completed_locations, message: I18n.t("validations.scheme.no_completed_locations") + end + end + + def validate_scheme(record) + scheme_during_startdate_validation(record) + end + def validate_managing_organisation_data_sharing_agremeent_signed(record) + return if record.skip_dpo_validation + if record.managing_organisation_id_changed? && record.managing_organisation.present? && !record.managing_organisation.data_protection_confirmed? record.errors.add :managing_organisation_id, I18n.t("validations.setup.managing_organisation.data_sharing_agreement_not_signed") end @@ -150,7 +151,7 @@ private elsif absorbing_owning_organisation_inactive?(record) record.errors.add :startdate, I18n.t("validations.setup.startdate.invalid_absorbing_organisations_start_date.same_organisation", owning_organisation: record.owning_organisation.name, - owning_organisation_available_from: record.owning_organisation.created_at.to_formatted_s(:govuk_date)) + owning_organisation_available_from: record.owning_organisation.available_from.to_formatted_s(:govuk_date)) end end diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb index c171231d9..3fd621972 100644 --- a/app/models/validations/shared_validations.rb +++ b/app/models/validations/shared_validations.rb @@ -121,6 +121,8 @@ module Validations::SharedValidations end def validate_owning_organisation_data_sharing_agremeent_signed(record) + return if record.skip_dpo_validation + if record.owning_organisation_id_changed? && record.owning_organisation.present? && !record.owning_organisation.data_protection_confirmed? record.errors.add :owning_organisation_id, I18n.t("validations.setup.owning_organisation.data_sharing_agreement_not_signed") end diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb index cdfb31cd9..6680b0b96 100644 --- a/app/services/feature_toggle.rb +++ b/app/services/feature_toggle.rb @@ -29,11 +29,11 @@ class FeatureToggle end def self.deduplication_flow_enabled? - !Rails.env.production? + true end def self.duplicate_summary_enabled? - !Rails.env.production? + true end def self.service_unavailable? diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index e0417704c..8dcef90e1 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -59,8 +59,9 @@ private end def merge_users(merging_organisation) - @merged_users[merging_organisation.name] = merging_organisation.users.map { |user| { name: user.name, email: user.email } } - merging_organisation.users.update_all(organisation_id: @absorbing_organisation.id) + users_to_merge = users_to_merge(merging_organisation) + @merged_users[merging_organisation.name] = users_to_merge.map { |user| { name: user.name, email: user.email } } + users_to_merge.update_all(organisation_id: @absorbing_organisation.id) end def merge_schemes_and_locations(merging_organisation) @@ -87,18 +88,21 @@ private lettings_log.location = location_to_set if location_to_set.present? end lettings_log.owning_organisation = @absorbing_organisation - lettings_log.save!(validate: false) + lettings_log.skip_dpo_validation = true + lettings_log.save! end merging_organisation.managed_lettings_logs.after_date(@merge_date.to_time).each do |lettings_log| lettings_log.managing_organisation = @absorbing_organisation - lettings_log.save!(validate: false) + lettings_log.skip_dpo_validation = true + lettings_log.save! end end def merge_sales_logs(merging_organisation) merging_organisation.sales_logs.after_date(@merge_date.to_time).each do |sales_log| sales_log.owning_organisation = @absorbing_organisation - sales_log.save!(validate: false) + sales_log.skip_dpo_validation = true + sales_log.save! end end @@ -132,4 +136,25 @@ private def child_relationship_exists_on_absorbing_organisation?(child_organisation_relationship) child_organisation_relationship.child_organisation == @absorbing_organisation || @merging_organisations.include?(child_organisation_relationship.child_organisation) || @absorbing_organisation.child_organisation_relationships.where(child_organisation: child_organisation_relationship.child_organisation).exists? end + + def users_to_merge(merging_organisation) + return merging_organisation.users if merging_organisation.data_protection_confirmation.blank? + if merging_organisation.data_protection_confirmation.data_protection_officer.email.exclude?("@") + return merging_organisation.users.where.not(id: merging_organisation.data_protection_confirmation.data_protection_officer.id) + end + + new_dpo = User.new( + name: merging_organisation.data_protection_confirmation.data_protection_officer.name, + organisation: merging_organisation, + is_dpo: true, + encrypted_password: SecureRandom.hex(10), + email: SecureRandom.uuid, + confirmed_at: Time.zone.now, + active: false, + ) + new_dpo.save!(validate: false) + merging_organisation.data_protection_confirmation.update!(data_protection_officer: new_dpo) + + merging_organisation.users.where.not(id: new_dpo.id) + end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 3b051a8bc..30ec7d098 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -87,7 +87,7 @@ <%= govuk_header( classes: govuk_header_classes(current_user), - service_url: current_user.nil? ? "/" : "/logs", + service_url: current_user.nil? ? root_path : logs_path, navigation_classes: "govuk-header__navigation--end", ) do |component| component.product_name(name: t("service_name")) diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb index 75a4d799c..72d1d2f43 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -3,7 +3,7 @@ <% content_for :title, title %> -<%= render partial: "organisations/headings", locals: request.path == "/organisations" ? { main: "Organisations", sub: nil } : { main: @organisation.name, sub: "Organisations" } %> +<%= render partial: "organisations/headings", locals: request.path == organisations_path ? { main: "Organisations", sub: nil } : { main: @organisation.name, sub: "Organisations" } %> <% if current_user.support? %> <%= govuk_button_link_to "Create a new organisation", new_organisation_path, html: { method: :get } %> diff --git a/config.ru b/config.ru index 4a3c09a68..9b96fe939 100644 --- a/config.ru +++ b/config.ru @@ -2,5 +2,7 @@ require_relative "config/environment" -run Rails.application -Rails.application.load_server +map DataCollector::Application.config.relative_url_root || "/" do + run Rails.application + Rails.application.load_server +end diff --git a/config/application.rb b/config/application.rb index 4fd65c695..2cda05fea 100644 --- a/config/application.rb +++ b/config/application.rb @@ -50,5 +50,7 @@ module DataCollector helper_specs: false, controller_specs: false end + + config.relative_url_root = ENV["RAILS_RELATIVE_URL_ROOT"] end end diff --git a/config/environments/review.rb b/config/environments/review.rb index 27b4a5c50..ec4c67726 100644 --- a/config/environments/review.rb +++ b/config/environments/review.rb @@ -58,7 +58,7 @@ Rails.application.configure do config.action_mailer.perform_caching = false - config.action_mailer.default_url_options = { host: ENV["APP_HOST"] } + config.action_mailer.default_url_options = { host: ENV["APP_HOST"], script_name: Rails.application.config.relative_url_root } config.action_mailer.delivery_method = :smtp config.action_mailer.smtp_settings = { address: "smtp.gmail.com", diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 05eecc89f..da0a2a59e 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -24,3 +24,4 @@ Sentry.init do |config| end Sentry.set_tags("app_host": ENV["APP_HOST"]) +Sentry.set_tags("url_root": ENV["RAILS_RELATIVE_URL_ROOT"]) diff --git a/config/routes.rb b/config/routes.rb index 052570cb1..3c58e5a29 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -31,7 +31,7 @@ Rails.application.routes.draw do root to: "start#index" - get "/logs", to: redirect("/lettings-logs") + get "/logs", to: redirect("lettings-logs") get "/accessibility-statement", to: "content#accessibility_statement" get "/privacy-notice", to: "content#privacy_notice" get "/data-sharing-agreement", to: "content#data_sharing_agreement" diff --git a/docs/adr/adr-020-migration-to-aws.md b/docs/adr/adr-020-migration-to-aws.md new file mode 100644 index 000000000..f7a900750 --- /dev/null +++ b/docs/adr/adr-020-migration-to-aws.md @@ -0,0 +1,9 @@ +--- +parent: Architecture decisions +--- + +# 020: Migration to AWS + +GOV.UK PaaS is being decomissioned at the end of this year and by 23 December 2023 all services hosted on GOV.UK PaaS will need to have migrated to an alternate hosting platform. + +Like other DLUHC services, we are moving our service directly to DLUHC-owned AWS infrastructure. diff --git a/docs/images/architecture.drawio.png b/docs/images/architecture.drawio.png deleted file mode 100644 index 9f83da64f..000000000 Binary files a/docs/images/architecture.drawio.png and /dev/null differ diff --git a/docs/index.md b/docs/index.md index c8811ce76..b41df293c 100644 --- a/docs/index.md +++ b/docs/index.md @@ -12,8 +12,6 @@ Each data collection window runs from 1 April to 1 April the following year (plu ADD (Analytics & Data Directorate) statisticians are the other primary users of the service. The data collected is transferred to DLUHCs consolidated data store (CDS) via nightly XML exports to an S3 bucket. CDS ingests and transforms this data, ultimately storing it in a MS SQL database and exposing it to analysts and statisticians via Amazon Workspaces. -![Diagram of the CORE system architecture](https://raw.githubusercontent.com/communitiesuk/submit-social-housing-lettings-and-sales-data/main/docs/images/architecture.drawio.png) - ## Users External data providing organisations have 2 main user types: diff --git a/docs/infrastructure.md b/docs/infrastructure.md index a4b8b940d..6155dcf50 100644 --- a/docs/infrastructure.md +++ b/docs/infrastructure.md @@ -4,48 +4,51 @@ nav_order: 5 # Infrastructure -## Configuration - -On [GOV.UK PaaS](https://www.cloud.service.gov.uk/), service credentials are appended to the environment variable `VCAP_SERVICES` when services [are bound](https://docs.cloud.service.gov.uk/deploying_services/s3/#bind-an-aws-s3-bucket-to-your-app) to an application. -Such services include datastores and S3 buckets. - -Our application uses S3 and Redis clients and supports two different ways of parsing their configuration: -* Via the environment variable `VCAP_SERVICES` using the `PaasConfigurationService` class -* Via the environment variables `S3_CONFIG` and `REDIS_CONFIG` using the `EnvConfigurationService` class - -`S3_CONFIG` and `REDIS_CONFIG` are populated using a similar structure than `VCAP_SERVICES`: - -S3_CONFIG: -```json -[ - { - "instance_name": "bucket_1", - "credentials": { - "aws_access_key_id": "123", - "aws_secret_access_key": "456", - "aws_region": "eu-west-1", - "bucket_name": "my-bucket" - } - } -] -``` - -REDIS_CONFIG: -```json -[ - { - "instance_name": "redis_1", - "credentials": { - "uri": "redis_uri" - } - } -] -``` - -In order to switch from using [GOV.UK PaaS](https://www.cloud.service.gov.uk/) provided services to external ones, instances of `PaasConfigurationService` need to be replaced by `EnvConfigurationService`. -This assumes that `S3_CONFIG` or/and `REDIS_CONFIG` are available. - -Please check `full_import.rake` and `rack_attack.rb` for examples of how the configuration is used. +## Current infrastructure + +Currently, there are four environments with infrastructure: +- Meta +- Development (Review Apps) +- Staging +- Production + +### Meta +This holds the Terraform “backend” and the ECR(s). +The Terraform “backend” consists of: +- S3 buckets - for storing Terraform state files. One for all non-production environments (including the meta environment itself), and another just for production. +- DynamoDB - for managing access and locking of all state files. + +The ECR(s) are: +- core - holds the application Docker images. +- db-migration - holds the Docker images curated to help migrate a DB from PaaS to AWS. +- s3-migration - holds the Docker images curated to help migrate S3 files from PaaS to AWS. +N.B. the migration ECRs may or may not be present, depending on if the Terraform has been configured to create migration infrastructure. The migration infrastructure is only used to help migrate the DB and S3 from PaaS to AWS, so is usually therefore only temporarily present. + +### Development / Staging / Production +These are the main environments holding the “application” infrastructure. +Though not exhaustive, each of them will generally contain the following key components: +- ECS Fargate cluster +- RDS (PostgreSQL database) +- ElastiCache (Redis data store) +- S3 buckets + - One for Bulk upload (sometimes also to referred to as the CSV bucket) + - One for CDS Export +- VPC +- Private subnets +- Public subnets +- Load Balancer +- Other misc. networking components (e.g. routing tables, gateways) +- CloudFront (Content Delivery Network) +- AWS Shield (DDoS protection, when enabled) +- WAF (Firewall) + +### Development / Review Apps +The development environment is used for Review Apps, and has some infrastructure that is created per-review-app and some that is shared by all apps. +In general, each review app has its own ECS Fargate cluster and Redis instances (plus any infrastructure to enable this), while the rest is shared. + +Where to find the Infrastructure? +The infrastructure is managed as code. +In the terraform folder of the codebase, there will be dedicated sub-folders for each of the aforementioned environments, where all the infrastructure for them is defined. ## Deployment (Pipeline — Recommended) @@ -61,224 +64,50 @@ To deploy you need to: 6. Post success message on Slack. 7. Tag tickets as ‘Released’ and move tickets to done on JIRA. -## Deployment (Manual) - -It is unlikely you will need to deploy manually as the GitHub actions method supersedes this one. This application is running on [GOV.UK PaaS](https://www.cloud.service.gov.uk/). To deploy you need to: - -1. Contact your organisation manager to get an account in `dluhc-core` organization and in the relevant spaces (staging/production). - -2. [Install the Cloud Foundry CLI](https://docs.cloudfoundry.org/cf-cli/install-go-cli.html) - -3. Login: - - ```bash - cf login -a api.london.cloud.service.gov.uk -u - ``` - -4. Set your deployment target (staging/production): - - ```bash - cf target -o dluhc-core -s - ``` - -5. Deploy: - - ```bash - cf push dluhc-core --strategy rolling - ``` - - This will use the [manifest file](https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data/blob/main/manifest.yml) - -Once the app is deployed: - -1. Get a Rails console: - - ```bash - cf ssh dluhc-core-staging -t -c "/tmp/lifecycle/launcher /home/vcap/app 'rails console' ''" - ``` - -2. Check logs: - - ```bash - cf logs dluhc-core-staging --recent - ``` - -### Troubleshooting deployments - -A failed Github deployment action will occasionally leave a Cloud Foundry deployment in a broken state. As a result all subsequent Github deployment actions will also fail with the message `Cannot update this process while a deployment is in flight`. - -```bash -cf cancel-deployment dluhc-core -``` - -You would then need to check the logs and fix the issue that caused the initial deployment to fail. ## CI/CD When a commit is made to `main` the following GitHub action jobs are triggered: 1. **Test**: RSpec runs our test suite -2. **Deploy**: If the Test stage passes, this job will deploy the app to our GOV.UK PaaS account using the Cloud Foundry CLI +2. **AWS Deploy**: If the Test stage passes, this job will deploy the app to AWS When a pull request is opened to `main` only the Test stage runs. ## Review apps -When a pull request is opened a review app will be spun up. The reviews apps connect to their own PostgreSQL and Redis instances with its own worker. +When a pull request is opened a review app will be spun up. Each review app has its own ECS Fargate cluster and Redis instances (plus any infrastructure to enable this), while the rest is shared. The review app github pipeline is independent of any test pipeline and therefore it will attempt to deploy regardless of the state the code is in. The usual seeding process takes place when the review app boots so there will be some minimal data that can be used to login with. 2FA has been disabled in the review apps for easier access. -The app boots in a new environment called `review`. As such this is the environment you should filter by for sentry errors or to change any config. +The app boots in a new environment called `development`. As such this is the environment you should filter by for sentry errors or to change any config. After a sucessful deployment a comment will be added to the pull request with the URL to the review app for your convenience. When a pull request is updated e.g. more code is added it will re-deploy the new code. Once a pull request has been closed the review app infrastructure will be tore down to save on any costs. Should you wish to re-open a closed pull request the review app will be spun up again. -### How to fix review app deployment failures - -One reason a review app deployment might fail is that it is attempting to run migrations which conflict with data in the database. For example you might have introduced a unique constraint, but the database associated with the review app has duplicate data in it that would violate this constraint, and so the migration cannot be run. There are two main ways to remedy this: - -**Method 1 - Edit database via console** -1. Log in to Cloud Foundry - ```bash - cf login -a api.london.cloud.service.gov.uk -u - ``` - * Your username should be the email address you signed up to GOVUK PaaS with. - * Choose the dev environment whilst logging in. -2. If you were already logged in then Cloud Foundry, then instead just target the dev environment - ```bash - cf target -o dluhc-core -s dev - ``` -3. Find the name of your app - ```bash - cf apps - ``` - * The app name will be in this format: `dluhc-core-review-`. -4. Open a console for your app - ```bash - cf ssh -t -c "/tmp/lifecycle/launcher /home/vcap/app 'rails console' ''" - ``` -5. Edit the database as appropriate, e.g. delete dodgy data and recreate correctly - -**Method 2 - Nuke and restart** - -1. Find the name of your app - ```bash - cf apps - ``` - * The app name will be in this format: `dluhc-core-review-`. -2. Delete the app - ```bash - cf delete - ``` -3. Find the name of the matching Postgres service - ```bash - cf services - ``` - * The service name will be in this format: `dluhc-core-review--postgres`. -4. Delete the service - ```bash - cf delete-service - ``` - * Use `cf services` or `cf service ` to check the operation status. - * There's no need to delete the Redis service. -5. Re-run the whole review app pipeline in GitHub - * If it fails it's likely that the deletion from the previous step hadn't completed yet. So just wait a few minutes and re-run the pipeline again. - -## Setting up Infrastructure for a new environment - -### Staging - -1. Login: - - ```bash - cf login -a api.london.cloud.service.gov.uk -u - ``` - -2. Set your deployment target (staging): - - ```bash - cf target -o dluhc-core -s staging - ``` - -3. Create required Postgres, Redis and S3 bucket backing services (this will take ~15 mins to finish creating): - - ```bash - cf create-service postgres tiny-unencrypted-13 dluhc-core-staging-postgres - cf create-service redis micro-6.x dluhc-core-staging-redis - cf create-service aws-s3-bucket default dluhc-core-staging-csv-bucket - cf create-service aws-s3-bucket default dluhc-core-staging-import-bucket - cf create-service aws-s3-bucket default dluhc-core-staging-export-bucket - ``` - -4. Deploy manifest: - - ```bash - cf push dluhc-core-staging --strategy rolling - ``` - -5. Bind S3 services to app: - - ```bash - cf bind-service dluhc-core-staging dluhc-core-staging-csv-bucket - cf bind-service dluhc-core-staging dluhc-core-staging-redis - cf bind-service dluhc-core-staging dluhc-core-staging-import-bucket -c '{"permissions": "read-write"}' - cf bind-service dluhc-core-staging dluhc-core-staging-export-bucket -c '{"permissions": "read-write"}' - ``` - -6. Create a service keys for accessing the S3 bucket from outside Gov PaaS: - - ```bash - cf create-service-key dluhc-core-staging-csv-bucket csv-bucket -c '{"allow_external_access": true}' - cf create-service-key dluhc-core-staging-import-bucket data-import -c '{"allow_external_access": true}' - cf create-service-key dluhc-core-staging-export-bucket data-export -c '{"allow_external_access": true, "permissions": "read-only"}' - ``` - -### Production - -1. Login: - - ```bash - cf login -a api.london.cloud.service.gov.uk -u - ``` - -2. Set your deployment target (production): - - ```bash - cf target -o dluhc-core -s production - ``` - -3. Create required Postgres, Redis and S3 bucket backing services (this will take ~15 mins to finish creating): - - ```bash - cf create-service postgres small-ha-13 dluhc-core-production-postgres - cf create-service redis micro-ha-6.x dluhc-core-production-redis - cf create-service aws-s3-bucket default dluhc-core-production-csv-bucket - cf create-service aws-s3-bucket default dluhc-core-production-import-bucket - cf create-service aws-s3-bucket default dluhc-core-production-export-bucket - ``` - -4. Deploy manifest: - - ```bash - cf push dluhc-core-production --strategy rolling - ``` - -5. Bind S3 services to app: - - ```bash - cf bind-service dluhc-core-production dluhc-core-production-csv-bucket - cf bind-service dluhc-core-production dluhc-core-production-redis - cf bind-service dluhc-core-production dluhc-core-production-import-bucket -c '{"permissions": "read-write"}' - cf bind-service dluhc-core-production dluhc-core-production-export-bucket -c '{"permissions": "read-write"}' - ``` - -6. Create a service keys for accessing the S3 bucket from outside Gov PaaS: - - ```bash - cf create-service-key dluhc-core-production-csv-bucket dluhc-core-production-csv-bucket-service-key -c '{"allow_external_access": true}' - cf create-service-key dluhc-core-production-import-bucket data-import -c '{"allow_external_access": true}' - cf create-service-key dluhc-core-production-export-bucket data-export -c '{"allow_external_access": true, "permissions": "read-only"}' - ``` +### Review app deployment failures + +One reason a review app deployment might fail is that it is attempting to run migrations which conflict with data in the database. For example you might have introduced a unique constraint, but the database associated with the review app has duplicate data in it that would violate this constraint, and so the migration cannot be run. + +## Destroying/recreating infrastructure + +Things to watch out for when destroying/creating infra: +- All resources + - The lifecycle meta-argument prevent_destroy will stop you destroying things. Best to set this to false before trying to destroy! +- Database + - skip_final_snapshot being false will prevent you from destroying the db without creating a final snapshot. +- Load Balancer + - Sometimes when creating infra, you may see the error message: failure configuring LB attributes: InvalidConfigurationRequest: Access Denied for bucket: . Please check S3bucket permission during a terraform apply. To get around this you may have wait a few minutes and try applying again to ensure everything is fully updated (the error shouldn’t appear on the second attempt). It’s unclear what the exact cause is, but as this is related to infra that enables load balancer access logging, it is suspected there might be a delay with the S3 bucket permissions being realised or the load balancer recognising it can access the bucket. +- S3 + - Terraform won’t let you delete buckets that have objects in them. +- Secrets + - If you destroy secrets, they will actually be marked as ‘scheduled to delete’ which will take effect after a minimum of 7 days. You can’t recreate secrets with the same name during this period. If you want to destroy immediately, you need to do it from the command line (using your staging developer role, rather than your MHCLG-wide role used to apply Terraform) with this command: aws secretsmanager delete-secret --force-delete-without-recovery --secret-id . (Note that if a secret is marked as scheduled to delete, you can undo this in the console to make it an ‘active’ secret again.) + - You may need to manually re-enter secret values into Secrets Manager at some point. When you do, just paste the secret value as plain text (don’t enter a key name, or format it as JSON). +- ECS + - Sometimes task definitions don’t get deleted. You may need to manually delete them. + - After destroying the db, you’ll need to make sure the ad hoc ECS task which seeds the database gets run in order to set up the database correctly. +- SNS + - When creating an email subscription in an environment, Terraform will look up the email to use as the subscription endpoint from Secrets Manager. If you haven’t already created this (e.g. by running terraform apply -target="module.monitoring" -var="create_secrets_first=true") then this will lead to the subscription creation erroring, because it can’t retrieve the value of the secret (because it doesn’t exist yet). If this happens, remember you’ll need to go to Secrets Manager in the console and enter the desired email (as plaintext, no quotation marks or anything else required) as the value of the secret (which is most likely called MONITORING_EMAIL). Then run another apply with Terraform and this time it should succeed. diff --git a/docs/monitoring.md b/docs/monitoring.md index ac1a12f83..dd2234743 100644 --- a/docs/monitoring.md +++ b/docs/monitoring.md @@ -2,20 +2,48 @@ nav_order: 6 --- -# Monitoring - -We use self-hosted Prometheus and Grafana for monitoring infrastructure metrics. These are run in a dedicated Gov PaaS space called "monitoring" and are deployed as Docker images using GitHub action pipelines. The repository for these and more information is here: [dluhc-data-collection-monitoring](https://github.com/communitiesuk/dluhc-data-collection-monitoring). - -## Performance monitoring and alerting - -For application error and performance monitoring we use managed [Sentry](https://sentry.io/organizations/dluhc-core). You will need to be added to the DLUHC account to access this. It triggers slack notifications to the #team-data-collection-alerts channel for all application errors in staging and production and for any controller endpoints that have a P95 transaction duration > 250ms over a 24 hour period. - +# Logs and Debugging ## Logs +Logs can be found in two locations: +- AWS CloudWatch (for general application / infrastructure logging) +- Sentry (for application error logging) + +### CloudWatch +The CloudWatch service can be accessed from the AWS Console. You should authenticate onto the infrastructure environment whose logs you want to check. +From CloudWatch, navigate to the desired log group (e.g. for the app task running on ECS) and open the desired log stream, in order to read its log “events”. +Alternatively, you can also navigate to a specific AWS service / resource in question (e.g. ECS tasks), selecting the instance of interest (e.g. a specific ECS task), and finding the “logs” tab (or similar) to view the log “events”. + +### Sentry +To access Sentry, ensure you have been added to the DLUHC account. +Generally error logs in Sentry will also be present somewhere in the CloudWatch logs, but they will be easier to assess here (e.g. number of occurrences over a time period). The logs in Sentry are created by the application when it makes Rails.logger.error calls. + +## Debugging +### Application infrastructure +For debugging / investigating infrastructure issues you can use the AWS CloudWatch automatic dashboards. (e.g. is there a lack of physical space on the database, how long has the ECS had very high compute usage for etc.) +They can be found in the CloudWatch service on AWS console, by going to dashboards → automatic dashboards, and selecting the desired dashboard (e.g. Elastic Container Service). +Alternatively, you can also navigate to the AWS resource in question (e.g. RDS database), selecting the instance of interest, and selecting the “monitoring” / ”metrics” tab (or similar), as this can provide alternate useful information also. + +### Exec into a container +You can open a terminal directly on a running container / app, in order to run some commands that may help with debugging an issue. +To do this, you will need to “exec” into the container. + +#### Prerequisites +- AWS CLI +- AWS Session manager plugin Install the Session Manager plugin for the AWS CLI - AWS Systems Manager +- AWS access + +#### Accessing the rails console +1. Find the cluster name of the relevant cluster +2. Find the task arn of a relevant task +3. In a shell using suitable AWS credentials for the relevant account (e.g. the development, staging, or production account), run `aws ecs execute-command --cluster cluster-name --task task-arn --interactive --command "rails c"` + +N.B. You can run other commands on the container similarly. -For log persistence we use a managed ELK (Elasticsearch, Logstash, Kibana) stack provided by [Logit](https://logit.io/). You will need to be added to the DLUHC account to access this. Longs are retained for 14 days with a daily limit of 2GB. - -Logs are also available from Gov PaaS directly via CLI: - -```bash -cf logs --recent ``` +env=staging +taskArns=$(aws ecs list-tasks --cluster "core-$env-app" --query "taskArns[*]") +aws ecs describe-tasks --cluster "core-$env-app" --tasks "${taskArns[@]}" --query "tasks[*].{arn:taskArn, status:lastStatus, startedAt:startedAt, group:group, image:containers[0].image}" --output text +``` + +### Database +In order to investigate or look more closely at the database, you can exec into a container as above, and use the rails console to query the database. \ No newline at end of file diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index 5730d52ae..0c6ced3ba 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -3,9 +3,6 @@ require "rails_helper" RSpec.describe NavigationItemsHelper do let(:current_user) { create(:user, :data_coordinator) } - let(:users_path) { "/organisations/#{current_user.organisation.id}/users" } - let(:organisation_path) { "/organisations/#{current_user.organisation.id}" } - describe "#primary_items" do context "when the user is a data coordinator" do context "when the user's org does not own stock" do @@ -17,8 +14,8 @@ RSpec.describe NavigationItemsHelper do [ NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -40,8 +37,8 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -59,8 +56,8 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -77,8 +74,8 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", true), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -95,15 +92,15 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, true), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", 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) + expect(primary_items("/organisations/#{current_user.organisation.id}/users", current_user)).to eq(expected_navigation_items) end end @@ -113,15 +110,15 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, true), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", true), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] 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) + expect(primary_items("/organisations/#{current_user.organisation.id}/details", current_user)).to eq(expected_navigation_items) end end @@ -132,7 +129,7 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-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), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -150,7 +147,7 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-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), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -168,7 +165,7 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-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), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -196,8 +193,8 @@ RSpec.describe NavigationItemsHelper do [ NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] @@ -219,8 +216,8 @@ RSpec.describe NavigationItemsHelper do NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", false), NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] diff --git a/spec/models/form/lettings/questions/stock_owner_spec.rb b/spec/models/form/lettings/questions/stock_owner_spec.rb index 985a12432..74ca0d088 100644 --- a/spec/models/form/lettings/questions/stock_owner_spec.rb +++ b/spec/models/form/lettings/questions/stock_owner_spec.rb @@ -221,6 +221,33 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do expect(question.displayed_answer_options(log, user)).to eq(expected_opts) expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id) end + + context "and org has recently absorbed other orgs and does not have available from date" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:org) { create(:organisation, name: "User org") } + let(:options) do + { + "" => "Select an option", + org.id => "User org", + user.organisation.id => user.organisation.name, + log.owning_organisation.id => log.owning_organisation.name, + merged_organisation.id => "Merged org (inactive as of 2 February 2023)", + } + end + + before do + merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: org) + Timecop.freeze(Time.zone.local(2023, 11, 10)) + end + + after do + Timecop.return + end + + it "shows merged organisation as an option" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end end end diff --git a/spec/models/form/sales/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb index 79d8be710..ceb081459 100644 --- a/spec/models/form/sales/questions/owning_organisation_id_spec.rb +++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb @@ -224,7 +224,7 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do before do merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: organisation_1) - organisation_1.update!(created_at: Time.zone.local(2021, 2, 2)) + organisation_1.update!(created_at: Time.zone.local(2021, 3, 2), available_from: Time.zone.local(2021, 2, 2)) Timecop.freeze(Time.zone.local(2023, 11, 10)) end @@ -257,6 +257,32 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do expect(question.displayed_answer_options(log, user)).to eq(options) end end + + context "when an existing org has recently absorbed other orgs" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:options) do + { + "" => "Select an option", + organisation_1.id => "first test org", + organisation_2.id => "second test org", + merged_organisation.id => "Merged org (inactive as of 2 February 2023)", + } + end + + before do + merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: organisation_1) + organisation_1.update!(created_at: Time.zone.local(2021, 2, 2), available_from: nil) + Timecop.freeze(Time.zone.local(2023, 11, 10)) + end + + after do + Timecop.return + end + + it "does not show abailable from for absorbing organisation" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end end end diff --git a/spec/models/validations/date_validations_spec.rb b/spec/models/validations/date_validations_spec.rb index 977d47e4c..0fc706707 100644 --- a/spec/models/validations/date_validations_spec.rb +++ b/spec/models/validations/date_validations_spec.rb @@ -72,213 +72,6 @@ RSpec.describe Validations::DateValidations do date_validator.validate_startdate(record) expect(record.errors["startdate"]).to be_empty end - - context "with a deactivated location" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:) } - - before do - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), location:) - location.reload - end - - it "produces error when tenancy start date is during deactivated location period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.deactivated.startdate", postcode: location.postcode, date: "4 June 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.deactivated.location_id", postcode: location.postcode, date: "4 June 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.location.deactivated.location_id", postcode: location.postcode, date: "4 June 2022")) - end - - it "produces no error when tenancy start date is during an active location period" do - record.startdate = Time.zone.local(2022, 6, 1) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - end - - context "with a location that is reactivating soon" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:) } - - before do - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 8, 4), location:) - location.reload - end - - it "produces error when tenancy start date is during deactivated location period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.startdate", postcode: location.postcode, date: "4 August 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.location_id", postcode: location.postcode, date: "4 August 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.location_id", postcode: location.postcode, date: "4 August 2022")) - end - - it "produces no error when tenancy start date is during an active location period" do - record.startdate = Time.zone.local(2022, 9, 1) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - end - - context "with a location that has many reactivations soon" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:) } - - before do - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 1), reactivation_date: Time.zone.local(2022, 9, 4), location:) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 8, 4), location:) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 2), reactivation_date: Time.zone.local(2022, 8, 3), location:) - location.reload - end - - it "produces error when tenancy start date is during deactivated location period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.startdate", postcode: location.postcode, date: "4 September 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.location_id", postcode: location.postcode, date: "4 September 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.location_id", postcode: location.postcode, date: "4 September 2022")) - end - - it "produces no error when tenancy start date is during an active location period" do - record.startdate = Time.zone.local(2022, 10, 1) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - end - - context "with a location that is activating soon (has no deactivation periods)" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:, startdate: Time.zone.local(2022, 9, 15)) } - - it "produces no error" do - record.startdate = Time.zone.local(2022, 10, 15) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - - it "produces an error when the date is before available_from date" do - record.startdate = Time.zone.local(2022, 8, 15) - record.location = location - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.activating_soon.startdate", postcode: location.postcode, date: "15 September 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.activating_soon.location_id", postcode: location.postcode, date: "15 September 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.location.activating_soon.location_id", postcode: location.postcode, date: "15 September 2022")) - end - end - - context "with a deactivated scheme" do - let(:scheme) { create(:scheme) } - - before do - create(:location, scheme:) - create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), scheme:) - scheme.reload - end - - it "produces error when tenancy start date is during deactivated scheme period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.scheme = scheme - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.scheme.deactivated.startdate", name: scheme.service_name, date: "4 June 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.scheme.deactivated.scheme_id", name: scheme.service_name, date: "4 June 2022")) - end - - it "produces no error when tenancy start date is during an active scheme period" do - record.startdate = Time.zone.local(2022, 6, 1) - record.scheme = scheme - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - end - - context "with a scheme that is reactivating soon" do - let(:scheme) { create(:scheme) } - - before do - create(:location, scheme:) - create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 8, 4), scheme:) - scheme.reload - end - - it "produces error when tenancy start date is during deactivated scheme period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.scheme = scheme - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.scheme.reactivating_soon.startdate", name: scheme.service_name, date: "4 August 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.scheme.reactivating_soon.scheme_id", name: scheme.service_name, date: "4 August 2022")) - end - - it "produces no error when tenancy start date is during an active scheme period" do - record.startdate = Time.zone.local(2022, 9, 1) - record.scheme = scheme - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - end - - context "with a scheme that has many reactivations soon" do - let(:scheme) { create(:scheme) } - - before do - create(:location, scheme:) - create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 1), reactivation_date: Time.zone.local(2022, 9, 4), scheme:) - create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 8, 4), scheme:) - create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 2), reactivation_date: Time.zone.local(2022, 8, 3), scheme:) - scheme.reload - end - - it "produces error when tenancy start date is during deactivated scheme period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.scheme = scheme - date_validator.validate_startdate(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.scheme.reactivating_soon.startdate", name: scheme.service_name, date: "4 September 2022")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.setup.startdate.scheme.reactivating_soon.scheme_id", name: scheme.service_name, date: "4 September 2022")) - end - - it "produces no error when tenancy start date is during an active scheme period" do - record.startdate = Time.zone.local(2022, 10, 1) - record.scheme = scheme - date_validator.validate_startdate(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["scheme_id"]).to be_empty - end - end end describe "major repairs date" do diff --git a/spec/models/validations/setup_validations_spec.rb b/spec/models/validations/setup_validations_spec.rb index a226ee79d..2a05ecb77 100644 --- a/spec/models/validations/setup_validations_spec.rb +++ b/spec/models/validations/setup_validations_spec.rb @@ -136,8 +136,8 @@ RSpec.describe Validations::SetupValidations do Timecop.return end - let(:absorbing_organisation) { create(:organisation, created_at: Time.zone.local(2023, 2, 1, 4, 5, 6), available_from: Time.zone.local(2023, 2, 1, 4, 5, 6), name: "Absorbing org") } - let(:absorbing_organisation_2) { create(:organisation, created_at: Time.zone.local(2023, 2, 1), available_from: Time.zone.local(2023, 2, 1), name: "Absorbing org 2") } + let(:absorbing_organisation) { create(:organisation, created_at: Time.zone.local(2023, 1, 30, 4, 5, 6), available_from: Time.zone.local(2023, 2, 1, 4, 5, 6), name: "Absorbing org") } + let(:absorbing_organisation_2) { create(:organisation, created_at: Time.zone.local(2023, 1, 30), available_from: Time.zone.local(2023, 2, 1), name: "Absorbing org 2") } let(:merged_organisation) { create(:organisation, name: "Merged org") } let(:merged_organisation_2) { create(:organisation, name: "Merged org 2") } @@ -461,121 +461,6 @@ RSpec.describe Validations::SetupValidations do expect(record.errors["scheme_id"]).to be_empty end end - - context "with a deactivated location" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:) } - - before do - Timecop.freeze(Time.zone.local(2023, 11, 10)) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), location:) - Timecop.return - location.reload - end - - it "produces error when tenancy start date is during deactivated location period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.deactivated.startdate", postcode: location.postcode, date: "4 June 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.deactivated.location_id", postcode: location.postcode, date: "4 June 2022")) - end - - it "produces no error when tenancy start date is during an active location period" do - record.startdate = Time.zone.local(2022, 6, 1) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - end - end - - context "with a location that is reactivating soon" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:) } - - before do - Timecop.freeze(Time.zone.local(2023, 11, 10)) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 8, 4), location:) - Timecop.return - location.reload - end - - it "produces error when tenancy start date is during deactivated location period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.startdate", postcode: location.postcode, date: "4 August 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.location_id", postcode: location.postcode, date: "4 August 2022")) - end - - it "produces no error when tenancy start date is during an active location period" do - record.startdate = Time.zone.local(2022, 9, 1) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - end - end - - context "with a location that has many reactivations soon" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:) } - - before do - Timecop.freeze(Time.zone.local(2023, 11, 10)) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 1), reactivation_date: Time.zone.local(2022, 9, 4), location:) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 8, 4), location:) - create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 2), reactivation_date: Time.zone.local(2022, 8, 3), location:) - Timecop.return - location.reload - end - - it "produces error when tenancy start date is during deactivated location period" do - record.startdate = Time.zone.local(2022, 7, 5) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.startdate", postcode: location.postcode, date: "4 September 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.reactivating_soon.location_id", postcode: location.postcode, date: "4 September 2022")) - end - - it "produces no error when tenancy start date is during an active location period" do - record.startdate = Time.zone.local(2022, 10, 1) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - end - end - - context "with a location that is activating soon (has no deactivation periods)" do - let(:scheme) { create(:scheme) } - let(:location) { create(:location, scheme:, startdate: Time.zone.local(2022, 9, 15)) } - - it "produces no error" do - record.startdate = Time.zone.local(2022, 10, 15) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]).to be_empty - expect(record.errors["location_id"]).to be_empty - end - - it "produces an error when the date is before available_from date" do - record.startdate = Time.zone.local(2022, 8, 15) - record.location = location - setup_validator.validate_scheme(record) - expect(record.errors["startdate"]) - .to include(match I18n.t("validations.setup.startdate.location.activating_soon.startdate", postcode: location.postcode, date: "15 September 2022")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.setup.startdate.location.activating_soon.location_id", postcode: location.postcode, date: "15 September 2022")) - end - end end describe "#validate_location" do diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index ad15975ad..b6168d57a 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -160,11 +160,11 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.owned_schemes.count).to eq(1) expect(absorbing_organisation.owned_schemes.first.service_name).to eq(scheme.service_name) expect(absorbing_organisation.owned_schemes.first.old_id).to be_nil - expect(absorbing_organisation.owned_schemes.first.old_visible_id).to be_nil + expect(absorbing_organisation.owned_schemes.first.old_visible_id).to eq(nil) expect(absorbing_organisation.owned_schemes.first.locations.count).to eq(1) expect(absorbing_organisation.owned_schemes.first.locations.first.postcode).to eq(location.postcode) expect(absorbing_organisation.owned_schemes.first.locations.first.old_id).to be_nil - expect(absorbing_organisation.owned_schemes.first.locations.first.old_visible_id).to be_nil + expect(absorbing_organisation.owned_schemes.first.locations.first.old_visible_id).to eq(nil) expect(scheme.scheme_deactivation_periods.count).to eq(1) expect(scheme.scheme_deactivation_periods.first.deactivation_date.to_date).to eq(Time.zone.today) end @@ -179,7 +179,7 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(absorbing_organisation.owned_schemes.first) expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first) expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbing_organisation.owned_schemes.first) - expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil) + expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first) end it "rolls back if there's an error" do @@ -345,7 +345,7 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(absorbing_organisation.owned_schemes.first) expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first) expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbing_organisation.owned_schemes.first) - expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil) + expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first) end it "rolls back if there's an error" do @@ -404,20 +404,6 @@ RSpec.describe Merge::MergeOrganisationsService do create_list(:user, 5, organisation: merging_organisation_too) end - it "moves the users from merging organisations to absorbing organisation" do - expect(Rails.logger).to receive(:info).with("Merged users from fake org:") - expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})") - expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)") - expect(Rails.logger).to receive(:info).with("Merged users from second org:") - expect(Rails.logger).to receive(:info).with(/\tDanny Rojas/).exactly(6).times - expect(Rails.logger).to receive(:info).with("New schemes from fake org:") - expect(Rails.logger).to receive(:info).with("New schemes from second org:") - merge_organisations_service.call - - merging_organisation_user.reload - expect(merging_organisation_user.organisation).to eq(absorbing_organisation) - end - it "sets merge date and absorbing organisation on merged organisations" do merge_organisations_service.call @@ -451,6 +437,60 @@ RSpec.describe Merge::MergeOrganisationsService do expect(merging_organisation_user.organisation).to eq(merging_organisation) end + context "and merging users" do + it "moves the users from merging organisations to absorbing organisation" do + expect(Rails.logger).to receive(:info).with("Merged users from fake org:") + expect(Rails.logger).to receive(:info).with("\tDanny Rojas (#{merging_organisation.data_protection_officers.first.email})") + expect(Rails.logger).to receive(:info).with("\tfake name (fake@email.com)") + expect(Rails.logger).to receive(:info).with("Merged users from second org:") + expect(Rails.logger).to receive(:info).with(/\tDanny Rojas/).exactly(6).times + expect(Rails.logger).to receive(:info).with("New schemes from fake org:") + expect(Rails.logger).to receive(:info).with("New schemes from second org:") + merge_organisations_service.call + + merging_organisation_user.reload + expect(merging_organisation_user.organisation).to eq(absorbing_organisation) + end + + it "replaces dpo users with fake users if they have signed the data sharing agreement" do + merging_organisation_user.update!(is_dpo: true) + merging_organisation.data_protection_confirmation.update!(data_protection_officer: merging_organisation_user) + + merge_organisations_service.call + + merging_organisation_user.reload + merging_organisation.reload + expect(merging_organisation_user.organisation).to eq(absorbing_organisation) + expect(merging_organisation.users.count).to eq(1) + expect(merging_organisation.users.first.name).to eq(merging_organisation_user.name) + expect(merging_organisation.users.first.email).not_to eq(merging_organisation_user.email) + expect(merging_organisation.data_protection_confirmation.data_protection_officer).to eq(merging_organisation.users.first) + end + + it "does not move dpo users who have signed data sharing agreement if they have a fake email address" do + dpo = User.new( + name: merging_organisation.data_protection_confirmation.data_protection_officer.name, + organisation: merging_organisation, + is_dpo: true, + encrypted_password: SecureRandom.hex(10), + email: SecureRandom.uuid, + confirmed_at: Time.zone.now, + active: false, + ) + dpo.save!(validate: false) + merging_organisation.data_protection_confirmation.update!(data_protection_officer: dpo) + + merge_organisations_service.call + + dpo.reload + merging_organisation.reload + expect(dpo.organisation).to eq(merging_organisation) + expect(merging_organisation.users.count).to eq(1) + expect(merging_organisation.users.first).to eq(dpo) + expect(merging_organisation.data_protection_confirmation.data_protection_officer).to eq(dpo) + end + end + context "and merging organisation relationships" do let(:other_organisation) { create(:organisation) } let!(:merging_organisation_relationship) { create(:organisation_relationship, parent_organisation: merging_organisation) } @@ -688,7 +728,7 @@ RSpec.describe Merge::MergeOrganisationsService do expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first) expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first) expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first) - expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil) + expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first) end it "rolls back if there's an error" do @@ -849,12 +889,13 @@ RSpec.describe Merge::MergeOrganisationsService do new_absorbing_organisation.reload merging_organisation.reload + owned_lettings_log_no_location.reload expect(new_absorbing_organisation.owned_lettings_logs.count).to eq(2) expect(new_absorbing_organisation.managed_lettings_logs.count).to eq(1) expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first) expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first) expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(new_absorbing_organisation.owned_schemes.first) - expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil) + expect(new_absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(new_absorbing_organisation.owned_schemes.first.locations.first) end it "rolls back if there's an error" do