Repository: ucfopen/canvasapi (language: python)

Issue:
Attach files when creating a new discussion topic
# Describe the bug

Unable to upload attachment files in `Course.create_discussion_topic`

# To Reproduce

See discussion #620 (thanks @SebasMeza)

# Possible Solutions

This is a symptom of a larger problem with file handling. The library doesn't currently allow for files to be passed as arbitrary keyword arguments. That's a feature we should probably look into at some point (in a new issue), but for the purposes of this issue we can use an optional argument `assignment` and the `file_or_path` util function to achieve the same effect, like in `Course.import_outcome`. Since `assignment` isn't mandatory, the implementation will be slightly different.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cff78414..1a1aad4b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,10 @@
 
 - Added support for pagination with metadata when headers are missing (Thanks, [@bennettscience](https://github.com/bennettscience))
 
+### Bugfixes
+
+- Fixed an issue where `Course.create_discussion_topic` wouldn't accept attachment files.
+
 ## [3.1.0] - 2023-04-21
 
 ### New Endpoint Coverage
diff --git a/canvasapi/course.py b/canvasapi/course.py
index 5c2bb0d6..21bde43f 100644
--- a/canvasapi/course.py
+++ b/canvasapi/course.py
@@ -279,25 +279,37 @@ def create_custom_column(self, column, **kwargs):
 
         return CustomGradebookColumn(self._requester, column_json)
 
-    def create_discussion_topic(self, **kwargs):
+    def create_discussion_topic(self, attachment=None, **kwargs):
         """
         Creates a new discussion topic for the course or group.
 
         :calls: `POST /api/v1/courses/:course_id/discussion_topics \
         <https://canvas.instructure.com/doc/api/discussion_topics.html#method.discussion_topics.create>`_
 
+        :param attachment: (Optional) A file handler or path of the file to import.
+        :type attachment: file or str
+
         :rtype: :class:`canvasapi.discussion_topic.DiscussionTopic`
         """
-        response = self._requester.request(
-            "POST",
-            "courses/{}/discussion_topics".format(self.id),
-            _kwargs=combine_kwargs(**kwargs),
-        )
+        if attachment is not None:
+            attachment_file, is_path = file_or_path(attachment)
+            attachment = {"attachment": attachment_file}
 
-        response_json = response.json()
-        response_json.update({"course_id": self.id})
+        try:
+            response = self._requester.request(
+                "POST",
+                "courses/{}/discussion_topics".format(self.id),
+                file=attachment,
+                _kwargs=combine_kwargs(**kwargs),
+            )
 
-        return DiscussionTopic(self._requester, response_json)
+            response_json = response.json()
+            response_json.update({"course_id": self.id})
+
+            return DiscussionTopic(self._requester, response_json)
+        finally:
+            if attachment is not None and is_path:
+                attachment_file.close()
 
     def create_epub_export(self, **kwargs):
         """
diff --git a/canvasapi/requester.py b/canvasapi/requester.py
index b1b87450..bda2c991 100644
--- a/canvasapi/requester.py
+++ b/canvasapi/requester.py
@@ -98,7 +98,7 @@ def _post_request(self, url, headers, data=None, json=False):
         files = None
         for field, value in data:
             if field == "file":
-                if isinstance(value, dict):
+                if isinstance(value, dict) or value is None:
                     files = value
                 else:
                     files = {"file": value}


Test patch:
diff --git a/tests/fixtures/generic_file.txt b/tests/fixtures/generic_file.txt
new file mode 100644
index 00000000..d333441d
--- /dev/null
+++ b/tests/fixtures/generic_file.txt
@@ -0,0 +1,1 @@
+This is a generic file for testing file uploads.
diff --git a/tests/test_course.py b/tests/test_course.py
index 214bafb2..2e2669dd 100644
--- a/tests/test_course.py
+++ b/tests/test_course.py
@@ -1,3 +1,4 @@
+import os
 import unittest
 import uuid
 import warnings
@@ -898,7 +899,7 @@ def test_create_column_fail(self, m):
             self.course.create_custom_column(column={})
 
     # create_discussion_topic()
-    def test_create_discussion_topic(self, m):
+    def test_create_discussion_topic_no_file(self, m):
         register_uris({"course": ["create_discussion_topic"]}, m)
 
         title = "Topic 1"
@@ -908,6 +909,40 @@ def test_create_discussion_topic(self, m):
         self.assertEqual(title, discussion.title)
         self.assertEqual(discussion.course_id, 1)
 
+    def test_create_discussion_topic_file_path(self, m):
+        register_uris({"course": ["create_discussion_topic"]}, m)
+
+        filepath = os.path.join("tests", "fixtures", "generic_file.txt")
+
+        title = "Topic 1"
+        discussion = self.course.create_discussion_topic(attachment=filepath)
+        self.assertIsInstance(discussion, DiscussionTopic)
+        self.assertTrue(hasattr(discussion, "course_id"))
+        self.assertEqual(title, discussion.title)
+        self.assertEqual(discussion.course_id, 1)
+
+    def test_create_discussion_topic_file_path_invalid(self, m):
+        register_uris({"course": ["create_discussion_topic"]}, m)
+
+        filepath = "this/path/doesnt/exist"
+
+        with self.assertRaises(IOError):
+            self.course.create_discussion_topic(attachment=filepath)
+
+    def test_create_discussion_topic_file_obj(self, m):
+        register_uris({"course": ["create_discussion_topic"]}, m)
+
+        filepath = os.path.join("tests", "fixtures", "generic_file.txt")
+
+        title = "Topic 1"
+        with open(filepath, "rb") as f:
+            discussion = self.course.create_discussion_topic(attachment=f)
+
+        self.assertIsInstance(discussion, DiscussionTopic)
+        self.assertTrue(hasattr(discussion, "course_id"))
+        self.assertEqual(title, discussion.title)
+        self.assertEqual(discussion.course_id, 1)
+
     # reorder_pinned_topics()
     def test_reorder_pinned_topics(self, m):
         # Custom matcher to test that params are set correctly
@@ -1583,8 +1618,6 @@ def test_get_outcome_import_status_latest(self, m):
 
     # import_outcome()
     def test_import_outcome_filepath(self, m):
-        import os
-
         register_uris({"course": ["import_outcome"]}, m)
 
         filepath = os.path.join("tests", "fixtures", "test_import_outcome.csv")
@@ -1598,8 +1631,6 @@ def test_import_outcome_filepath(self, m):
         self.assertEqual(outcome_import.data["import_type"], "instructure_csv")
 
     def test_import_outcome_binary(self, m):
-        import os
-
         register_uris({"course": ["import_outcome"]}, m)
 
         filepath = os.path.join("tests", "fixtures", "test_import_outcome.csv")