Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cs/sc 3360 support road network algorithm #34024

Closed
wants to merge 16 commits into from
Closed
2 changes: 2 additions & 0 deletions corehq/apps/geospatial/const.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@

GPS_POINT_CASE_PROPERTY = 'gps_point'

ALGO_AES = 'aes'

# Max number of cases per geohash
MAX_GEOHASH_DOC_COUNT = 10_000

Expand Down
55 changes: 52 additions & 3 deletions corehq/apps/geospatial/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@

class GeospatialConfigForm(forms.ModelForm):

RADIAL_ALGORITHM_OPTION = (GeoConfig.RADIAL_ALGORITHM, _('Radial Algorithm'))
ROAD_NETWORK_ALGORITHM_OPTION = (GeoConfig.ROAD_NETWORK_ALGORITHM, _('Road Network Algorithm'))

DISBURSEMENT_ALGORITHM_OPTIONS = [
RADIAL_ALGORITHM_OPTION,
ROAD_NETWORK_ALGORITHM_OPTION,
]

class Meta:
model = GeoConfig
fields = [
Expand All @@ -26,6 +34,7 @@ class Meta:
"min_cases_per_group",
"target_group_count",
"selected_disbursement_algorithm",
"plaintext_api_token",
]

user_location_property_name = forms.CharField(
Expand All @@ -39,7 +48,6 @@ class Meta:
required=True,
help_text=_("The name of the case property storing the geo-location data of your cases."),
)

selected_grouping_method = forms.ChoiceField(
label=_("Grouping method"),
# TODO: Add relevant documentation link to help_text when geospatial feature is GA'ed
Expand Down Expand Up @@ -73,9 +81,18 @@ class Meta:
# '<a href="{}" target="_blank">support documentation</a>.'),
# 'https://confluence.dimagi.com/pages/viewpage.action?pageId=164694245'
# ),
choices=GeoConfig.VALID_DISBURSEMENT_ALGORITHMS,
choices=DISBURSEMENT_ALGORITHM_OPTIONS,
required=True,
)
plaintext_api_token = forms.CharField(
label=_("Enter mapbox token"),
help_text=_(
"Enter your Mapbox API token here. Make sure your token has the correct scope configured"
" for use of the Mapbox Matrix API."
),
required=False,
widget=forms.PasswordInput(),
)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand Down Expand Up @@ -133,7 +150,25 @@ def __init__(self, *args, **kwargs):
),
crispy.Fieldset(
_('Algorithms'),
crispy.Field('selected_disbursement_algorithm'),
crispy.Field(
'selected_disbursement_algorithm',
data_bind='value: selectedAlgorithm',
),
crispy.Div(
crispy.Field('plaintext_api_token', data_bind="value: plaintext_api_token"),
data_bind="visible: captureApiToken"
),
crispy.Div(
StrictButton(
_('Test API Key'),
type='button',
css_id='test-connection-button',
css_class='btn btn-default',
data_bind="click: validateApiToken",
),
css_class=hqcrispy.CSS_ACTION_CLASS,
data_bind="visible: captureApiToken"
),
),
hqcrispy.FormActions(
StrictButton(
Expand All @@ -146,6 +181,10 @@ def __init__(self, *args, **kwargs):
)
)

@property
def domain(self):
return self.instance.domain

def clean(self):
cleaned_data = self.cleaned_data
grouping_method = cleaned_data['selected_grouping_method']
Expand All @@ -164,4 +203,14 @@ def clean(self):
if not cleaned_data['target_group_count']:
raise ValidationError(_("Value for target group count required"))

algorithm = cleaned_data.get('selected_disbursement_algorithm')
token = cleaned_data.get('plaintext_api_token')
if algorithm == GeoConfig.ROAD_NETWORK_ALGORITHM and not token:
raise ValidationError(_("Mapbox API token required"))

return cleaned_data

def save(self, commit=True):
if self.cleaned_data.get('plaintext_api_token'):
self.instance.plaintext_api_token = self.cleaned_data.get('plaintext_api_token')
return super().save(commit)
23 changes: 23 additions & 0 deletions corehq/apps/geospatial/migrations/0005_auto_20240123_1413.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 3.2.23 on 2024-01-23 14:13

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('geospatial', '0004_auto_20230920_0821'),
]

operations = [
migrations.AddField(
model_name='geoconfig',
name='_api_token',
field=models.CharField(blank=True, db_column='api_token', max_length=255, null=True),
),
migrations.AlterField(
model_name='geoconfig',
name='selected_disbursement_algorithm',
field=models.CharField(choices=[('radial_algorithm', 'Radial Algorithm'), ('road_network_algorithm', 'Road Network Algorithm')], default='radial_algorithm', max_length=50),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.23 on 2024-01-29 12:28

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('geospatial', '0005_auto_20240123_1413'),
]

operations = [
migrations.RenameField(
model_name='geoconfig',
old_name='_api_token',
new_name='api_token',
),
]
26 changes: 24 additions & 2 deletions corehq/apps/geospatial/models.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from django.db import models
from django.utils.translation import gettext as _

from corehq.apps.geospatial.const import GPS_POINT_CASE_PROPERTY
from corehq.apps.geospatial.const import GPS_POINT_CASE_PROPERTY, ALGO_AES
from corehq.apps.geospatial.routing_solvers import pulp
from corehq.motech.utils import b64_aes_encrypt, b64_aes_decrypt


class GeoPolygon(models.Model):
Expand Down Expand Up @@ -58,9 +59,10 @@ class GeoConfig(models.Model):

selected_disbursement_algorithm = models.CharField(
choices=VALID_DISBURSEMENT_ALGORITHMS,
default=ROAD_NETWORK_ALGORITHM,
default=RADIAL_ALGORITHM,
max_length=50
)
api_token = models.CharField(max_length=255, blank=True, null=True, db_column="api_token")

def save(self, *args, **kwargs):
super().save(*args, **kwargs)
Expand All @@ -81,3 +83,23 @@ def disbursement_solver(self):
return self.VALID_DISBURSEMENT_ALGORITHM_CLASSES[
self.selected_disbursement_algorithm
]

@property
def plaintext_api_token(self):
if self.api_token and self.api_token.startswith(f'${ALGO_AES}$'):
ciphertext = self.api_token.split('$', 2)[2]
return b64_aes_decrypt(ciphertext)
return self.api_token

@plaintext_api_token.setter
def plaintext_api_token(self, value):
try:
if value is None:
self.api_token = None
elif value and not value.startswith(f'${ALGO_AES}$'):
ciphertext = b64_aes_encrypt(value)
self.api_token = f'${ALGO_AES}${ciphertext}'
else:
raise Exception("Unexpected value set for plaintext api token")
except AttributeError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvement @Charl1996
Can you share what will raise the exception AttributeError?

Additionally, possible to keep the two messages different? If one gets an error, it wont be clear where it came from.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When raising exceptions its nice to be explicit what code would raise the exception. It's not clear to me here what will raise the exception AttributeError, though if only the necessary code is kept in the try block, its easier to locate.

raise Exception("Unexpected value set for plaintext api token")
2 changes: 1 addition & 1 deletion corehq/apps/geospatial/routing_solvers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class DisbursementAlgorithmSolverInterface:
def __init__(self, request_json):
self.request_json = request_json

def solve(self):
def solve(self, *args, **kwargs):
"""
The solve method implementation should return either the results if it's readily
available or a poll_id which can be used to poll for the results.
Expand Down
2 changes: 1 addition & 1 deletion corehq/apps/geospatial/routing_solvers/mapbox_optimize.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,5 @@ def routing_status(poll_id):

class MapboxVRPSolver(DisbursementAlgorithmSolverInterface):

def solve(self):
def solve(self, *args, **kwargs):
return submit_routing_request(self.request_json), None
11 changes: 5 additions & 6 deletions corehq/apps/geospatial/routing_solvers/pulp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import requests
import pulp

from django.conf import settings
from .mapbox_optimize import validate_routing_request
from corehq.apps.geospatial.routing_solvers.base import DisbursementAlgorithmSolverInterface

Expand All @@ -21,7 +20,7 @@ def __init__(self, request_json):
self.user_locations = request_json['users']
self.case_locations = request_json['cases']

def calculate_distance_matrix(self):
def calculate_distance_matrix(self, config):
distance_matrix = []
for user in self.user_locations:
user_point = (float(user['lat']), float(user['lon']))
Expand All @@ -34,8 +33,8 @@ def calculate_distance_matrix(self):

return distance_matrix

def solve(self, print_solution=False):
costs = self.calculate_distance_matrix()
def solve(self, config, print_solution=False):
costs = self.calculate_distance_matrix(config)
user_count = len(costs)
case_count = len(costs[0])

Expand Down Expand Up @@ -87,7 +86,7 @@ class RoadNetworkSolver(RadialDistanceSolver):
Solves user-case location assignment based on driving distance
"""

def calculate_distance_matrix(self):
def calculate_distance_matrix(self, config):
# Todo; support more than Mapbox limit by chunking
if len(self.user_locations + self.case_locations) > 25:
raise Exception("This is more than Mapbox matrix API limit (25)")
Expand All @@ -107,7 +106,7 @@ def calculate_distance_matrix(self):
'sources': sources,
'destinations': destinations,
'annotations': 'distance',
'access_token': settings.MAPBOX_ACCESS_TOKEN
'access_token': config.plaintext_api_token,
}

response = requests.get(url, params=params)
Expand Down
32 changes: 31 additions & 1 deletion corehq/apps/geospatial/static/geospatial/js/geo_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ hqDefine("geospatial/js/geo_config", [
"jquery",
"knockout",
"hqwebapp/js/initial_page_data",
"hqwebapp/js/bootstrap3/alert_user",
], function (
$,
ko,
initialPageData
initialPageData,
alertUser
) {
const ROAD_NETWORK_ALGORITHM = initialPageData.get('road_network_algorithm_slug');

var geoConfigViewModel = function (configData) {
'use strict';
var self = {};
Expand Down Expand Up @@ -46,6 +50,32 @@ hqDefine("geospatial/js/geo_config", [
return self.selectedGroupMethod() === minMaxGroupingName;
});

self.selectedAlgorithm = ko.observable();
self.plaintext_api_token = ko.observable(data.plaintext_api_token);

self.captureApiToken = ko.computed(function () {
return self.selectedAlgorithm() === ROAD_NETWORK_ALGORITHM;
});

self.validateApiToken = function () {
Charl1996 marked this conversation as resolved.
Show resolved Hide resolved
const url = "https://api.mapbox.com/directions-matrix/v1/mapbox/driving/-122.42,37.78;-122.45,37.91;-122.48,37.73";
const params = {access_token: self.plaintext_api_token()};

$.ajax({
method: 'get',
url: url,
data: params,
success: function () {
alertUser.alert_user(gettext("Token successfully verified!"), "success");
},
}).fail(function () {
alertUser.alert_user(
gettext("Invalid API token. Please verify that the token matches the one on your Mapbox account and has the correct scope configured."),
"danger"
);
});
};

return self;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ hqDefine("geospatial/js/geospatial_map", [
},
error: function () {
alertUser.alert_user(
gettext("Oops! Something went wrong! Please contact admin if the problem persists."), 'danger'
gettext("Oops! Something went wrong! Please check that your geospatial settings are configured correctly or contact admin if the problem persists."), 'danger'
);
self.setBusy(false);
},
Expand Down
1 change: 1 addition & 0 deletions corehq/apps/geospatial/templates/geospatial/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
{% initial_page_data 'gps_case_props_deprecated_state' gps_case_props_deprecated_state %}
{% initial_page_data 'target_grouping_name' target_grouping_name %}
{% initial_page_data 'min_max_grouping_name' min_max_grouping_name %}
{% initial_page_data 'road_network_algorithm_slug' road_network_algorithm_slug %}

<form id="geospatial-config-form" class="form-horizontal disable-on-submit ko-template" method="post">
{% crispy form %}
Expand Down
19 changes: 18 additions & 1 deletion corehq/apps/geospatial/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from django.test import TestCase

from ..const import GPS_POINT_CASE_PROPERTY
from ..const import GPS_POINT_CASE_PROPERTY, ALGO_AES
from ..models import GeoConfig
from ..utils import get_geo_case_property

Expand All @@ -21,6 +21,23 @@ def test_geo_config(self):
case_property = get_geo_case_property(self.domain)
self.assertEqual(case_property, GPS_POINT_CASE_PROPERTY)

def test_geo_config_api_token(self):
with self.get_geo_config() as config:
config.plaintext_api_token = '1234'
self.assertEqual(config.plaintext_api_token, '1234')
self.assertTrue(config.api_token.startswith(f"${ALGO_AES}$"))

config.plaintext_api_token = None
self.assertEqual(config.plaintext_api_token, None)
self.assertEqual(config.api_token, None)

def test_geo_config_api_token_cannot_be_empty(self):
with self.assertRaises(Exception) as context:
with self.get_geo_config() as config:
config.plaintext_api_token = ""

self.assertEqual(str(context.exception), "Unexpected value set for plaintext api token")

@contextmanager
def get_geo_config(self):
conf = GeoConfig(
Expand Down
3 changes: 2 additions & 1 deletion corehq/apps/geospatial/tests/test_pulp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from corehq.apps.geospatial.routing_solvers.pulp import (
RadialDistanceSolver
)
from corehq.apps.geospatial.models import GeoConfig


class TestRadialDistanceSolver(SimpleTestCase):
Expand All @@ -27,7 +28,7 @@ def test(self):
{"id": "Jackson", "lat": 40.55517003526139, "lon": -106.34189549259928},
],
},
).solve(), (
).solve(GeoConfig()), (
None,
{
'New York': ['New Hampshire', 'Newark', 'NY2'],
Expand Down
Loading
Loading