geeksforsocialchange / PlaceCal

Bring your community together
https://placecal.org
GNU Affero General Public License v3.0
16 stars 6 forks source link

[Draft] [Bug]: Some postcodes are not valid on our site because they do not have a matching ward #1944

Closed r-ferrier closed 1 year ago

r-ferrier commented 1 year ago

Description

Some valid addresses cannot be added to the site.

Steps to reproduce

  1. Log into PlaceCal admin and create a new partner with the postcode EC1V 2NX

Why we think this is happening

We have saved a 2019 database as a csv containing lists of postcodes and the wards they are associated with. When someone uploads a postcode to our site, we use this database to retrieve the ward id for this postcode. We then cross reference this ward id at postcode.io to access further data about the ward. In some instances, this data is not being retrieved. My first guess at likely causes is:

Approaches we may try

We want to be careful not to break existing working definitions of wards. If your site is set up based around a locale and the boundaries change regularly this is probably not helpful - users may prefer to define which ward a partner belongs to. So we could:

Other things to consider

How does this operate in a location outside of the UK? How tightly coupled to this specific data set are our geographic definitions and can we loosen this?

Plan

First pass (getting everything working again)

Second pass (allowing selection of different wards)

r-ferrier commented 1 year ago

The error

partner params used to get back neighbourhood info

Parameters: {"authenticity_token"=>"pf2QNF7WSv1zlhdRwOnLLD8kWiQLBMq7OBrQqbZmevDcxKTCmS-FJ_Sd2WSylayJ9zBMEGksMK4JRv0vuXxpoA", "partner"=>{"name"=>"Test Partner broken address", "slug"=>"test-partner-broken-address", "summary"=>"", "description"=>"", "accessibility_info"=>"", "address_attributes"=>{"street_address"=>"Test address", "street_address2"=>"", "street_address3"=>"", "city"=>"", "postcode"=>"EC1V 2NX", "id"=>"983253"}, "url"=>"", "facebook_link"=>"", "twitter_handle"=>"", "public_name"=>"", "public_email"=>"", "public_phone"=>"", "partner_name"=>"", "partner_email"=>"", "partner_phone"=>"", "opening_times"=>"[]", "tag_ids"=>[""]}, "commit"=>"Save Partner", "subdomain"=>"admin", "id"=>"test-partner-broken-address"}

the bit that is breaking

app/controllers/admin/partners_controller.rb:72:in `update'
09:16:02 web.1        |   Neighbourhood Load (4.8ms)  SELECT "neighbourhoods".* FROM "neighbourhoods" WHERE "neighbourhoods"."unit" = $1 AND "neighbourhoods"."unit_code_key" = $2 AND "neighbourhoods"."unit_code_value" = $3 AND "neighbourhoods"."unit_name" = $4 LIMIT $5  [["unit", "ward"], ["unit_code_key", "WD19CD"], ["unit_code_value", "E05013699"], ["unit_name", "Bunhill"], ["LIMIT", 1]]
09:16:02 web.1        |   ↳ app/models/neighbourhood.rb:108:in `find_from_postcodesio_response'
09:16:02 web.1        |   Partner Exists? (1.6ms)  SELECT 1 AS one FROM "partners" WHERE LOWER("partners"."name") = LOWER($1) AND "partners"."id" != $2 LIMIT $3  [["name", "Test Partner broken address"], ["id", 246], ["LIMIT", 1]]
09:16:02 web.1        |   ↳ app/controllers/admin/partners_controller.rb:72:in `update'
09:16:02 web.1        |   CACHE Neighbourhood Load (0.0ms)  SELECT "neighbourhoods".* FROM "neighbourhoods" WHERE "neighbourhoods"."unit" = $1 AND "neighbourhoods"."unit_code_key" = $2 AND "neighbourhoods"."unit_code_value" = $3 AND "neighbourhoods"."unit_name" = $4 LIMIT $5  [["unit", "ward"], ["unit_code_key", "WD19CD"], ["unit_code_value", "E05013699"], ["unit_name", "Bunhill"], ["LIMIT", 1]]
09:16:02 web.1        |   ↳ app/models/neighbourhood.rb:108:in `find_from_postcodesio_response'
09:16:02 web.1        |    (4.3ms)  SELECT COUNT(*) FROM "tags" INNER JOIN "partner_tags" ON "tags"."id" = "partner_tags"."tag_id" WHERE "tags"."type" = $1 AND "partner_tags"."partner_id" = $2  [["type", "Category"], ["partner_id", 246]]
09:16:02 web.1        |   ↳ app/models/partner.rb:370:in `three_or_less_category_tags'
09:16:02 web.1        |   TRANSACTION (1.4ms)  ROLLBACK

What is happening to cause this

We create and update a partner. While updating the partner, in a before_action, we get the neighbourhoods from the partner and set them.

app/controllers/admin/partners_controller.rb line 114

def set_service_area_map_ids
      # maps neighbourhood ID to service_area ID
      @service_area_id_map = if @partner
                               @partner
                                 .service_areas.select(:id, :neighbourhood_id)
                                 .map { |sa| { sa.neighbourhood_id => sa.id } }
                                 .reduce({}, :merge)
                             else
                               {}
                             end
    end

We validate address updates in the neighbourhood with: (in app/models/address.rb, line 78, def geocode_with_ward)

 # There shouldn't be any wards that are outside our system, if there are we just fail.
    neighbourhood = Neighbourhood.find_from_postcodesio_response(res)
    if neighbourhood.nil?
      errors.add :postcode, 'has been found but could not be mapped to a neighbourhood at this time'
      return
    end

we use this to find a neighbourhood app/models/neighbourhood.rb line 107

 Neighbourhood.find_by(
        unit: 'ward',
        unit_code_key: 'WD19CD',
        unit_code_value: res['codes']['admin_ward'],
        unit_name: res['admin_ward']
      )
r-ferrier commented 1 year ago

First observation:

If we only match on the following we can assign an (out of date) ward, assuming the list of wards is the same:

Neighbourhood.find_by(
        unit: 'ward',
        unit_code_key: 'WD19CD',
        unit_name: res['admin_ward']
      )
r-ferrier commented 1 year ago

Also found a document which expressed the problem back in 2020!

https://github.com/geeksforsocialchange/PlaceCal/blob/main/doc/adr/0006-adding-extra-neighbourhood-info.md

ivan-kocienski-gfsc commented 1 year ago

Leaving this here as I don't know where else to put this.

Having run the migration and data load I now have both 2019 and 2023 neighbourhoods loaded into the table.

I then did a lookup for each postcode to see which dataset each postcode resolved to.

found 259 valid postcodes
  M144SL: 2019, 2023
  M155RG: 2019, 2023
  N16NE: 2019, 2023
  M155FS: 2019, 2023
  M112EX: 2019, 2023
  HX78LN: 2019, 2023
  YO104LS: 2019, 2023
  SK41QA: 2019
  SK44RL: 2019
  M145JT: 2019, 2023
  W85SA: 2019, 2023
  E95EF: 2019, 2023
  NR11QH: 2019, 2023
  M217QJ: 2019, 2023
  M34LZ: 2019, 2023
  M155FD: 2019, 2023
  M154HY: 2019, 2023
  M169QP: 2019
  M155NN: 2019, 2023
  M145AA: 2019, 2023
  M144HW: 2019, 2023
  M154EQ: 2019, 2023
  M167LA: 2019, 2023
  M167WD: 2019, 2023
  M154ZY: 2019, 2023
  M167HU: 2019, 2023
  M156AX: 2019, 2023
  M156GX: 2019, 2023
  M167DG: 2019, 2023
  M156FG: 2019, 2023
  M167JQ: 2019, 2023
  M155ZA: 2019, 2023
  M156NN: 2019, 2023
  M156DP: 2019, 2023
  M155RL: 2019, 2023
  M144SW: 2019, 2023
  M145NF: 2019, 2023
  M95QR: 2019, 2023
  M218BF: 2019, 2023
  M139PL: 2019, 2023
  M600AB: 2019, 2023
  M147BU: 2019, 2023
  M144LZ: 2019, 2023
  M147FZ: 2019, 2023
  M155RF: 2019, 2023
  M155BP: 2019, 2023
  M155LA: 2019, 2023
  M17HB: 2019, 2023
  M147DW: 2019, 2023
  M168GW: 2019, 2023
  M168EZ: 2019, 2023
  M41LW: 2019, 2023
  M169AB: 2019
  M34FN: 2019, 2023
  M155BJ: 2019, 2023
  M154PN: 2019, 2023
  M155DD: 2019, 2023
  M156SY: 2019, 2023
  M156ER: 2019, 2023
  M80PF: 2019, 2023
  M23HZ: 2019, 2023
  M11JW: 2019, 2023
  M156BR: 2019, 2023
  M144SS: 2019, 2023
  M169NW: 2019
  OL50LN: 2019
  OL50HR: 2019
  M167JL: 2019, 2023
  M144TH: 2019, 2023
  M144ET: 2019, 2023
  OL59DR: 2019
  OL59AH: 2019
  OL50EX: 2019
  OL50SE: 2019
  OL59RR: 2019
  OL68BH: 2019
  OL50ES: 2019
  OL50HF: 2019
  OL59JL: 2019
  OL59DP: 2019
  OL50BL: 2019
  OL59AY: 2019
  OL50SG: 2019
  OL50LP: 2019
  M154HW: 2019, 2023
  M156PA: 2019, 2023
  M167DA: 2019, 2023
  M154JY: 2019, 2023
  M156AD: 2019, 2023
  M167AQ: 2019, 2023
  M155EU: 2019, 2023
  M403RN: 2019, 2023
  M403TB: 2019, 2023
  M400BH: 2019, 2023
  M155AX: 2019, 2023
  OL50AP: 2019
  M403GL: 2019, 2023
  SK153LJ: 2019
  M350DY: 2019
  M403LN: 2019, 2023
  M400FJ: 2019, 2023
  M409JE: 2019, 2023
  M400DJ: 2019, 2023
  M400EW: 2019, 2023
  M97BL: 2019, 2023
  M409NB: 2019, 2023
  M403SL: 2019, 2023
  M350AE: 2019
  M403RQ: 2019, 2023
  OL50RD: 2019
  OL50HT: 2019
  M359BH: 2019
  M154EA: 2019, 2023
  M85UF: 2019, 2023
  M409PA: 2019, 2023
  M409QD: 2019, 2023
  M409PY: 2019, 2023
  M409DL: 2019, 2023
  SK176BD: 2019, 2023
  NG92AR: 2019, 2023
  BH231AJ: 2019, 2023
  BH234AB: 2019, 2023
  BH231PA: 2019, 2023
  BH64EN: 2019, 2023
  TD145ES: 2019, 2023
  BH231AS: 2019, 2023
  BH231BU: 2019, 2023
  BH233EH: 2019, 2023
  TD113AU: 2019, 2023
  TD135XY: 2019, 2023
  TD145PL: 2019, 2023
  TD151UH: 2019, 2023
  TD60SG: 2019, 2023
  TD60AL: 2019, 2023
  TD145HT: 2019, 2023
  TD145JW: 2019, 2023
  TD113AF: 2019, 2023
  M15NH: 2019, 2023
  M156LL: 2019, 2023
  M15QA: 2019, 2023
  M35DW: 2023
  M34FP: 2019, 2023
  M33EH: 2019, 2023
  M33WD: 2019, 2023
  M217GL: 2019, 2023
  SK178RH: 2019
  SK224BH: 2019, 2023
  CW15DU: 2019, 2023
  SK11EU: 2019
  TD145DE: 2019, 2023
  TD145ET: 2019, 2023
  TD145EU: 2019, 2023
  TD145SD: 2019, 2023
  TD145DH: 2019, 2023
  TD135YG: 2019, 2023
  OL11BE: 2019
  M139SU: 2019, 2023
  M144RE: 2019, 2023
  M155RH: 2019, 2023
  M156BG: 2019, 2023
  N19JP: 2023
  SE19JH: 2019, 2023
  W1W7LT: 2023
  EC1V2NX: 2023
  EX46NA: 
  N19DN: 2023
  SE11RB: 2019, 2023
  WC1N3XX: 2023
  W106DZ: 2019, 2023
  UB83PH: 2023
  W139LA: 2023
  SW197NB: 2023
  UB79JL: 2023
  CR43UD: 2023
  SM11EA: 2023
  SM51JJ: 
  UB82DE: 2023
  SW40JL: 2023
  SW192HR: 2023
  TW13AA: 2023
  KT12PT: 2023
  W1J7NF: 2023
  TD113NA: 2019, 2023
  W1D6AQ: 2023
  EC1R0DU: 2023
  TQ33BP: 2019, 2023
  N78TQ: 2023
  N193RQ: 2023
  WC2N6HH: 
  TD145PA: 2019, 2023
  TD145TN: 2019, 2023
  SE249HE: 2019, 2023
  E20FG: 2019, 2023
  WC1R4BH: 2023
  M225RQ: 2019, 2023
  M130GX: 2019, 2023
  M41LE: 2019, 2023
  OL99ES: 2019
  E82BT: 2019, 2023
  E179HQ: 2023
  SE156JL: 2019, 2023
  N102AE: 2023
  E96LL: 2019, 2023
  E28AS: 2019, 2023
  E27QL: 2019, 2023
  N51AR: 2023
  E29DA: 
  SK13UR: 2019
  SE85HD: 2023
  M124QE: 2019, 2023
  TQ33BN: 2019, 2023
  TQ25EL: 2019, 2023
  TQ46AE: 2019, 2023
  E95EN: 2019, 2023
  N167XB: 2019, 2023
  E82PB: 2019, 2023
  EC3R6DB: 2019, 2023
  M261RJ: 2019
  E26DG: 2019, 2023
  E83DF: 2019, 2023
  M94DT: 2019, 2023
  N160LH: 2019, 2023
  E83RR: 2019, 2023
  ST15BE: 2019
  WA143RB: 2019, 2023
  WN59AN: 2019
  SK11PP: 2019
  WN74ND: 2019
  WA141LE: 2019
  M113BS: 2019, 2023
  SK64EA: 2019
  SK44PB: 2019
  WA141EJ: 2019
  M168BP: 2019, 2023
  TD106AB: 2019, 2023
  TQ28EU: 2019, 2023
  TQ33UZ: 2019, 2023
  TQ33SU: 2019, 2023
  TQ33HF: 2019, 2023
  N160NY: 2019, 2023
  YO19QX: 2019, 2023
  TD113UA: 2019, 2023
  E96AS: 2019, 2023
  SE100BN: 2023
  TQ25QS: 2019, 2023
  TQ14BX: 2019, 2023
  M45JW: 2019, 2023
  TQ45JR: 2019, 2023
  TQ126NQ: 2019, 2023
  M114SG: 2019, 2023
  TQ31RN: 2019, 2023
  SE16TG: 
  TQ28JG: 2019, 2023
  TQ96PL: 2019, 2023
  TQ96BJ: 2019, 2023
  TQ28HY: 2019, 2023
  TQ46HA: 2019, 2023
  TQ28ET: 2019, 2023
  SE20QG: 2019, 2023