Merge pull request #5 from ucfopen/issue/1-python3

Issue/1 python3

.gitignore

@@ -110,6 +110,7 @@ celerybeat-schedule
 .env
 .venv
 env/
+env2/
 env3/
 venv/
 ENV/

lti.py

@@ -226,7 +226,9 @@ def index(lti=lti):
     )
 
     try:
-        tools_by_category = filter_tool_list(session["course_id"], session["api_key"])
+        tools_by_category, cagetory_order = filter_tool_list(
+            session["course_id"], session["api_key"]
+        )
     except CanvasException:
         app.logger.exception("Couldn't connect to Canvas")
         return return_error(
@@ -243,6 +245,7 @@ def index(lti=lti):
     return render_template(
         "main_template.html",
         tools_by_category=tools_by_category,
+        category_order=cagetory_order,
         course=session["course_id"],
     )
 
@@ -300,8 +303,7 @@ def status():
         app.logger.exception("Dev Key check failed.")
 
     # Overall health check - if all checks are True
-    status["healthy"] = all(v is True for k, v in status["checks"].items())
-
+    status["healthy"] = all(v is True for k, v in list(status["checks"].items()))
     return Response(json.dumps(status), mimetype="application/json")
 
 

requirements.txt

@@ -1,6 +1,6 @@
-canvasapi==0.14.0
+canvasapi==0.15.0
 Flask==1.1.1
-Flask-SQLAlchemy==2.4.0
-mysql-python==1.2.5
+Flask-SQLAlchemy==2.4.1
+mysqlclient
 -e git+https://github.com/ucfcdl/pylti.git@roles#egg=PyLTI
 requests==2.22.0

settings.py.template

@@ -41,8 +41,8 @@ oauth2_id = ""
 oauth2_key = ""
 
 # Logging configuration
-LOG_MAX_BYTES = 10000
-LOG_BACKUP_COUNT = 1
+LOG_MAX_BYTES = 1024 * 1024 * 5,  # 5 MB
+LOG_BACKUP_COUNT = 2
 ERROR_LOG = "logs/faculty-tools.log"
 
 whitelist = "whitelist.json"

setup.cfg

@@ -1,13 +1,17 @@
 [flake8]
 exclude=
+	venv*/*
 	env/*
+	env2/*
 	env3/*
 	src/*
 max_line_length=99
 ignore = W503, E203
 
 [coverage:run]
 omit=
+	venv*/*
 	env/*
+	env2/*
 	env3/*
 	src/*

templates/lti_list.html

@@ -44,9 +44,8 @@ <h3 class="card-title text-center">{{lti.display_name}}</h3>
 	{% endfor %}
 {% else %}
 	<div class="row">
-		<div class="col-md-8 ml-auto">
+		<div class="col-md-8 ml-auto mr-auto">
 			<p class="text-center" id="no_ltis">No LTIs available in this category.</p>
-			<hr />
 		</div>
 	</div>
 {% endif %}

templates/main_template.html

@@ -4,7 +4,7 @@
 <div class="container-fluid jump-to-section">
 	<h2>Jump to section:</h2>
 	<ul>
-		{% for category in tools_by_category %}
+		{% for category in category_order %}
 		<li><a class="btn btn-primary" href="#{{ category|slugify}}">{{ category }}</a></li>
 		{% endfor %}
 		{% include "extra_links.html" ignore missing %}
@@ -14,18 +14,16 @@ <h2>Jump to section:</h2>
 
 {% block content %}
 
-{% for category, tools in tools_by_category.items() %}
-<div class="row">
-	<div class="col-md-8 ml-auto mr-auto">
-		<h2 class="text-center section_headline" id="{{ category|slugify}}">{{ category }}</h2>
-		<hr />
+{% for category in category_order %}
+	<div class="row">
+		<div class="col-md-8 ml-auto mr-auto">
+			<h2 class="text-center section_headline" id="{{ category|slugify}}">{{ category }}</h2>
+			<hr />
+		</div>
 	</div>
-</div>
-
-{% with ltis=tools %}
-	{% include "lti_list.html" %}
-{% endwith %}
-
+	{% with ltis=tools_by_category[category] %}
+		{% include "lti_list.html" %}
+	{% endwith %}
 {% endfor %}
 
 {% include "custom_tools.html" ignore missing %}

tests.py

@@ -1,6 +1,7 @@
+from json.decoder import JSONDecodeError
 import logging
 import unittest
-from urllib import urlencode
+from urllib.parse import urlencode
 
 import canvasapi
 import oauthlib.oauth1
@@ -220,7 +221,7 @@ def test_index_no_auth(self, m):
         self.assert_template_used("error.html")
 
         self.assertIn(
-            "Authentication error, please refresh and try again", response.data
+            b"Authentication error, please refresh and try again", response.data
         )
 
     def test_index_api_key_none(self, m):
@@ -234,7 +235,7 @@ def test_index_api_key_none(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "Authentication error: missing API key. Please refresh and try again.",
+            b"Authentication error: missing API key. Please refresh and try again.",
             response.data,
         )
 
@@ -276,7 +277,7 @@ def test_index_api_key_invalid(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "You are not enrolled in this course as a Teacher or Designer.",
+            b"You are not enrolled in this course as a Teacher or Designer.",
             response.data,
         )
 
@@ -322,7 +323,7 @@ def test_index_no_canvas_conn(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "Couldn't connect to Canvas, please refresh and try again",
+            b"Couldn't connect to Canvas, please refresh and try again",
             response.data,
         )
 
@@ -366,7 +367,7 @@ def test_index_whitelist_error(self, m, filter_tool_list):
 
         self.assert_template_used("error.html")
         self.assertIn(
-            "There is something wrong with the whitelist.json file", response.data
+            b"There is something wrong with the whitelist.json file", response.data
         )
 
     @patch("lti.filter_tool_list")
@@ -410,7 +411,7 @@ def test_index_canvas_error(self, m, filter_tool_list):
         response = self.client.get(self.generate_launch_request(url_for("index")))
 
         self.assert_template_used("error.html")
-        self.assertIn("Couldn't connect to Canvas", response.data)
+        self.assertIn(b"Couldn't connect to Canvas", response.data)
 
     def test_index(self, m):
         with self.client.session_transaction() as sess:
@@ -530,7 +531,7 @@ def test_oauth_login_cancelled(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "Authentication error, please refresh and try again.", response.data
+            b"Authentication error, please refresh and try again.", response.data
         )
 
     def test_oauth_login_no_access_token(self, m):
@@ -552,7 +553,7 @@ def test_oauth_login_no_access_token(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "Authentication error, please refresh and try again.", response.data
+            b"Authentication error, please refresh and try again.", response.data
         )
 
     def test_oauth_login_new_user(self, m):
@@ -650,7 +651,7 @@ def test_oauth_login_new_user_db_error(self, m):
 
         self.assert_template_used("error.html")
         self.assertIn(
-            "Authentication error, please refresh and try again.", response.data
+            b"Authentication error, please refresh and try again.", response.data
         )
 
     def test_oauth_login_existing_user(self, m):
@@ -758,7 +759,7 @@ def test_oauth_login_existing_user_db_error(self, m):
             self.assert_200(response)
             self.assert_template_used("error.html")
             self.assertIn(
-                "Authentication error, please refresh and try again.", response.data
+                b"Authentication error, please refresh and try again.", response.data
             )
 
     # refresh_access_token
@@ -1149,7 +1150,7 @@ def test_get_sessionless_url_is_course_nav_fail(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "Error in a response from Canvas, please refresh and try again.",
+            b"Error in a response from Canvas, please refresh and try again.",
             response.data,
         )
 
@@ -1178,7 +1179,7 @@ def test_get_sessionless_url_is_course_nav_succeed(self, m):
         )
 
         self.assert_200(response)
-        self.assertEqual(response.data, launch_url)
+        self.assertEqual(response.data, launch_url.encode("utf-8"))
 
     def test_get_sessionless_url_not_course_nav_fail(self, m):
         with self.client.session_transaction() as sess:
@@ -1204,7 +1205,7 @@ def test_get_sessionless_url_not_course_nav_fail(self, m):
         self.assert_200(response)
         self.assert_template_used("error.html")
         self.assertIn(
-            "Error in a response from Canvas, please refresh and try again.",
+            b"Error in a response from Canvas, please refresh and try again.",
             response.data,
         )
 
@@ -1233,7 +1234,7 @@ def test_get_sessionless_url_not_course_nav_succeed(self, m):
         )
 
         self.assert_200(response)
-        self.assertEqual(response.data, launch_url)
+        self.assertEqual(response.data, launch_url.encode("utf-8"))
 
 
 class UtilsTests(unittest.TestCase):
@@ -1243,13 +1244,13 @@ def setUpClass(cls):
         settings.whitelist = "whitelist.json"
 
     def test_filter_tool_list_empty_file(self):
-        with self.assertRaisesRegexp(ValueError, r"No JSON object could be decoded"):
-            with patch("__builtin__.open", mock_open(read_data="")):
+        with self.assertRaises(JSONDecodeError):
+            with patch("builtins.open", mock_open(read_data="")):
                 utils.filter_tool_list(1, "password")
 
     def test_filter_tool_list_empty_data(self):
-        with self.assertRaisesRegexp(ValueError, r"whitelist\.json is empty"):
-            with patch("__builtin__.open", mock_open(read_data="{}")):
+        with self.assertRaisesRegex(ValueError, r"whitelist\.json is empty"):
+            with patch("builtins.open", mock_open(read_data="{}")):
                 utils.filter_tool_list(1, "password")
 
     @patch("canvasapi.canvas.Canvas.get_course")

utils.py

@@ -7,6 +7,23 @@
 import settings
 
 
+def get_tool_info(whitelist, tool_name):
+    """
+    Search the whitelist by tool name.
+
+    :returns: A dictionary , or none if no tool matching that name was found.
+    :rtype: dict or None
+    """
+    for category, category_tools in whitelist.items():
+        for tool_info in category_tools:
+            print(tool_info)
+            if tool_info.get("name") == tool_name:
+                tool_info.update({"category": category})
+                return tool_info
+
+    return None
+
+
 def filter_tool_list(course_id, access_token):
     """
     Filter tool list down to those on whitelist and sort by category.
@@ -31,38 +48,45 @@ def filter_tool_list(course_id, access_token):
 
     course = canvas.get_course(course_id)
     installed_tools = course.get_external_tools(include_parents=True)
-
     tools_by_category = defaultdict(list)
     for installed_tool in installed_tools:
-        for tool in whitelist:
-            if installed_tool.name != tool.get("name"):
-                continue
-
-            is_course_navigation = hasattr(installed_tool, "course_navigation")
-
-            if tool.get("is_launchable", False):
-                if is_course_navigation:
-                    sessionless_launch_url = installed_tool.get_sessionless_launch_url(
-                        launch_type="course_navigation"
-                    )
-                else:
-                    sessionless_launch_url = installed_tool.get_sessionless_launch_url()
-            else:
-                sessionless_launch_url = None
+        tool_info = get_tool_info(whitelist, installed_tool.name)
+        if not tool_info:
+            continue
 
-            tool_info = tool
-            tool_info.update(
-                {
-                    "id": installed_tool.id,
-                    "lti_course_navigation": is_course_navigation,
-                    "sessionless_launch_url": sessionless_launch_url,
-                    "screenshot": "screenshots/" + tool["screenshot"],
-                }
-            )
+        is_course_navigation = hasattr(installed_tool, "course_navigation")
 
-            tools_by_category[tool.get("category", "Uncategorized")].append(tool_info)
-
-    return tools_by_category
+        if tool_info.get("is_launchable", False):
+            if is_course_navigation:
+                sessionless_launch_url = installed_tool.get_sessionless_launch_url(
+                    launch_type="course_navigation"
+                )
+            else:
+                sessionless_launch_url = installed_tool.get_sessionless_launch_url()
+        else:
+            sessionless_launch_url = None
+
+        tool_info.update(
+            {
+                "id": installed_tool.id,
+                "lti_course_navigation": is_course_navigation,
+                "sessionless_launch_url": sessionless_launch_url,
+                "screenshot": "screenshots/" + tool_info["screenshot"],
+            }
+        )
+
+        tools_by_category[tool_info.get("category", "Uncategorized")].append(tool_info)
+
+    for category, tools in tools_by_category.items():
+        # Determine tool order based on order in whitelist
+        order = {
+            tool.get("display_name"): i for i, tool in enumerate(whitelist[category])
+        }
+        tools_by_category[category] = sorted(
+            tools, key=lambda k: order[k["display_name"]]
+        )
+
+    return tools_by_category, list(whitelist.keys())
 
 
 def slugify(value):