Repository: ucfopen/canvasapi

Issue:
Canvas Release 2017-11-18
https://community.canvaslms.com/docs/DOC-13339#jive_content_id_API

The November 18th release of Canvas includes some changes to the API that we should keep an eye on. Of particular interest:
- subaccounts can now be deleted
- `sis_login_id` is deprecated. should use `login_id` instead.

This gives us two action items:

- [x] Add support for subaccount deletion (https://canvas.beta.instructure.com/doc/api/accounts.html#method.sub_accounts.destroy)
- [x] Change references of `sis_login_id` in our documentation to `login_id`

  
  

Discussion:

Patch:
diff --git a/README.md b/README.md
index a9e5ece..29d9dee 100644
--- a/README.md
+++ b/README.md
@@ -92,8 +92,8 @@ Individual users can be pulled from the API as well:
 # Retrieve a list of courses the user is enrolled in
 >>> courses = user.get_courses()
 
-# Grab a different user by their SIS ID
->>> sis_user = canvas.get_user('some_user', 'sis_login_id')
+# Grab a different user by their login ID
+>>> login_id_user = canvas.get_user('some_user', 'login_id')
 ```
 
 #### Paginated Lists
diff --git a/canvasapi/account.py b/canvasapi/account.py
index eb52ec0..f2dff76 100644
--- a/canvasapi/account.py
+++ b/canvasapi/account.py
@@ -4,7 +4,7 @@ from six import python_2_unicode_compatible
 
 from canvasapi.canvas_object import CanvasObject
 from canvasapi.grading_standard import GradingStandard
-from canvasapi.exceptions import RequiredFieldMissing
+from canvasapi.exceptions import CanvasException, RequiredFieldMissing
 from canvasapi.paginated_list import PaginatedList
 from canvasapi.rubric import Rubric
 from canvasapi.util import combine_kwargs, obj_or_id
@@ -154,6 +154,29 @@ class Account(CanvasObject):
         )
         return AccountNotification(self._requester, response.json())
 
+    def delete(self):
+        """
+        Delete the current account
+
+        Note: Cannot delete an account with active courses or active
+        sub accounts. Cannot delete a root account.
+
+        :calls: `DELETE /api/v1/accounts/:account_id/sub_accounts/:id \
+        <https://canvas.beta.instructure.com/doc/api/accounts.html#method.sub_accounts.destroy>`_
+
+        :returns: True if successfully deleted; False otherwise.
+        :rtype: bool
+        """
+        if not hasattr(self, 'parent_account_id') or not self.parent_account_id:
+            raise CanvasException("Cannot delete a root account.")
+
+        response = self._requester.request(
+            'DELETE',
+            'accounts/{}/sub_accounts/{}'.format(self.parent_account_id, self.id)
+        )
+
+        return response.json().get('workflow_state') == 'deleted'
+
     def delete_user(self, user):
         """
         Delete a user record from a Canvas root account.
diff --git a/docs/getting-started.rst b/docs/getting-started.rst
index eebb59e..9159afe 100644
--- a/docs/getting-started.rst
+++ b/docs/getting-started.rst
@@ -67,8 +67,8 @@ User objects
     # Retrieve a list of courses the user is enrolled in
     >>> courses = user.get_courses()
 
-    # Grab a different user by their SIS ID
-    >>> sis_user = canvas.get_user('some_user', 'sis_login_id')
+    # Grab a different user by their login ID
+    >>> login_id_user = canvas.get_user('some_user', 'login_id')
 
 Paginated Lists
 ~~~~~~~~~~~~~~~


Test patch:
diff --git a/tests/fixtures/account.json b/tests/fixtures/account.json
index 0044b61..8ee4012 100644
--- a/tests/fixtures/account.json
+++ b/tests/fixtures/account.json
@@ -83,7 +83,8 @@
 			"default_storage_quota_mb": 500,
 			"default_user_storage_quota_mb": 50,
 			"default_group_storage_quota_mb": 50,
-			"default_time_zone": "America/New_York"
+			"default_time_zone": "America/New_York",
+			"workflow_state": "active"
 		},
 		"status_code": 200
 	},
@@ -97,6 +98,22 @@
 		},
 		"status_code": 200
 	},
+	"delete_subaccount": {
+		"method": "DELETE",
+		"endpoint": "accounts/1/sub_accounts/101",
+		"data": {
+			"id": 101,
+			"name": "New Subaccount",
+			"parent_account_id": 1,
+			"root_account_id": 1,
+			"default_storage_quota_mb": 500,
+			"default_user_storage_quota_mb": 50,
+			"default_group_storage_quota_mb": 50,
+			"default_time_zone": "America/New_York",
+			"workflow_state": "deleted"
+		},
+		"status_code": 200
+	},
 	"delete_user": {
 		"method": "DELETE",
 		"endpoint": "accounts/1/users/1",
diff --git a/tests/fixtures/course.json b/tests/fixtures/course.json
index 16cf256..1aa9eb8 100644
--- a/tests/fixtures/course.json
+++ b/tests/fixtures/course.json
@@ -357,11 +357,11 @@
 	},
 	"get_user_id_type": {
 		"method": "GET",
-		"endpoint": "courses/1/users/sis_login_id:SISLOGIN",
+		"endpoint": "courses/1/users/login_id:LOGINID",
 		"data": {
 			"id": 123456,
 			"name": "Abby Smith",
-			"sis_login_id": "SISLOGIN"
+			"login_id": "LOGINID"
 		},
 		"status_code": 200
 	},
diff --git a/tests/test_account.py b/tests/test_account.py
index dac1e25..cb82092 100644
--- a/tests/test_account.py
+++ b/tests/test_account.py
@@ -12,7 +12,7 @@ from canvasapi.course import Course
 from canvasapi.enrollment import Enrollment
 from canvasapi.enrollment_term import EnrollmentTerm
 from canvasapi.external_tool import ExternalTool
-from canvasapi.exceptions import RequiredFieldMissing
+from canvasapi.exceptions import CanvasException, RequiredFieldMissing
 from canvasapi.grading_standard import GradingStandard
 from canvasapi.group import Group, GroupCategory
 from canvasapi.login import Login
@@ -133,6 +133,18 @@ class TestAccount(unittest.TestCase):
         with self.assertRaises(RequiredFieldMissing):
             self.account.create_notification({})
 
+    # delete()
+    def test_delete(self, m):
+        register_uris({'account': ['create_subaccount', 'delete_subaccount']}, m)
+
+        subaccount = self.account.create_subaccount({'name': 'New Subaccount'})
+
+        self.assertTrue(subaccount.delete())
+
+    def test_delete_root_account(self, m):
+        with self.assertRaises(CanvasException):
+            self.account.delete()
+
     # delete_user()
     def test_delete_user_id(self, m):
         register_uris({'account': ['delete_user']}, m)
diff --git a/tests/test_course.py b/tests/test_course.py
index 11dcc13..888ca59 100644
--- a/tests/test_course.py
+++ b/tests/test_course.py
@@ -95,7 +95,7 @@ class TestCourse(unittest.TestCase):
     def test_get_user_id_type(self, m):
         register_uris({'course': ['get_user_id_type']}, m)
 
-        user = self.course.get_user("SISLOGIN", "sis_login_id")
+        user = self.course.get_user("LOGINID", "login_id")
 
         self.assertIsInstance(user, User)
         self.assertTrue(hasattr(user, 'name'))