Repository: ucfopen/canvasapi

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:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/ucfopen/canvasapi/pull/622?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen) Report
> Merging [#622](https://app.codecov.io/gh/ucfopen/canvasapi/pull/622?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen) (9c98efe) into [develop](https://app.codecov.io/gh/ucfopen/canvasapi/commit/44c61caf161fca3ed68972d02881fd7c807d61ce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen) (44c61ca) will **decrease** coverage by `0.09%`.
> The diff coverage is `75.00%`.

```diff
@@             Coverage Diff             @@
##           develop     #622      +/-   ##
===========================================
- Coverage   100.00%   99.91%   -0.09%     
===========================================
  Files           73       73              
  Lines         3726     3732       +6     
===========================================
+ Hits          3726     3729       +3     
- Misses           0        3       +3     
```


| [Impacted Files](https://app.codecov.io/gh/ucfopen/canvasapi/pull/622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen) | Coverage Δ | |
|---|---|---|
| [canvasapi/course.py](https://app.codecov.io/gh/ucfopen/canvasapi/pull/622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen#diff-Y2FudmFzYXBpL2NvdXJzZS5weQ==) | `99.47% <72.72%> (-0.53%)` | :arrow_down: |
| [canvasapi/requester.py](https://app.codecov.io/gh/ucfopen/canvasapi/pull/622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen#diff-Y2FudmFzYXBpL3JlcXVlc3Rlci5weQ==) | `100.00% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ucfopen)

Thetwam: > Do tests need to be updated to check cases where the file isn't valid?

Good call, updated.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cff7841..1a1aad4 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 5c2bb0d..21bde43 100644
--- a/canvasapi/course.py
+++ b/canvasapi/course.py
@@ -279,25 +279,37 @@ class Course(CanvasObject):
 
         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 b1b8745..bda2c99 100644
--- a/canvasapi/requester.py
+++ b/canvasapi/requester.py
@@ -98,7 +98,7 @@ class Requester(object):
         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 0000000..d333441
--- /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 214bafb..2e2669d 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 @@ class TestCourse(unittest.TestCase):
             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 @@ class TestCourse(unittest.TestCase):
         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 @@ class TestCourse(unittest.TestCase):
 
     # 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 @@ class TestCourse(unittest.TestCase):
         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")