-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
) | ||
code_type.save() | ||
|
||
url = "https://raw.githubusercontent.com/alphagov/frontend/master/lib/data/authorities.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're putting slugs into MapIt so that they're available centrally and consistently to all apps, which means that MapIt should become the canonical source of LA slugs. Once we've got the slugs in MapIt we want to delete this file from Frontend and use the slugs from MapIt instead there, and also use the slugs from MapIt in local links manager and delete the import which uses this file. Instead of importing the slugs into MapIt from this file in Frontend we should store the data in this repo and load it from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We talked about this while implementing and thought this might be step 1: "Import from frontend". We'd then follow this up with steps 2: "make frontend use mapit slugs instead of authorities.json", and 3: "move authorities.json out of frontend and into mapit as a fixture, changing this importer in the process". Was a bit worried about creating another list (even if the intention is that it becomes the canonical list) and someone has to remember to update it as well if (when?) any changes are made.
Happy to skip straight to the step 3 (with a copy instead of a move) and deal with the duplication later when we're finally ready to remove authorities.json post step 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think skipping straight to step 3 sounds reasonable. I've added the data file and the importer uses it to get the slugs.
b989d96
to
8876420
Compare
17a51a7
to
5014a76
Compare
"north-down": { | ||
"name": "North Down Borough Council", | ||
"ons": "95W", | ||
"gss": null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not importing these slugs because they don't have GSS codes, so I wonder if we might as well delete them from the file?
Pro: smaller file, less to get confused about if you're not also looking at the importer
Con: we lose the history and the old slugs which would have existed as pages somewhere - perhaps if we do delete them we do it in a separate commit that only does the deletion so that there's some history.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to deleting these old local authorities from this file in a separate commit with some explanation.
There are a few points in the second commit message which aren't quite accurate:
|
{ | ||
"buckinghamshire": { | ||
"name": "Buckinghamshire County Council", | ||
"ons": "11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't using the SNACs or names from this file, so maybe we should remove them in a separate commit. We know we want to add more accurate names into MapIt for these areas so preserving this data structure has some value (instead of switching to a simpler structure like gss_code: slug
) but these names aren't up-to-date at the moment and they aren't the ones in our database so it's probably less confusing to remove them (or change all the name
values to null
?).
I don't have a working dev VM at the moment to try running any of this, but it looks to me like it should do the right thing... |
f644ba5
to
04b793a
Compare
defaults={'description': 'Slug for use by GOV.UK'} | ||
) | ||
|
||
file_path = os.path.join(dirname(__file__), '/../../data/authorities.json') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be os.path.dirname
, and to remove the leading /
on the path.
As part of the work to import local authority slugs, we need to be able to tell which Areas (that are Local Authorities) do not have a slug. We can reuse the existing 'mapit_UK_show_missing_codes' command by allowing it to accept arguments such as the `code_types` and `area_types`. We set the default values to be the previously hardcoded values so the script can still be called as it was before. However we do not want to check area_type 'EUR' for code_type 'govuk_slug', as we do not assign slugs to that area type. We add this exception to the code as a method call `is_code_type_required_for_area_type`.
MapIt will be the new canonical source for local authority slugs. We copy the authorities.json file from the Frontend app to MapIt. We also add the command to import the slugs from this file and assign them to the local authorities in MapIt. We need to create a new CodeType called 'govuk_slug’. We step through each entry in the authorities.json file and retrieve the Area that matches the GSS code. For each matched Area, we create a new Code of CodeType 'govuk_slug' with the relevant slug.
We are not importing these entries as they have no GSS codes, so we delete them to avoid confusion about their presence in the file. These represent the old NI local authorities which were reformed in 2015. The new NI local authorities are already present in the file.
We are not using SNAC codes or names from this file, so we remove them. We retain the data structure (instead of switching to simpler structure like `gss_code: slug`) because we may want to reintroduce the `name` field with updated names in future.
04b793a
to
4386c0a
Compare
Left some comments about minor code problems, which I promptly fixup and rebased away. Probably wants someone else to merge this now as I worked with @davidbasalla on the original implementation as well as these latest fixes. |
I ran the import command: Now, if I start up Mapit on the dev vm: 👍 |
As of alphagov/mapit#20 MapIt returns the code type 'govuk_slug' for local authorities. We use it to set the slug for each LocalAuthority during the import.
As of alphagov/mapit#20 MapIt returns the code type 'govuk_slug' for local authorities. We use it to set the slug for each LocalAuthority during the import.
In order to add slugs to local authorities, we add a command to download the JSON file from the Frontend app, match the contained entries to Areas by 'gss' code and a new code of type 'govuk_slug' with the relevant slug.
We also update an existing script to check for any local authority Areas that we do not import a slug for.