Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from setuptools import setup, find_packages

VERSION = '1.3.1'
VERSION = '1.4.0'

with open('requirements.txt') as f:
requirements = f.read().splitlines()
Expand Down
1 change: 1 addition & 0 deletions surge/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os

from surge.projects import Project
from surge.blueprints import Blueprint
from surge.tasks import Task
from surge.teams import Team
from surge.reports import Report
Expand Down
94 changes: 94 additions & 0 deletions surge/blueprints.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import re
from surge.projects import Project
from surge.errors import SurgeMissingAttributeError


class Blueprint(Project):
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to validate that launch/pause/cancel etc. in Project aren't called for a blueprint, either in this class or in Project (perhaps in a follow-up PR) https://github.com/CivJ/surge-python/blob/3134a80007faad6b524a28bdc2cf9885df0a1136/surge/projects.py#L200

Copy link
Contributor

Choose a reason for hiding this comment

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

This is of course validated on the server side though so maybe it doesn't matter

Copy link
Author

Choose a reason for hiding this comment

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

Hi Tim! Yea I was a little worried about this. But since a Blueprint project can either be "just the template" or a real project that was derived from a template, it seems like we'd want the full project API available in the second case. For the first case, I am hoping the server will protect us (which may not be a good assumption).

def __int__(self, **kwargs):
super().__init__(kwargs)

@classmethod
def _from_project(cls, project):
'''Converts a Project to a Blueprint. This is useful when we want to create a Project and then return
a Blueprint to the user.

Arguments:
project (Project): The project to convert to a Blueprint

Returns:
blueprint (Blueprint): A Blueprint derived from the given Project
'''
kwargs = project.__dict__
# A Project object will have already parsed created_at, so omit that and save current value to avoid errors.
created_at = kwargs.pop('created_at', None)
b = Blueprint(**project.__dict__)
b.created_at = created_at
return b

def required_data_fields(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably is outside the scope here but is this validated on the server side? I think we have fairly elaborate templating logic that could continue to evolve so this probably won't be exhaustive

'''
Returns all keys surrounded by {{}} from the fields_template string.

Returns:
matches (list): all the required data fields from fields_template
e.g. ['field1', 'field2']
'''
if not self.fields_template:
return []
pattern = r"{{(.*?)}}"
matches = re.findall(pattern, self.fields_template)
return matches

def create_new_batch(self, name):
'''
Create a new project from this blueprint. Once created, call create_tasks on the returned object
to assign tasks.

Arguments:
name (str): Name of the project

Returns:
blueprint (Blueprint): a created project from a blueprint project which can be assigned tasks and launched.
'''
if not name:
raise SurgeMissingAttributeError('name is required when creating a project from a template')

create_params = {'template_id': self.id, 'name': name}
project = Project.create(**create_params)
blueprint = Blueprint._from_project(project)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm - if you wanted to split the concept of a Blueprint project from a blueprint-derived project, could we split off the methods below this one, plus required_data_fields, into a separate class?

Copy link
Author

Choose a reason for hiding this comment

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

It's a good question! I lingered over this for a while. But the real sticking point for me is the fact that list_blueprints was already returning Project objects. I was trying to paint "within the lines" as I saw them, and to my eye we already decided that Projects types were going to represent both Blueprints (or templates) and the Projects derived from them. If we decided that list_blueprints should return a non-Project Blueprint or Template object that would make sense to me, but it would be backward-incompatible which I assumed we didn't want in this PR.

return blueprint

def create_tasks(self, tasks_data: list, launch=False):
'''Create tasks for this Blueprint project. Ensures that task_data contains fields referenced in
fields_template.

Arguments:
tasks_data (list): List of dicts. Each dict is key/value pairs for populating fields_template.

Returns:
tasks (list): list of Tasks objects
'''
Blueprint._validate_fields_data(self.required_data_fields(), tasks_data)
return super().create_tasks(tasks_data, launch)

@classmethod
def _validate_fields_data(cls, required_fields, tasks_data):
'''
Checks the keys in task_data exist in required_fields.
NOTE: this assumes tasks_data is a flat dict (no nested keys).

Arguments:
required_fields (list): list of required field names
tasks_data (list) list of dicts of task_data. All keys in these dicts should exist in required_fields

Returns:
None. Only raises if there are required fields missing.
'''
missing_keys = []
for data in tasks_data:
diff = set(required_fields) - set(data)
if diff:
missing_keys.append(diff)
if missing_keys:
msg = f'task_data is missing required keys: {missing_keys}'
raise SurgeMissingAttributeError(msg)
7 changes: 5 additions & 2 deletions surge/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,14 @@ def list_blueprints(cls):
Lists blueprint projects for your organization.

Returns:
projects (list): list of Project objects.
projects (list): list of Blueprint objects.
'''
endpoint = f"{PROJECTS_ENDPOINT}/blueprints"
response_json = cls.get(endpoint)
projects = [cls(**project_json) for project_json in response_json]

# Avoid circular dependency with deferred import.
from surge import Blueprint
projects = [Blueprint(**project_json) for project_json in response_json]
return projects

@classmethod
Expand Down
78 changes: 78 additions & 0 deletions tests/test_blueprint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
from datetime import datetime
from dateutil.tz import tzutc
import unittest

from surge.errors import SurgeMissingAttributeError
from surge.api_resource import APIResource
from surge.projects import Project
from surge.blueprints import Blueprint


class BlueprintTests(unittest.TestCase):
'''Use unittest in this class to assert on error messages.'''

def test_validate_fields_data_fail(self):
b = Blueprint(id=id,
name='name_blueprint',
created_at='2021-01-22T19:49:03.185Z')
b.fields_template = '<p><iframe src="{{video}}" width="560" height="315"></iframe><br></p>'
key = 'video'
with self.assertRaises(SurgeMissingAttributeError) as context:
Blueprint._validate_fields_data(b.required_data_fields(), [{'foo': 'task'}])
self.assertTrue(key in str(context.exception), f'error message {context.exception} should contain {key}')


def test_validate_fields_data():
# TODO: more test coverage
b = Blueprint(id=id,
name='name_blueprint',
created_at='2021-01-22T19:49:03.185Z')
b.fields_template = '<p><iframe src="{{video}}" width="560" height="315"></iframe><br></p>'

key = 'video'
Blueprint._validate_fields_data(b.required_data_fields(), [{key: 'task'}])


def test_required_data_fields():
'''Ensure a Blueprint object knows how to find required fields in fields_template.'''
assert_required_data_fields(None, [])
assert_required_data_fields('', [])
assert_required_data_fields('foo', [])
assert_required_data_fields('{{one}}', ['one'])
assert_required_data_fields('{{video}}{{audio}}{{spacial}}', ["video", "audio", "spacial"])
assert_required_data_fields('<p><iframe src="{{video}}" width="560" height="315"></iframe><br></p>', ['video'])


def test_init_basic():
project_id = "ABC1234"
name = "Hello World Blueprint"
blueprint = Blueprint(id=project_id,
name=name,
created_at='2021-01-22T19:49:03.185Z')

assert isinstance(blueprint, APIResource)
assert isinstance(blueprint, Project)
assert isinstance(blueprint, Blueprint)
assert blueprint.id == project_id
assert blueprint.name == name
assert blueprint.created_at == datetime(2021,
1,
22,
19,
49,
3,
185000,
tzinfo=tzutc())


def assert_required_data_fields(fields_template, expected_fields):
blueprint = Blueprint(id="id1",
name="name1",
created_at='2021-01-22T19:49:03.185Z')
blueprint.fields_template = fields_template
assert_values_in(blueprint.required_data_fields(), expected_fields)


def assert_values_in(ary, expected):
for value in expected:
assert value in ary
7 changes: 7 additions & 0 deletions tests/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def test_init_complete():
}],
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': False
}]
}

Expand Down Expand Up @@ -206,6 +207,7 @@ def test_convert_questions_to_objects():
}],
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': False
}, {
'id':
'c4b0d6a9-f735-40c1-9b42-0414945ef2db',
Expand Down Expand Up @@ -241,6 +243,7 @@ def test_convert_questions_to_objects():
}],
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': False
}, {
'id': '6123463e-349e-4450-80d2-6684a28755b3',
'text': 'Free response for {{url}}',
Expand All @@ -253,6 +256,7 @@ def test_convert_questions_to_objects():
'options_objects': [],
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': False
}, {
'id':
'c46e2714-9bf6-44a8-aac3-f01f9fec8ae2',
Expand Down Expand Up @@ -297,6 +301,7 @@ def test_convert_questions_to_objects():
}],
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': False
}, {
'id': '6123463e-349e-4450-80d2-6684a28755b4',
'text': 'Text area for {{url}}',
Expand All @@ -306,6 +311,7 @@ def test_convert_questions_to_objects():
'options_objects': [],
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': True
}, {
'id': '6123463e-349e-4450-80d2-6684a28755b5',
'text': 'Chatbot for {{url}}',
Expand All @@ -317,6 +323,7 @@ def test_convert_questions_to_objects():
'preexisting_annotations': None,
'hidden_by_item_option_id': None,
'shown_by_item_option_id': None,
'holistic': True
}]

project = Project(id="ABC1234", name="Hello World")
Expand Down