From 8072f77acf7cedcd093882019a9d42a7d2102bad Mon Sep 17 00:00:00 2001
From: Pradeep Kumar <pradeepsixer@gmail.com>
Date: Tue, 26 Dec 2017 00:30:43 -0700
Subject: [PATCH 1/5] API Changes and CO Timestamp support

1. Updated the API to support customizations by the API consumer
2. Added support for Created and Closed Timestamps for Change Requests.
---
 django_snow/helpers/snow_request_handler.py   |  55 +++++----
 ...002_changemgmt_add_createtime_closetime.py |  28 +++++
 django_snow/models.py                         |  20 +++-
 setup.py                                      |   2 +-
 testapp/tests.py                              | 113 ++++++++++++------
 5 files changed, 155 insertions(+), 63 deletions(-)
 create mode 100644 django_snow/migrations/0002_changemgmt_add_createtime_closetime.py

diff --git a/django_snow/helpers/snow_request_handler.py b/django_snow/helpers/snow_request_handler.py
index aea3951..8032ae6 100644
--- a/django_snow/helpers/snow_request_handler.py
+++ b/django_snow/helpers/snow_request_handler.py
@@ -2,6 +2,8 @@
 
 import pysnow
 from django.conf import settings
+from django.utils import timezone
+from requests.exceptions import HTTPError
 
 from ..models import ChangeRequest
 from .exceptions import ChangeRequestException
@@ -17,31 +19,30 @@ class ChangeRequestHandler:
 
     group_guid_dict = {}
 
-    # Service Now table name
-    CHANGE_REQUEST_TABLE_NAME = 'change_request'
-    USER_GROUP_TABLE_NAME = 'sys_user_group'
+    # Service Now table REST endpoints
+    CHANGE_REQUEST_TABLE_PATH = '/table/change_request'
+    USER_GROUP_TABLE_PATH = '/table/sys_user_group'
 
     def __init__(self):
         self._client = None
         self.snow_instance = settings.SNOW_INSTANCE
         self.snow_api_user = settings.SNOW_API_USER
         self.snow_api_pass = settings.SNOW_API_PASS
-        self.snow_assignment_group = settings.SNOW_ASSIGNMENT_GROUP
 
-    def create_change_request(self, title, description, co_type='Automated', assignment_group=None):
+    def create_change_request(self, payload):
+        """
+        Create a change request with the given payload.
+        """
         client = self._get_client()
-        assignment_group_guid = self._get_snow_group_guid(assignment_group or self.snow_assignment_group)
-        result = client.insert(
-            table=self.CHANGE_REQUEST_TABLE_NAME,
-            payload={
-                'short_description': title,
-                'state': ChangeRequest.TICKET_STATE_OPEN,
-                'description': description,
-                'type': co_type,
-                'assignment_group': assignment_group_guid
-            }
-        )
+        change_requests = client.resource(api_path=self.CHANGE_REQUEST_TABLE_PATH)
 
+        try:
+            result = change_requests.create(payload=payload)
+        except HTTPError as e:
+            logger.error('Could not create change request due to %s', e.response.text)
+            raise ChangeRequestException('Could not create change request due to %s.' % e.response.text)
+
+        # This piece of code is for legacy SNow instances. (probably Geneva and before it)
         if 'error' in result:
             logger.error('Could not create change request due to %s', result['error'])
             raise ChangeRequestException('Could not create change request due to %s' % result['error'])
@@ -61,6 +62,7 @@ def close_change_request(self, change_request):
         """Mark the change request as completed."""
 
         payload = {'state': ChangeRequest.TICKET_STATE_COMPLETE}
+        change_request.closed_time = timezone.now()
         self.update_change_request(change_request, payload)
 
     def close_change_request_with_error(self, change_request, payload):
@@ -76,6 +78,7 @@ def close_change_request_with_error(self, change_request, payload):
         :type payload: dict
         """
         payload['state'] = ChangeRequest.TICKET_STATE_COMPLETE_WITH_ERRORS
+        change_request.closed_time = timezone.now()
         self.update_change_request(change_request, payload)
 
     def update_change_request(self, change_request, payload):
@@ -94,14 +97,23 @@ def update_change_request(self, change_request, payload):
         client = self._get_client()
 
         # Get the record and update it
-        record = client.query(table=self.CHANGE_REQUEST_TABLE_NAME, query={'sys_id': change_request.sys_id.hex})
+        change_requests = client.resource(api_path=self.CHANGE_REQUEST_TABLE_PATH)
+
+        try:
+            result = change_requests.update(query={'sys_id': change_request.sys_id.hex}, payload=payload)
+        except HTTPError as e:
+            logger.error('Could not update change request due to %s', e.response.text)
+            raise ChangeRequestException('Could not update change request due to %s' % e.response.text)
 
-        result = record.update(payload=payload)
+        # This piece of code is for legacy SNow instances. (probably Geneva and before it)
         if 'error' in result:
             logger.error('Could not update change request due to %s', result['error'])
             raise ChangeRequestException('Could not update change request due to %s' % result['error'])
 
         change_request.state = result['state']
+        change_request.title = result['short_description']
+        change_request.description = result['description']
+        change_request.assignment_group_guid = result['assignment_group']['value']
         change_request.save()
 
         return result
@@ -113,15 +125,16 @@ def _get_client(self):
             )
         return self._client
 
-    def _get_snow_group_guid(self, group_name):
+    def get_snow_group_guid(self, group_name):
         """
         Get the SNow Group's GUID from the Group Name
         """
 
         if group_name not in self.group_guid_dict:
             client = self._get_client()
-            query = client.query(table=self.USER_GROUP_TABLE_NAME, query={'name': group_name})
-            result = query.get_one()
+            user_groups = client.resource(api_path=self.USER_GROUP_TABLE_PATH)
+            response = user_groups.get(query={'name': group_name})
+            result = response.one()
             self.group_guid_dict[group_name] = result['sys_id']
 
         return self.group_guid_dict[group_name]
diff --git a/django_snow/migrations/0002_changemgmt_add_createtime_closetime.py b/django_snow/migrations/0002_changemgmt_add_createtime_closetime.py
new file mode 100644
index 0000000..1f1c594
--- /dev/null
+++ b/django_snow/migrations/0002_changemgmt_add_createtime_closetime.py
@@ -0,0 +1,28 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.5 on 2017-12-21 22:57
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.utils.timezone
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('django_snow', '0001_initial'),
+    ]
+
+    # TODO state field must be altered to include the latest changes (once finalized).
+    operations = [
+        migrations.AddField(
+            model_name='changerequest',
+            name='closed_time',
+            field=models.DateTimeField(help_text='Timestamp when the Change Request was closed', null=True),
+        ),
+        migrations.AddField(
+            model_name='changerequest',
+            name='created_time',
+            field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now, help_text='Timestamp when the Change Request was created'),
+            preserve_default=False,
+        ),
+    ]
diff --git a/django_snow/models.py b/django_snow/models.py
index 2f331da..9326ae0 100644
--- a/django_snow/models.py
+++ b/django_snow/models.py
@@ -7,6 +7,10 @@ class ChangeRequest(models.Model):
     SNow Change Request Model Class.
     """
 
+    # TODO: Review the states to be included by default. Some are used only in legacy instances (Geneva and before),
+    # and some are used only in later instances.
+    # https://docs.servicenow.com/bundle/kingston-it-service-management/page/product/change-management/task/state-model-activate-tasks.html
+
     # The state of the Change Request
     TICKET_STATE_OPEN = '1'
     TICKET_STATE_IN_PROGRESS = '2'
@@ -47,9 +51,21 @@ class ChangeRequest(models.Model):
     )
 
     state = models.CharField(
-        max_length=max([len(x[0]) for x in TICKET_STATE_CHOICES]),
+        max_length=3,  # TODO: Review this decision.
         choices=TICKET_STATE_CHOICES,
-        help_text='The current state the the change order is in.'
+        help_text='The current state the change order is in.'
+    )
+
+    # The time at which the Change Request was created.
+    created_time = models.DateTimeField(
+        auto_now_add=True,
+        help_text='Timestamp when the Change Request was created'
+    )
+
+    # The time at which the Change Request was closed.
+    closed_time = models.DateTimeField(
+        null=True,
+        help_text='Timestamp when the Change Request was closed'
     )
 
     def __str__(self):
diff --git a/setup.py b/setup.py
index 113a68c..00fb134 100644
--- a/setup.py
+++ b/setup.py
@@ -26,7 +26,7 @@
     download_url='https://github.com/godaddy/django-snow/archive/master.tar.gz',
     install_requires=[
         'Django>=1.8',
-        'pysnow'
+        'pysnow>=0.6.4'
     ],
 
     classifiers=[
diff --git a/testapp/tests.py b/testapp/tests.py
index 66acc2e..10eca9e 100644
--- a/testapp/tests.py
+++ b/testapp/tests.py
@@ -5,7 +5,7 @@
 from django_snow.helpers import ChangeRequestHandler
 from django_snow.helpers.exceptions import ChangeRequestException
 from django_snow.models import ChangeRequest
-
+from requests.exceptions import HTTPError
 
 try:
     from unittest import mock
@@ -16,8 +16,7 @@
 @override_settings(
     SNOW_INSTANCE='devgodaddy',
     SNOW_API_USER='snow_user',
-    SNOW_API_PASS='snow_pass',
-    SNOW_ASSIGNMENT_GROUP='dev-networking'
+    SNOW_API_PASS='snow_pass'
 )
 @mock.patch('django_snow.helpers.snow_request_handler.pysnow')
 class TestChangeRequestHandler(TestCase):
@@ -46,12 +45,10 @@ def test_settings_and_table_name(self, mock_pysnow):
         self.assertEqual(self.change_request_handler.snow_instance, 'devgodaddy')
         self.assertEqual(self.change_request_handler.snow_api_user, 'snow_user')
         self.assertEqual(self.change_request_handler.snow_api_pass, 'snow_pass')
-        self.assertEqual(self.change_request_handler.CHANGE_REQUEST_TABLE_NAME, 'change_request')
-        self.assertEqual(self.change_request_handler.USER_GROUP_TABLE_NAME, 'sys_user_group')
-        self.assertEqual(self.change_request_handler.snow_assignment_group, 'dev-networking')
+        self.assertEqual(self.change_request_handler.CHANGE_REQUEST_TABLE_PATH, '/table/change_request')
+        self.assertEqual(self.change_request_handler.USER_GROUP_TABLE_PATH, '/table/sys_user_group')
 
-    @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler._get_snow_group_guid')
-    def test_create_change_request(self, mock_get_snow_group_guid, mock_pysnow):
+    def test_create_change_request(self, mock_pysnow):
         fake_insert_retval = {
             'sys_id': uuid.uuid4(),
             'number': 'CHG0000001',
@@ -60,15 +57,16 @@ def test_create_change_request(self, mock_get_snow_group_guid, mock_pysnow):
             'assignment_group': {'value': uuid.uuid4()},
             'state': '2'
         }
-        self.mock_pysnow_client.insert.return_value = fake_insert_retval
+
+        fake_resource = mock.MagicMock()
+        fake_resource.create.return_value = fake_insert_retval
+
+        self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        co = self.change_request_handler.create_change_request(
-            'title', 'description', assignment_group='assignment_group'
-        )
+        co = self.change_request_handler.create_change_request({})
         last_co = ChangeRequest.objects.last()
 
-        mock_get_snow_group_guid.assert_called_with('assignment_group')
         self.assertEqual(co.pk, last_co.pk)
         self.assertEqual(co.sys_id, fake_insert_retval['sys_id'])
         self.assertEqual(co.number, fake_insert_retval['number'])
@@ -76,70 +74,107 @@ def test_create_change_request(self, mock_get_snow_group_guid, mock_pysnow):
         self.assertEqual(co.description, fake_insert_retval['description'])
         self.assertEqual(co.assignment_group_guid, fake_insert_retval['assignment_group']['value'])
 
-    @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler._get_snow_group_guid')
-    def test_create_change_request_raises_exception_when_error_in_result(self, mock_get_snow_group_guid, mock_pysnow):
+    def test_create_change_request_raises_exception_for_http_error(self, mock_pysnow):
+        fake_resource = mock.MagicMock()
+        fake_resource.create.side_effect = HTTPError()
+
+        self.mock_pysnow_client.resource.return_value = fake_resource
+        mock_pysnow.Client.return_value = self.mock_pysnow_client
+
+        with self.assertRaises(ChangeRequestException):
+            self.change_request_handler.create_change_request({})
+
+    def test_create_change_request_raises_exception_when_error_in_result(self, mock_pysnow):
         fake_insert_retval = {
             'error': 'some error message'
         }
-        self.mock_pysnow_client.insert.return_value = fake_insert_retval
+
+        fake_resource = mock.MagicMock()
+        fake_resource.create.return_value = fake_insert_retval
+
+        self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
         with self.assertRaises(ChangeRequestException):
-            self.change_request_handler.create_change_request(
-                'title', 'description', assignment_group='assignment_group'
-            )
+            self.change_request_handler.create_change_request({})
 
     @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler.update_change_request')
     def test_close_change_request(self, mock_update_request, mock_pysnow):
+        fake_change_order = mock.MagicMock()
+
         mock_update_request.return_value = 'foo'
         change_request_handler = ChangeRequestHandler()
 
-        change_request_handler.close_change_request('some change order')
+        change_request_handler.close_change_request(fake_change_order)
 
-        mock_update_request.assert_called_with('some change order', {'state': ChangeRequest.TICKET_STATE_COMPLETE})
+        mock_update_request.assert_called_with(fake_change_order, {'state': ChangeRequest.TICKET_STATE_COMPLETE})
 
     @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler.update_change_request')
     def test_close_change_request_with_error(self, mock_update_request, mock_pysnow):
+        fake_change_order = mock.MagicMock()
         mock_update_request.return_value = 'foo'
         change_request_handler = ChangeRequestHandler()
         payload = {'description': 'foo'}
-        change_request_handler.close_change_request_with_error('some change order', payload)
+        change_request_handler.close_change_request_with_error(fake_change_order, payload)
 
         mock_update_request.assert_called_with(
-            'some change order', {'state': ChangeRequest.TICKET_STATE_COMPLETE_WITH_ERRORS, 'description': 'foo'}
+            fake_change_order, {'state': ChangeRequest.TICKET_STATE_COMPLETE_WITH_ERRORS, 'description': 'foo'}
         )
 
     def test_update_change_request(self, mock_pysnow):
-        fake_query = mock.MagicMock()
+        fake_resource = mock.MagicMock()
         fake_change_order = mock.MagicMock()
 
-        fake_query.update.return_value = {'state': ChangeRequest.TICKET_STATE_COMPLETE}
-        self.mock_pysnow_client.query.return_value = fake_query
+        retval = {
+            'state': ChangeRequest.TICKET_STATE_COMPLETE,
+            'short_description': 'Short Description',
+            'description': 'Long Description',
+            'assignment_group': {'value': uuid.uuid4()}
+        }
+
+        fake_resource.update.return_value = retval
+        self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
         ret_val = self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
         self.assertEqual(fake_change_order.state, ChangeRequest.TICKET_STATE_COMPLETE)
-        self.assertEqual(ret_val, {'state': ChangeRequest.TICKET_STATE_COMPLETE})
+        self.assertEqual(fake_change_order.title, retval['short_description'])
+        self.assertEqual(fake_change_order.description, retval['description'])
+        self.assertEqual(fake_change_order.assignment_group_guid, retval['assignment_group']['value'])
+        self.assertEqual(ret_val, retval)
 
-    def test_update_change_request_raises_exception(self, mock_pysnow):
-        fake_query = mock.MagicMock()
+    def test_update_change_request_raises_exception_for_http_error(self, mock_pysnow):
+        fake_resource = mock.MagicMock()
         fake_change_order = mock.MagicMock()
+        fake_resource.update.side_effect = HTTPError()
 
-        fake_query.update.return_value = {'error': '3'}
-        self.mock_pysnow_client.query.return_value = fake_query
+        self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
         with self.assertRaises(ChangeRequestException):
             self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
 
-    def test__get_snow_group_guid_cached_result(self, mock_pysnow):
-        self.change_request_handler.group_guid_dict['foo'] = 'bar'
-        self.assertEqual(self.change_request_handler._get_snow_group_guid('foo'), 'bar')
+    def test_update_change_request_raises_exception_for_error_in_result(self, mock_pysnow):
+        fake_resource = mock.MagicMock()
+        fake_change_order = mock.MagicMock()
 
-    def test__get_snow_group_guid(self, mock_pysnow):
-        fake_query = mock.MagicMock()
-        fake_query.get_one.return_value = {'sys_id': 'yo'}
-        self.mock_pysnow_client.query.return_value = fake_query
+        fake_resource.update.return_value = {'error': '3'}
+        self.mock_pysnow_client.resource.return_value = fake_resource
+        mock_pysnow.Client.return_value = self.mock_pysnow_client
+
+        with self.assertRaises(ChangeRequestException):
+            self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
+
+    def test_get_snow_group_guid_cached_result(self, mock_pysnow):
+        self.change_request_handler.group_guid_dict['foo'] = 'bar'
+        self.assertEqual(self.change_request_handler.get_snow_group_guid('foo'), 'bar')
+
+    def test_get_snow_group_guid(self, mock_pysnow):
+        fake_resource = mock.MagicMock()
+        fake_response = mock.MagicMock()
+        fake_response.one.return_value = {'sys_id': 'yo'}
+        fake_resource.get.return_value = fake_response
+        self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        self.assertEqual(self.change_request_handler._get_snow_group_guid('hello'), 'yo')
+        self.assertEqual(self.change_request_handler.get_snow_group_guid('hello'), 'yo')

From b01ee4036d5b4c1d257722cdcde14ea791858b30 Mon Sep 17 00:00:00 2001
From: Pradeep Kumar <pradeepsixer@gmail.com>
Date: Tue, 26 Dec 2017 17:39:51 -0700
Subject: [PATCH 2/5] API changes

---
 README.rst                                  |  9 +--
 django_snow/helpers/snow_request_handler.py | 12 ++-
 testapp/tests.py                            | 82 +++++++++++++++++++--
 3 files changed, 89 insertions(+), 14 deletions(-)

diff --git a/README.rst b/README.rst
index ca69fdd..92e68ce 100644
--- a/README.rst
+++ b/README.rst
@@ -32,6 +32,8 @@ Configuration
 * ``SNOW_ASSIGNMENT_GROUP`` (Optional) - The group to which the tickets should be assigned.
   If this is not provided, each call to create the tickets should be provided with an `assignment_group` argument.
   See the API documentation for more details
+* ``SNOW_DEFAULT_CHANGE_TYPE`` (Optional) - Default Change Request Type. If not provided,
+  `standard` will considered as the default type.
 
 Usage
 =====
@@ -44,14 +46,9 @@ Creation
 
 * ``title`` - The title of the change request
 * ``description`` - The description of the change request
-* ``co_type`` (Optional) - One of the following:
-
-  * ``Automated`` (Default)
-  * ``Manual``
-  * ``Emergency``
-
 * ``assignment_group`` - The group to which the change request is to be assigned.
   This is **optional** if ``SNOW_ASSIGNMENT_GROUP`` django settings is available, else, it is **mandatory**
+* ``payload`` (Optional) - The payload for creating the Change Request.
 
 **Returns**
 
diff --git a/django_snow/helpers/snow_request_handler.py b/django_snow/helpers/snow_request_handler.py
index 8032ae6..5f2da85 100644
--- a/django_snow/helpers/snow_request_handler.py
+++ b/django_snow/helpers/snow_request_handler.py
@@ -28,13 +28,23 @@ def __init__(self):
         self.snow_instance = settings.SNOW_INSTANCE
         self.snow_api_user = settings.SNOW_API_USER
         self.snow_api_pass = settings.SNOW_API_PASS
+        self.snow_assignment_group = getattr(settings, 'SNOW_ASSIGNMENT_GROUP', None)
+        self.snow_default_cr_type = getattr(settings, 'SNOW_DEFAULT_CHANGE_TYPE', 'standard')
 
-    def create_change_request(self, payload):
+    def create_change_request(self, title, description, assignment_group=None, payload=None):
         """
         Create a change request with the given payload.
         """
         client = self._get_client()
         change_requests = client.resource(api_path=self.CHANGE_REQUEST_TABLE_PATH)
+        payload = payload or {}
+        payload['short_description'] = title
+        payload['description'] = description
+
+        if 'type' not in payload:
+            payload['type'] = self.snow_default_cr_type
+        if 'assignment_group' not in payload:
+            payload['assignment_group'] = self.get_snow_group_guid(assignment_group or self.snow_assignment_group)
 
         try:
             result = change_requests.create(payload=payload)
diff --git a/testapp/tests.py b/testapp/tests.py
index 10eca9e..720b695 100644
--- a/testapp/tests.py
+++ b/testapp/tests.py
@@ -1,11 +1,13 @@
 import uuid
+from collections import OrderedDict
 
 from django.test import TestCase, override_settings
+from requests.exceptions import HTTPError
 
 from django_snow.helpers import ChangeRequestHandler
 from django_snow.helpers.exceptions import ChangeRequestException
 from django_snow.models import ChangeRequest
-from requests.exceptions import HTTPError
+
 
 try:
     from unittest import mock
@@ -16,7 +18,8 @@
 @override_settings(
     SNOW_INSTANCE='devgodaddy',
     SNOW_API_USER='snow_user',
-    SNOW_API_PASS='snow_pass'
+    SNOW_API_PASS='snow_pass',
+    SNOW_ASSIGNMENT_GROUP='assignment_group'
 )
 @mock.patch('django_snow.helpers.snow_request_handler.pysnow')
 class TestChangeRequestHandler(TestCase):
@@ -45,6 +48,8 @@ def test_settings_and_table_name(self, mock_pysnow):
         self.assertEqual(self.change_request_handler.snow_instance, 'devgodaddy')
         self.assertEqual(self.change_request_handler.snow_api_user, 'snow_user')
         self.assertEqual(self.change_request_handler.snow_api_pass, 'snow_pass')
+        self.assertEqual(self.change_request_handler.snow_assignment_group, 'assignment_group')
+        self.assertEqual(self.change_request_handler.snow_default_cr_type, 'standard')
         self.assertEqual(self.change_request_handler.CHANGE_REQUEST_TABLE_PATH, '/table/change_request')
         self.assertEqual(self.change_request_handler.USER_GROUP_TABLE_PATH, '/table/sys_user_group')
 
@@ -64,7 +69,7 @@ def test_create_change_request(self, mock_pysnow):
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        co = self.change_request_handler.create_change_request({})
+        co = self.change_request_handler.create_change_request('Title', 'Description', payload={})
         last_co = ChangeRequest.objects.last()
 
         self.assertEqual(co.pk, last_co.pk)
@@ -74,15 +79,73 @@ def test_create_change_request(self, mock_pysnow):
         self.assertEqual(co.description, fake_insert_retval['description'])
         self.assertEqual(co.assignment_group_guid, fake_insert_retval['assignment_group']['value'])
 
+    def test_create_change_request_parameters(self, mock_pysnow):
+        expected_payload = OrderedDict()
+        expected_payload['type'] = 'normal'
+        expected_payload['assignment_group'] = 'bar'
+        expected_payload['short_description'] = 'Title'
+        expected_payload['description'] = 'Description'
+
+        fake_insert_retval = {
+            'sys_id': uuid.uuid4(),
+            'number': 'CHG0000001',
+            'short_description': 'Title',
+            'description': 'Description',
+            'assignment_group': {'value': uuid.uuid4()},
+            'state': '2'
+        }
+
+        fake_resource = mock.MagicMock()
+        fake_resource.create.return_value = fake_insert_retval
+        self.mock_pysnow_client.resource.return_value = fake_resource
+        mock_pysnow.Client.return_value = self.mock_pysnow_client
+
+        payload = OrderedDict()
+        payload['type'] = 'normal'
+        payload['assignment_group'] = 'bar'
+        self.change_request_handler.create_change_request('Title', 'Description', None, payload=payload)
+        fake_resource.create.assert_called_with(payload=expected_payload)
+
+    def test_create_change_request_default_parameters(self, mock_pysnow):
+        expected_payload = OrderedDict()
+        expected_payload['short_description'] = 'Title'
+        expected_payload['description'] = 'Description'
+        expected_payload['type'] = 'standard'
+        expected_payload['assignment_group'] = 'bar'
+
+        self.change_request_handler.group_guid_dict['assignment_group'] = 'bar'
+
+        fake_insert_retval = {
+            'sys_id': uuid.uuid4(),
+            'number': 'CHG0000001',
+            'short_description': 'Title',
+            'description': 'Description',
+            'assignment_group': {'value': uuid.uuid4()},
+            'state': '2'
+        }
+
+        fake_resource = mock.MagicMock()
+        fake_resource.create.return_value = fake_insert_retval
+        self.mock_pysnow_client.resource.return_value = fake_resource
+        mock_pysnow.Client.return_value = self.mock_pysnow_client
+
+        self.change_request_handler.create_change_request('Title', 'Description', None, payload=OrderedDict())
+        fake_resource.create.assert_called_with(payload=expected_payload)
+
     def test_create_change_request_raises_exception_for_http_error(self, mock_pysnow):
         fake_resource = mock.MagicMock()
-        fake_resource.create.side_effect = HTTPError()
+
+        fake_exception = HTTPError()
+        fake_exception.response = mock.MagicMock()
+        fake_exception.response.text.return_value = 'Foobar'
+
+        fake_resource.create.side_effect = fake_exception
 
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
         with self.assertRaises(ChangeRequestException):
-            self.change_request_handler.create_change_request({})
+            self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
 
     def test_create_change_request_raises_exception_when_error_in_result(self, mock_pysnow):
         fake_insert_retval = {
@@ -96,7 +159,7 @@ def test_create_change_request_raises_exception_when_error_in_result(self, mock_
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
         with self.assertRaises(ChangeRequestException):
-            self.change_request_handler.create_change_request({})
+            self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
 
     @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler.update_change_request')
     def test_close_change_request(self, mock_update_request, mock_pysnow):
@@ -146,7 +209,12 @@ def test_update_change_request(self, mock_pysnow):
     def test_update_change_request_raises_exception_for_http_error(self, mock_pysnow):
         fake_resource = mock.MagicMock()
         fake_change_order = mock.MagicMock()
-        fake_resource.update.side_effect = HTTPError()
+
+        fake_exception = HTTPError()
+        fake_exception.response = mock.MagicMock()
+        fake_exception.response.text.return_value = 'Foobar'
+
+        fake_resource.update.side_effect = fake_exception
 
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client

From 56e7c45537797df0e3f28b2df11b4a4f59c42d33 Mon Sep 17 00:00:00 2001
From: Pradeep Kumar <pradeepsixer@gmail.com>
Date: Tue, 9 Jan 2018 13:43:27 -0700
Subject: [PATCH 3/5] Refactoring of Test Cases

1. Replaced usage of OrderedDict usage with dictionary.
2. Rewritten test cases that required internal working knowledge.
3. assertRaises replaced with assertRaisesRegex
---
 django_snow/helpers/snow_request_handler.py |  6 ++
 testapp/tests.py                            | 83 +++++++++++++++------
 2 files changed, 66 insertions(+), 23 deletions(-)

diff --git a/django_snow/helpers/snow_request_handler.py b/django_snow/helpers/snow_request_handler.py
index 5f2da85..a754603 100644
--- a/django_snow/helpers/snow_request_handler.py
+++ b/django_snow/helpers/snow_request_handler.py
@@ -148,3 +148,9 @@ def get_snow_group_guid(self, group_name):
             self.group_guid_dict[group_name] = result['sys_id']
 
         return self.group_guid_dict[group_name]
+
+    def clear_group_guid_cache(self):
+        """
+        Clear the SNow Group Name - GUID cache.
+        """
+        self.group_guid_dict.clear()
diff --git a/testapp/tests.py b/testapp/tests.py
index 720b695..b0dc0c2 100644
--- a/testapp/tests.py
+++ b/testapp/tests.py
@@ -1,5 +1,4 @@
 import uuid
-from collections import OrderedDict
 
 from django.test import TestCase, override_settings
 from requests.exceptions import HTTPError
@@ -28,6 +27,9 @@ def setUp(self):
         self.mock_pysnow_client = mock.MagicMock()
         self.change_request_handler = ChangeRequestHandler()
 
+    def tearDown(self):
+        self.change_request_handler.clear_group_guid_cache()
+
     def test__get_client(self, mock_pysnow):
         mock_pysnow.Client.return_value = self.mock_pysnow_client
         self.assertIs(
@@ -80,11 +82,12 @@ def test_create_change_request(self, mock_pysnow):
         self.assertEqual(co.assignment_group_guid, fake_insert_retval['assignment_group']['value'])
 
     def test_create_change_request_parameters(self, mock_pysnow):
-        expected_payload = OrderedDict()
-        expected_payload['type'] = 'normal'
-        expected_payload['assignment_group'] = 'bar'
-        expected_payload['short_description'] = 'Title'
-        expected_payload['description'] = 'Description'
+        expected_payload = {
+            'type': 'normal',
+            'assignment_group': 'bar',
+            'short_description': 'Title',
+            'description': 'Description'
+        }
 
         fake_insert_retval = {
             'sys_id': uuid.uuid4(),
@@ -100,20 +103,20 @@ def test_create_change_request_parameters(self, mock_pysnow):
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        payload = OrderedDict()
-        payload['type'] = 'normal'
-        payload['assignment_group'] = 'bar'
+        payload = {
+            'type': 'normal',
+            'assignment_group': 'bar'
+        }
         self.change_request_handler.create_change_request('Title', 'Description', None, payload=payload)
         fake_resource.create.assert_called_with(payload=expected_payload)
 
     def test_create_change_request_default_parameters(self, mock_pysnow):
-        expected_payload = OrderedDict()
-        expected_payload['short_description'] = 'Title'
-        expected_payload['description'] = 'Description'
-        expected_payload['type'] = 'standard'
-        expected_payload['assignment_group'] = 'bar'
-
-        self.change_request_handler.group_guid_dict['assignment_group'] = 'bar'
+        expected_payload = {
+            'short_description': 'Title',
+            'description': 'Description',
+            'type': 'standard',
+            'assignment_group': 'bar'
+        }
 
         fake_insert_retval = {
             'sys_id': uuid.uuid4(),
@@ -125,11 +128,17 @@ def test_create_change_request_default_parameters(self, mock_pysnow):
         }
 
         fake_resource = mock.MagicMock()
+
+        # For Assignment Group GUID
+        fake_asgn_group_guid_response = mock.MagicMock()
+        fake_asgn_group_guid_response.one.return_value = {'sys_id': 'bar'}
+        fake_resource.get.return_value = fake_asgn_group_guid_response
+
         fake_resource.create.return_value = fake_insert_retval
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        self.change_request_handler.create_change_request('Title', 'Description', None, payload=OrderedDict())
+        self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
         fake_resource.create.assert_called_with(payload=expected_payload)
 
     def test_create_change_request_raises_exception_for_http_error(self, mock_pysnow):
@@ -144,7 +153,7 @@ def test_create_change_request_raises_exception_for_http_error(self, mock_pysnow
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaises(ChangeRequestException):
+        with self.assertRaisesRegex(ChangeRequestException, 'Could not create change request due to.*'):
             self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
 
     def test_create_change_request_raises_exception_when_error_in_result(self, mock_pysnow):
@@ -158,7 +167,7 @@ def test_create_change_request_raises_exception_when_error_in_result(self, mock_
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaises(ChangeRequestException):
+        with self.assertRaisesRegex(ChangeRequestException, 'Could not create change request due to.*'):
             self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
 
     @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler.update_change_request')
@@ -219,7 +228,7 @@ def test_update_change_request_raises_exception_for_http_error(self, mock_pysnow
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaises(ChangeRequestException):
+        with self.assertRaisesRegex(ChangeRequestException, 'Could not update change request due to '):
             self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
 
     def test_update_change_request_raises_exception_for_error_in_result(self, mock_pysnow):
@@ -230,12 +239,23 @@ def test_update_change_request_raises_exception_for_error_in_result(self, mock_p
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaises(ChangeRequestException):
+        with self.assertRaisesRegex(ChangeRequestException, 'Could not update change request due to '):
             self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
 
     def test_get_snow_group_guid_cached_result(self, mock_pysnow):
-        self.change_request_handler.group_guid_dict['foo'] = 'bar'
-        self.assertEqual(self.change_request_handler.get_snow_group_guid('foo'), 'bar')
+        fake_resource = mock.MagicMock()
+        fake_response = mock.MagicMock()
+        fake_response.one.return_value = {'sys_id': 'bar'}
+        fake_resource.get.return_value = fake_response
+        self.mock_pysnow_client.resource.return_value = fake_resource
+        mock_pysnow.Client.return_value = self.mock_pysnow_client
+
+        self.change_request_handler.get_snow_group_guid('foo')
+        cached_guid = self.change_request_handler.get_snow_group_guid('foo')
+
+        # resource.get() should be called only once, since the value from previous call should have been cached.
+        fake_resource.get.assert_called_once_with(query={'name': 'foo'})
+        self.assertEqual(cached_guid, 'bar')
 
     def test_get_snow_group_guid(self, mock_pysnow):
         fake_resource = mock.MagicMock()
@@ -246,3 +266,20 @@ def test_get_snow_group_guid(self, mock_pysnow):
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
         self.assertEqual(self.change_request_handler.get_snow_group_guid('hello'), 'yo')
+
+    def test_clear_snow_group_guid_cache(self, mock_pysnow):
+        fake_resource = mock.MagicMock()
+        fake_response = mock.MagicMock()
+        fake_response.one.return_value = {'sys_id': 'some_id'}
+        fake_resource.get.return_value = fake_response
+        self.mock_pysnow_client.resource.return_value = fake_resource
+        mock_pysnow.Client.return_value = self.mock_pysnow_client
+
+        self.change_request_handler.get_snow_group_guid('hello')
+        self.change_request_handler.get_snow_group_guid('hello')
+        self.assertEqual(fake_resource.get.call_count, 1)
+
+        self.change_request_handler.clear_group_guid_cache()
+        self.change_request_handler.get_snow_group_guid('hello')
+        self.change_request_handler.get_snow_group_guid('hello')
+        self.assertEqual(fake_resource.get.call_count, 2)

From b78cb872cb2435690244f62387420259503fe38e Mon Sep 17 00:00:00 2001
From: Pradeep Kumar <pradeepsixer@gmail.com>
Date: Tue, 9 Jan 2018 20:12:09 -0700
Subject: [PATCH 4/5] Fix test cases for Python 2.7

Added 'six' package for assertRaisesRegex shim for Python 2.7
---
 setup.py         | 3 ++-
 testapp/tests.py | 9 +++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/setup.py b/setup.py
index 00fb134..ba34796 100644
--- a/setup.py
+++ b/setup.py
@@ -26,7 +26,8 @@
     download_url='https://github.com/godaddy/django-snow/archive/master.tar.gz',
     install_requires=[
         'Django>=1.8',
-        'pysnow>=0.6.4'
+        'pysnow>=0.6.4',
+        'six'
     ],
 
     classifiers=[
diff --git a/testapp/tests.py b/testapp/tests.py
index b0dc0c2..2d1299b 100644
--- a/testapp/tests.py
+++ b/testapp/tests.py
@@ -1,5 +1,6 @@
 import uuid
 
+import six
 from django.test import TestCase, override_settings
 from requests.exceptions import HTTPError
 
@@ -153,7 +154,7 @@ def test_create_change_request_raises_exception_for_http_error(self, mock_pysnow
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaisesRegex(ChangeRequestException, 'Could not create change request due to.*'):
+        with six.assertRaisesRegex(self, ChangeRequestException, 'Could not create change request due to.*'):
             self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
 
     def test_create_change_request_raises_exception_when_error_in_result(self, mock_pysnow):
@@ -167,7 +168,7 @@ def test_create_change_request_raises_exception_when_error_in_result(self, mock_
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaisesRegex(ChangeRequestException, 'Could not create change request due to.*'):
+        with six.assertRaisesRegex(self, ChangeRequestException, 'Could not create change request due to.*'):
             self.change_request_handler.create_change_request('Title', 'Description', None, payload={})
 
     @mock.patch('django_snow.helpers.snow_request_handler.ChangeRequestHandler.update_change_request')
@@ -228,7 +229,7 @@ def test_update_change_request_raises_exception_for_http_error(self, mock_pysnow
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaisesRegex(ChangeRequestException, 'Could not update change request due to '):
+        with six.assertRaisesRegex(self, ChangeRequestException, 'Could not update change request due to '):
             self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
 
     def test_update_change_request_raises_exception_for_error_in_result(self, mock_pysnow):
@@ -239,7 +240,7 @@ def test_update_change_request_raises_exception_for_error_in_result(self, mock_p
         self.mock_pysnow_client.resource.return_value = fake_resource
         mock_pysnow.Client.return_value = self.mock_pysnow_client
 
-        with self.assertRaisesRegex(ChangeRequestException, 'Could not update change request due to '):
+        with six.assertRaisesRegex(self, ChangeRequestException, 'Could not update change request due to '):
             self.change_request_handler.update_change_request(fake_change_order, payload='{"foo": "bar"}')
 
     def test_get_snow_group_guid_cached_result(self, mock_pysnow):

From 4c69e299c63a18c02f78d07dadcdd40a00e933e9 Mon Sep 17 00:00:00 2001
From: Pradeep Kumar <pradeepsixer@gmail.com>
Date: Wed, 10 Jan 2018 16:27:27 -0700
Subject: [PATCH 5/5] Fix dependencies

---
 setup.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index ba34796..af4a716 100644
--- a/setup.py
+++ b/setup.py
@@ -27,7 +27,9 @@
     install_requires=[
         'Django>=1.8',
         'pysnow>=0.6.4',
-        'six'
+    ],
+    tests_require=[
+        'six',
     ],
 
     classifiers=[