diff --git a/.github/workflows/review_pipeline.yml b/.github/workflows/review_pipeline.yml index 5656898bc..74880eabd 100644 --- a/.github/workflows/review_pipeline.yml +++ b/.github/workflows/review_pipeline.yml @@ -109,6 +109,7 @@ jobs: API_USER: ${{ secrets.API_USER }} API_KEY: ${{ secrets.API_KEY }} GOVUK_NOTIFY_API_KEY: ${{ secrets.GOVUK_NOTIFY_API_KEY }} + OS_DATA_KEY: ${{ secrets.OS_DATA_KEY }} RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} IMPORT_PAAS_INSTANCE: ${{ secrets.IMPORT_PAAS_INSTANCE }} EXPORT_PAAS_INSTANCE: ${{ secrets.EXPORT_PAAS_INSTANCE }} @@ -119,6 +120,7 @@ jobs: 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 OS_DATA_KEY $OS_DATA_KEY cf set-env $APP_NAME RAILS_MASTER_KEY $RAILS_MASTER_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" diff --git a/Gemfile b/Gemfile index 358e71070..f980a6582 100644 --- a/Gemfile +++ b/Gemfile @@ -102,3 +102,5 @@ end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] + +gem "httparty" diff --git a/Gemfile.lock b/Gemfile.lock index 7da2ef437..390ebe1b9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -195,6 +195,9 @@ GEM hashdiff (1.0.1) html-attributes-utils (0.9.2) activesupport (>= 6.1.4.4) + httparty (0.21.0) + mini_mime (>= 1.0.0) + multi_xml (>= 0.5.2) i18n (1.12.0) concurrent-ruby (~> 1.0) iniparse (1.5.0) @@ -223,6 +226,7 @@ GEM mini_mime (1.1.2) minitest (5.17.0) msgpack (1.6.0) + multi_xml (0.6.0) net-imap (0.3.4) date net-protocol @@ -464,6 +468,7 @@ DEPENDENCIES govuk-components govuk_design_system_formbuilder (= 3.1.2) govuk_markdown + httparty jsbundling-rails json-schema listen (~> 3.3) diff --git a/UPRN.md b/UPRN.md new file mode 100644 index 000000000..b7f940c6a --- /dev/null +++ b/UPRN.md @@ -0,0 +1,34 @@ +# Question: Which OS product can we use to do our address to UPRN lookup? + +- #UPRN: to check if provided UPRN exists + +API DOCS: https://osdatahub.os.uk/docs/places/technicalSpecification + +- #Find: freetext to search results containing UPRNs +https://api.os.uk/search/places/v1/find?query=Ordnance%20Survey%2C%20Adanac%20Drive%2C%20SO16&key= + + +# Question: Can we do API lookups? Are there any limitations on the product (API requests etc.) + +- Can't see any sort of limitation for our account: "As a member of the PSGA you are entitled to free Public Sector data transactions" +https://osdatahub.os.uk/dashboard + + + +# Additional QQ and notes: +- when searching UPRN from address there are usually multiple results and match accuracy isn't provided. Do we want users to pick the matching address or should we just pick the first match? +- could not find a free OS ruby client for the API that I would recommend using. Best option is this but seems to be relying on a private repository https://github.com/DEFRA/defra-ruby-address/ +- Auth strategy, there's 3 of them HTTP Header / Query String / OAuth2. + + +Flows that need to be handled: +- UPRN check: + - API is down / not responding + - UPRN is invalid + - UPRN is valid +- UPRN retrieval via address: + - API is down / not responding + - Address is valid but no UPRN found + - Address is valid and UPRN found + - Address is valid but multiple UPRNs found + - Address is not found and multiple UPRNs found diff --git a/app/models/form/lettings/pages/uprn.rb b/app/models/form/lettings/pages/uprn.rb new file mode 100644 index 000000000..9245fca0d --- /dev/null +++ b/app/models/form/lettings/pages/uprn.rb @@ -0,0 +1,16 @@ +class Form::Lettings::Pages::Uprn < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "uprn" + end + + def questions + @questions ||= [ + Form::Lettings::Questions::Uprn.new(nil, nil, self), + ] + end + + def routed_to?(_log, _current_user) + true + end +end diff --git a/app/models/form/lettings/pages/uprn_query.rb b/app/models/form/lettings/pages/uprn_query.rb new file mode 100644 index 000000000..f6887d0b7 --- /dev/null +++ b/app/models/form/lettings/pages/uprn_query.rb @@ -0,0 +1,16 @@ +class Form::Lettings::Pages::UprnQuery < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "uprn_query" + end + + def questions + @questions ||= [ + Form::Lettings::Questions::UprnQuery.new(nil, nil, self), + ] + end + + def routed_to?(_log, _current_user) + true + end +end diff --git a/app/models/form/lettings/pages/uprn_validation.rb b/app/models/form/lettings/pages/uprn_validation.rb new file mode 100644 index 000000000..2006dd935 --- /dev/null +++ b/app/models/form/lettings/pages/uprn_validation.rb @@ -0,0 +1,16 @@ +class Form::Lettings::Pages::UprnValidation < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "uprn_validation" + end + + def questions + @questions ||= [ + Form::Lettings::Questions::UprnValidation.new(nil, nil, self), + ] + end + + def routed_to?(_log, _current_user) + true + end +end diff --git a/app/models/form/lettings/questions/uprn.rb b/app/models/form/lettings/questions/uprn.rb new file mode 100644 index 000000000..8647d5573 --- /dev/null +++ b/app/models/form/lettings/questions/uprn.rb @@ -0,0 +1,40 @@ +class Form::Lettings::Questions::Uprn < ::Form::Question + def initialize(id, hsh, page) + super + @id = "uprn" + @check_answer_label = "UPRN" + @header = "Select your address from the list so that we can retrieve its UPRN" + @type = "radio" + @answer_options = answer_options + end + + def answer_options(log = nil, user = nil) + opts = { "" => "Select an option" } + + return opts unless ActiveRecord::Base.connected? + return opts unless user + return opts unless log + + q = log.uprn_query + + return [] unless q + + # TODO: handle non-200 responses + resp = HTTParty.get("https://api.os.uk/search/places/v1/find?query=#{q}&key=#{ENV['OS_DATA_KEY']}&minmatch=0.8&maxresults=50") + opts = {} + + resp["results"]&.each do |result| + uprn = result.dig("DPA", "UPRN") + address = result.dig("DPA", "ADDRESS") + opts[uprn] = { "value" => "#{uprn}: #{address}" } + end + + opts[""] = { "value" => "I can't find my address" } + + opts + end + + def displayed_answer_options(log, user) + answer_options(log, user) + end +end diff --git a/app/models/form/lettings/questions/uprn_query.rb b/app/models/form/lettings/questions/uprn_query.rb new file mode 100644 index 000000000..dfd683a91 --- /dev/null +++ b/app/models/form/lettings/questions/uprn_query.rb @@ -0,0 +1,9 @@ +class Form::Lettings::Questions::UprnQuery < ::Form::Question + def initialize(id, hsh, page) + super + @id = "uprn_query" + @check_answer_label = "Uprn Query" + @header = "UPRN query" + @type = "text" + end +end diff --git a/app/models/form/lettings/questions/uprn_validation.rb b/app/models/form/lettings/questions/uprn_validation.rb new file mode 100644 index 000000000..c4bb1c6cd --- /dev/null +++ b/app/models/form/lettings/questions/uprn_validation.rb @@ -0,0 +1,9 @@ +class Form::Lettings::Questions::UprnValidation < ::Form::Question + def initialize(id, hsh, page) + super + @id = "uprn" + @check_answer_label = "Uprn validation" + @header = "UPRN validation" + @type = "text" + end +end diff --git a/app/models/form/lettings/subsections/setup.rb b/app/models/form/lettings/subsections/setup.rb index e2dbb7f8b..28658f388 100644 --- a/app/models/form/lettings/subsections/setup.rb +++ b/app/models/form/lettings/subsections/setup.rb @@ -8,6 +8,9 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection def pages @pages ||= [ + Form::Lettings::Pages::UprnValidation.new(nil, nil, self), + Form::Lettings::Pages::UprnQuery.new(nil, nil, self), + Form::Lettings::Pages::Uprn.new(nil, nil, self), organisation_page, stock_owner_page, managing_organisation_page, diff --git a/app/models/validations/setup_validations.rb b/app/models/validations/setup_validations.rb index 93105f9ea..3db1dc6e9 100644 --- a/app/models/validations/setup_validations.rb +++ b/app/models/validations/setup_validations.rb @@ -25,6 +25,20 @@ module Validations::SetupValidations end end + def validate_uprn(record) + return if record.uprn.blank? + + resp = HTTParty.get("https://api.os.uk/search/places/v1/uprn?uprn=#{record.uprn}&key=#{ENV['OS_DATA_KEY']}") + + if resp.code == 200 + if resp["results"].blank? + record.errors.add :uprn, "invalid UPRN" + end + else + record.errors.add :uprn, "invalid UPRN" + end + end + private def intermediate_product_rent_type?(record) diff --git a/app/services/postcode_service.rb b/app/services/postcode_service.rb index 74c1f4895..1c759372c 100644 --- a/app/services/postcode_service.rb +++ b/app/services/postcode_service.rb @@ -1,3 +1,4 @@ +# NB class PostcodeService def initialize @pio = Postcodes::IO.new diff --git a/db/migrate/20230223101454_add_uprn_to_logs.rb b/db/migrate/20230223101454_add_uprn_to_logs.rb new file mode 100644 index 000000000..f9ce22371 --- /dev/null +++ b/db/migrate/20230223101454_add_uprn_to_logs.rb @@ -0,0 +1,13 @@ +class AddUprnToLogs < ActiveRecord::Migration[7.0] + def change + change_table :sales_logs, bulk: true do |t| + t.column :uprn, :integer + t.column :uprn_query, :integer + end + + change_table :lettings_logs, bulk: true do |t| + t.column :uprn, :integer + t.column :uprn_query, :integer + end + end +end diff --git a/db/schema.rb b/db/schema.rb index d1668b738..913f5ab18 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_02_15_112932) do +ActiveRecord::Schema[7.0].define(version: 2023_02_23_101454) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -278,6 +278,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_15_112932) do t.boolean "unresolved" t.bigint "updated_by_id" t.bigint "bulk_upload_id" + t.string "uprn" + t.string "uprn_query" t.index ["bulk_upload_id"], name: "index_lettings_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id" t.index ["location_id"], name: "index_lettings_logs_on_location_id" @@ -531,6 +533,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_15_112932) do t.integer "prevshared" t.integer "staircasesale" t.string "old_id" + t.integer "income2_value_check" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["old_id"], name: "index_sales_logs_on_old_id", unique: true