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

Revert "Add support for extra_properties in per task instructions (#5… #626

Merged
merged 1 commit into from
Jun 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 1 addition & 6 deletions client/app/admin/edit-project/edit-project.html
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,9 @@ <h1 class="section__aside-title">{{ 'In this area' | translate }}</h1>
ng-model="info.perTaskInstructions" rows="4">
</textarea>
<p><strong>{{ 'Tip' | translate }}:</strong> {{ 'You can use Markdown. (HTML is not allowed)' | translate }}</p>
<p ng-if="editProjectCtrl.project.taskCreationMode == 'GRID' ">
Put here anything that can be useful to users while taking a task. {x}, {y} and {z} will be replaced by the corresponding parameters for each task.
<p>Put here anything that can be useful to users while taking a task. {x}, {y} and {z} will be replaced by the corresponding parameters for each task.
{x}, {y} and {z} parameters can only be be used on tasks generated in the Tasking Manager and not on imported tasks.
For example: « This task involves loading extra data. Click [here](http://localhost:8111/import?new_layer=true&url=http://www.domain.com/data/{x}/{y}/{z}/routes_2009.osm) to load the data into JOSM ».</p>
<p ng-if="editProjectCtrl.project.taskCreationMode == 'ARBITRARY'">
Put here anything that can be useful to users while taking a task. If you have added extra properties within the GeoJSON of the task, they can be referenced by surrounding them in curly braces. For eg. if you have a property called "import_url" in your GeoJSON, you can reference it like:
<code>This task involves loading extra data. Click [here](http://localhost:8111/import?new_layer=true&url={import_url}) to load the data into JOSM</code>
</p>
<button class="button button--secondary button--small"
ng-click="editProjectCtrl.showPreviewPerTaskInstructions = !editProjectCtrl.showPreviewPerTaskInstructions">
{{ 'Preview' | translate }}
Expand Down
6 changes: 0 additions & 6 deletions devops/tm2-pg-migration/migrationscripts.sql
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@ Update hotnew.projects
where a.id = hotnew.projects.aoi_id


-- Set the task_creation_mode to 'arbitrary' when project's zoom was None in
-- TM2
Update hotnew.projects
set task_creation_mode = 1
from hotold.projects as p
where p.id = hotnew.projects.id and p.zoom is NULL;



Expand Down
11 changes: 0 additions & 11 deletions server/models/dtos/project_dto.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,6 @@ def is_known_mapping_type(value):
f'{MappingTypes.LAND_USE.name}, {MappingTypes.OTHER.name}')


def is_known_task_creation_mode(value):
""" Validates Task Creation Mode is known value """
try:
TaskCreationMode[value.upper()]
except KeyError:
raise ValidationError(f'Unknown taskCreationMode: {value} Valid values are {TaskCreationMode.GRID.name}, '
f'{TaskCreationMode.ARBITRARY.name}')


class DraftProjectDTO(Model):
""" Describes JSON model used for creating draft project """
cloneFromProjectId = IntType(serialized_name='cloneFromProjectId')
Expand Down Expand Up @@ -98,8 +89,6 @@ class ProjectDTO(Model):
priority_areas = BaseType(serialized_name='priorityAreas')
last_updated = DateTimeType(serialized_name='lastUpdated')
author = StringType()
task_creation_mode = StringType(required=True, serialized_name='taskCreationMode',
validators=[is_known_task_creation_mode], serialize_when_none=False)


class ProjectSearchDTO(Model):
Expand Down
4 changes: 1 addition & 3 deletions server/models/postgis/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from server.models.dtos.project_dto import ProjectDTO, DraftProjectDTO, ProjectSummary, PMDashboardDTO
from server.models.postgis.priority_area import PriorityArea, project_priority_areas
from server.models.postgis.project_info import ProjectInfo
from server.models.postgis.statuses import ProjectStatus, ProjectPriority, MappingLevel, TaskStatus, MappingTypes, TaskCreationMode
from server.models.postgis.statuses import ProjectStatus, ProjectPriority, MappingLevel, TaskStatus, MappingTypes
from server.models.postgis.tags import Tags
from server.models.postgis.task import Task
from server.models.postgis.user import User
Expand Down Expand Up @@ -52,7 +52,6 @@ class Project(db.Model):
license_id = db.Column(db.Integer, db.ForeignKey('licenses.id', name='fk_licenses'))
geometry = db.Column(Geometry('MULTIPOLYGON', srid=4326))
centroid = db.Column(Geometry('POINT', srid=4326))
task_creation_mode = db.Column(db.Integer, default=TaskCreationMode.GRID.value, nullable=False)

# Tags
mapping_types = db.Column(ARRAY(db.Integer), index=True)
Expand Down Expand Up @@ -318,7 +317,6 @@ def _get_project_and_base_dto(self):
base_dto.license_id = self.license_id
base_dto.last_updated = self.last_updated
base_dto.author = User().get_by_id(self.author_id).username
base_dto.task_creation_mode = TaskCreationMode(self.task_creation_mode).name

if self.private:
# If project is private it should have a list of allowed users
Expand Down
6 changes: 0 additions & 6 deletions server/models/postgis/statuses.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@ class ProjectPriority(Enum):
LOW = 3


class TaskCreationMode(Enum):
""" Enum to describe task creation mode """
GRID = 0
ARBITRARY = 1


class TaskStatus(Enum):
""" Enum describing available Task Statuses """
READY = 0
Expand Down
32 changes: 12 additions & 20 deletions server/models/postgis/task.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import bleach
import datetime
import geojson
import json
from enum import Enum
from geoalchemy2 import Geometry
from server import db
Expand Down Expand Up @@ -135,7 +134,6 @@ class Task(db.Model):
x = db.Column(db.Integer)
y = db.Column(db.Integer)
zoom = db.Column(db.Integer)
extra_properties = db.Column(db.Unicode)
# Tasks are not splittable if created from an arbitrary grid or were clipped to the edge of the AOI
splittable = db.Column(db.Boolean, default=True)
geometry = db.Column(Geometry('MULTIPOLYGON', srid=4326))
Expand Down Expand Up @@ -192,10 +190,6 @@ def from_geojson_feature(cls, task_id, task_feature):
except KeyError as e:
raise InvalidData(f'Task: Expected property not found: {str(e)}')

if 'extra_properties' in task_feature.properties:
task.extra_properties = json.dumps(
task_feature.properties['extra_properties'])

task.id = task_id
task_geojson = geojson.dumps(task_geometry)
task.geometry = ST_SetSRID(ST_GeomFromGeoJSON(task_geojson), 4326)
Expand Down Expand Up @@ -441,20 +435,18 @@ def format_per_task_instructions(self, instructions) -> str:
if not instructions:
return '' # No instructions so return empty string

properties = {}
# If there's no dynamic URL (e.g. url containing '{x}, {y} and {z}' pattern)
# - ALWAYS return instructions unaltered

if self.x:
properties['x'] = str(self.x)
if self.y:
properties['y'] = str(self.y)
if self.zoom:
properties['z'] = str(self.zoom)
if self.extra_properties:
properties.update(json.loads(self.extra_properties))
if not all(item in instructions for item in ['{x}','{y}','{z}']):
return instructions

try:
instructions = instructions.format(**properties)
except KeyError:
pass
return instructions
# If there is a dyamic URL only return instructions if task is splittable, since we have the X, Y, Z
if not self.splittable:
return 'No extra instructions available for this task'

instructions = instructions.replace('{x}', str(self.x))
instructions = instructions.replace('{y}', str(self.y))
instructions = instructions.replace('{z}', str(self.zoom))

return instructions
4 changes: 1 addition & 3 deletions server/services/grid/grid_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,11 @@ def tasks_from_aoi_features(feature_collection: str) -> geojson.FeatureCollectio
feature.geometry = shapely.geometry.mapping(feature.geometry)

# set default properties
# and put any already existing properties in `extra_properties`
feature.properties = {
'x': None,
'y': None,
'zoom': None,
'splittable': False,
'extra_properties': feature.properties
'splittable': False
}

tasks.append(feature)
Expand Down
2 changes: 0 additions & 2 deletions server/services/project_admin_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

from server.models.dtos.project_dto import DraftProjectDTO, ProjectDTO, ProjectCommentsDTO
from server.models.postgis.project import Project, Task, ProjectStatus
from server.models.postgis.statuses import TaskCreationMode
from server.models.postgis.task import TaskHistory
from server.models.postgis.utils import NotFound, InvalidData, InvalidGeoJson
from server.services.grid.grid_service import GridService
Expand Down Expand Up @@ -50,7 +49,6 @@ def create_draft_project(draft_project_dto: DraftProjectDTO) -> int:
# if arbitrary_tasks requested, create tasks from aoi otherwise use tasks in DTO
if draft_project_dto.has_arbitrary_tasks:
tasks = GridService.tasks_from_aoi_features(draft_project_dto.area_of_interest)
draf_project.task_creation_mode = TaskCreationMode.ARBITRARY.value
else:
tasks = draft_project_dto.tasks
ProjectAdminService._attach_tasks_to_project(draft_project, tasks)
Expand Down
87 changes: 82 additions & 5 deletions tests/server/helpers/test_files/tasks_from_aoi_features.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
1275016.6455451597,
2828781.542271093
],
[
1275087.6232993654,
2829087.2903841813
],
[
1275275.3796814454,
2829087.2903841813
],
[
1275275.3796814454,
2828783.0982895694
Expand All @@ -30,13 +38,82 @@
"splittable": false,
"x": null,
"y": null,
"zoom": null,
"extra_properties": {
"foo": "bar"
}
"zoom": null
},
"type": "Feature"
},
{
"geometry": {
"coordinates": [
[
[
[
1275581.1277945302,
2829306.621247852
],
[
1275827.894530152,
2829303.193932765
],
[
1275872.7042755112,
2829087.2903841813
],
[
1275581.1277945302,
2829087.2903841813
],
[
1275581.1277945302,
2829306.621247852
]
]
]
],
"type": "MultiPolygon"
},
"properties": {
"splittable": false,
"x": null,
"y": null,
"zoom": null
},
"type": "Feature"
},
{
"geometry": {
"coordinates": [
[
[
[
1275886.8759076186,
2829019.008869979
],
[
1275932.9954440442,
2828796.798620377
],
[
1275886.8759076186,
2828795.8377961316
],
[
1275886.8759076186,
2829019.008869979
]
]
]
],
"type": "MultiPolygon"
},
"properties": {
"splittable": false,
"x": null,
"y": null,
"zoom": null
},
"type": "Feature"
}
],
"type": "FeatureCollection"
}
}
36 changes: 0 additions & 36 deletions tests/server/helpers/test_files/test_arbitrary.json

This file was deleted.

29 changes: 23 additions & 6 deletions tests/server/unit/models/postgis/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,27 +77,44 @@ def test_per_task_instructions_with_underscores_formatted_correctly(self):
# Assert
self.assertEqual(instructions, 'Test Url is http://test.com/1_2_3')

def test_per_task_instructions_returns_instructions_when_no_dynamic_url_and_task_splittable(self):
def test_per_task_instructions_warning_returned_on_clipped_grids(self):
# Arrange
test_task = Task()
test_task.x = 1
test_task.y = 2
test_task.zoom = 3
test_task.splittable = True
test_task.splittable = False

# Act
instructions = test_task.format_per_task_instructions('Test Url is http://test.com/{x}/{y}/{z}')

# Assert
self.assertEqual(instructions, 'No extra instructions available for this task')

def test_per_task_instructions_returns_instructions_when_no_dynamic_url_and_task_not_splittable(self):
# Arrange
test_task = Task()
test_task.x = 1
test_task.y = 2
test_task.zoom = 3
test_task.splittable = False

# Act
instructions = test_task.format_per_task_instructions('Use map box')

# Assert
self.assertEqual(instructions, 'Use map box')

def test_per_task_instructions_returns_instructions_with_extra_properties(self):
def test_per_task_instructions_returns_instructions_when_no_dynamic_url_and_task_splittable(self):
# Arrange
test_task = Task()
test_task.extra_properties = '{"foo": "bar"}'
test_task.x = 1
test_task.y = 2
test_task.zoom = 3
test_task.splittable = True

# Act
instructions = test_task.format_per_task_instructions('Foo is replaced by {foo}')
instructions = test_task.format_per_task_instructions('Use map box')

# Assert
self.assertEqual(instructions, 'Foo is replaced by bar')
self.assertEqual(instructions, 'Use map box')
4 changes: 2 additions & 2 deletions tests/server/unit/services/grid/test_grid_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ def test_trim_grid_to_aoi_noclip(self):

def test_tasks_from_aoi_features(self):
# arrange
grid_json = get_canned_json('test_arbitrary.json')
grid_json = get_canned_json('test_grid.json')
grid_dto = GridDTO(grid_json)
expected = geojson.loads(json.dumps(get_canned_json('tasks_from_aoi_features.json')))

# act

result = GridService.tasks_from_aoi_features(grid_dto.area_of_interest)
# assert
self.assertEquals(str(expected), str(result))
Expand Down