Adds field to select or type an administrative region in the georeferenciation by city name panel #3306
Conversation
- CitiesColumnsPoints -> CitiesColumnsTextPoints - CitiesTextPoints -> CitiesTextTextPoints
|
Frontend tests were OK 👍 (details) |
| @@ -96,7 +96,7 @@ | |||
| // Set model | |||
| var a = {}; | |||
| a[this.options.property] = val; | |||
| a['text'] = isText ? true : false; | |||
| a[this.options.text || 'text'] = isText ? true : false; | |||
There was a problem hiding this comment.
We were always passing "text" as "options.text", but we were not using that option.
alonsogarciapablo
changed the title
|
This PR touches both the backend and the frontend. @Kartones @javisantana @xavijam can you please take a look at it? Thanks! |
| @@ -11,6 +11,10 @@ | |||
| <div class="geocoding-pane-select kind"></div> | |||
| </li> | |||
| <li> | |||
| <label class="geocoding-pane-label">Admin.Region where city's located, if known</label> | |||
There was a problem hiding this comment.
A whitespace is missing here. "Admin. Region" should be the correct one
|
Frontend tests were OK 👍 (details) |
|
About frontend code 👍 |
|
I know there are not many specs, but I'm afraid we're propagating the illness by not adding tests for at least the new system. Either mocking/stubbing, or creating a table and populating with some dummy data to check it "looks ok". But I also don't know the internals of SQL functions like 👍 |
|
Frontend tests were OK 👍 (details) |
|
Thanks @Kartones! I'm adding some basic tests. |
alonsogarciapablo
force-pushed
the
3105-add-admin1-to-city-geocoder
branch
from
5624003
to
8fbcfe9
Compare
|
Frontend tests were OK 👍 (details) |
|
Frontend tests were OK 👍 (details) |
- Country column is present but region is empty - Country name is present but region is empty
|
Frontend tests were OK 👍 (details) |
|
Frontend tests were OK 👍 (details) |
Adds field to select or type an administrative region in the georeferenciation by city name panel
This PR basically adds a new field to select a column or type the name of an admin region when georeferencing by city name:
The frontend now sends two new params to the GeocodingsController:
region, which contains the name of the column that contains regions or the region typed by the user.region_text, which is true if region contains some text, or false if it contains the name of the column.We now have 4 SQL generators to geocode by name:
CitiesTextTextPointsCitiesTextColumnPointsCitiesColumnTextPointsCitiesColumnColumnPointsI'm not super happy with the names, so I'm open to suggestions :)
The rest of generators had to be accommodated to work with the new
regioncolumn that was added to the temporary table that is used during the georeferencing process.If you want to give it a try, you can use the city "Dover", which is present in many different states, like: "Minnesota", "Delaware", "New Jersey", etc.
I didn't find much tests related to geocoding and that's the reason why this PR contains no tests :(
Fixes #3105.