[Student][Teacher][MBL-13227] Fix routing for multi-shard courses (#314)

instructure · Sep 13, 2019 · fff541d · fff541d
1 parent 914aba1
commit fff541d
Show file tree

Hide file tree

Showing 9 changed files with 193 additions and 6 deletions.
diff --git a/apps/student/src/androidTest/java/com/instructure/student/ui/e2e/ShardE2ETest.kt b/apps/student/src/androidTest/java/com/instructure/student/ui/e2e/ShardE2ETest.kt
@@ -0,0 +1,23 @@
+package com.instructure.student.ui.e2e
+
+import com.instructure.canvas.espresso.E2E
+import com.instructure.panda_annotations.FeatureCategory
+import com.instructure.panda_annotations.Priority
+import com.instructure.panda_annotations.TestCategory
+import com.instructure.panda_annotations.TestMetaData
+import com.instructure.student.ui.utils.StudentTest
+import org.junit.Test
+
+class ShardE2ETest: StudentTest() {
+    override fun displaysPageObjects() {
+        TODO("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    @E2E
+    @Test
+    @TestMetaData(Priority.P0, FeatureCategory.NONE, TestCategory.E2E, true)
+    fun testShardE2E() {
+        // TODO: Test against institutions across multiple shards, for courses/assignments/etc... that have cross shard ids
+        //  (i.e., 12345~1234 instead of 1234500000000001234)
+    }
+}
diff --git a/apps/student/src/main/java/com/instructure/student/util/ModuleUtility.java b/apps/student/src/main/java/com/instructure/student/util/ModuleUtility.java
@@ -23,6 +23,7 @@
 import com.instructure.canvasapi2.models.Course;
 import com.instructure.canvasapi2.models.ModuleItem;
 import com.instructure.canvasapi2.models.ModuleObject;
+import com.instructure.canvasapi2.utils.APIHelper;
 import com.instructure.interactions.router.Route;
 import com.instructure.student.fragment.DiscussionDetailsFragment;
 import com.instructure.student.fragment.FileDetailsFragment;
@@ -212,9 +213,9 @@ private static long getIdFromUrl(String url, int index) {
         long assignmentId;
         int endIndex = url.indexOf("/", index);
         if(endIndex != -1) {
-            assignmentId = Long.parseLong(url.substring(index, endIndex));
+            assignmentId = Long.parseLong(APIHelper.INSTANCE.expandTildeId(url.substring(index, endIndex)));
         } else {
-            assignmentId = Long.parseLong(url.substring(index));
+            assignmentId = Long.parseLong(APIHelper.INSTANCE.expandTildeId(url.substring(index)));
         }
         return assignmentId;
     }

diff --git a/apps/student/src/test/java/com/instructure/student/test/util/ModuleUtilityTest.kt b/apps/student/src/test/java/com/instructure/student/test/util/ModuleUtilityTest.kt
@@ -114,6 +114,46 @@ class ModuleUtilityTest : TestCase() {
         TestCase.assertEquals(expectedBundle.toString(), parentFragment.arguments!!.toString())
     }
 
+    @Test
+    fun testGetFragment_assignmentShardId() {
+        val url = "https://mobile.canvas.net/api/v1/courses/222/assignments/12345~6789"
+        val moduleItem = ModuleItem(
+                id = 4567,
+                type = "Assignment",
+                url = url
+        )
+
+        val course = Course()
+        val expectedBundle = Bundle()
+        expectedBundle.putParcelable(Const.CANVAS_CONTEXT, course)
+        expectedBundle.putLong(Const.ASSIGNMENT_ID, 123450000000006789)
+
+        val parentFragment = callGetFragment(moduleItem, course, null)
+        TestCase.assertNotNull(parentFragment)
+        TestCase.assertEquals(AssignmentDetailsFragment::class.java, parentFragment!!.javaClass)
+        TestCase.assertEquals(expectedBundle.toString(), parentFragment.arguments!!.toString())
+    }
+
+    @Test
+    fun testGetFragment_assignmentSubmissionShardId() {
+        val url = "https://mobile.canvas.net/api/v1/courses/222/assignments/12345~6789/submissions"
+        val moduleItem = ModuleItem(
+                id = 4567,
+                type = "Assignment",
+                url = url
+        )
+
+        val course = Course()
+        val expectedBundle = Bundle()
+        expectedBundle.putParcelable(Const.CANVAS_CONTEXT, course)
+        expectedBundle.putLong(Const.ASSIGNMENT_ID, 123450000000006789)
+
+        val parentFragment = callGetFragment(moduleItem, course, null)
+        TestCase.assertNotNull(parentFragment)
+        TestCase.assertEquals(AssignmentDetailsFragment::class.java, parentFragment!!.javaClass)
+        TestCase.assertEquals(expectedBundle.toString(), parentFragment.arguments!!.toString())
+    }
+
     @Test
     fun testGetFragment_externalurl_externaltool() {
         val url = "https://instructure.com"

diff --git a/apps/teacher/src/test/java/com/instructure/teacher/unit/RouteTest.kt b/apps/teacher/src/test/java/com/instructure/teacher/unit/RouteTest.kt
@@ -123,6 +123,38 @@ class RouteTest : Assert() {
         Assert.assertEquals("module_item_id=6723096", route.getQueryString())
     }
 
+    @Test
+    fun testRouteQueryParams_shardCourse() {
+        val expectedParams = HashMap<String, String>()
+        expectedParams["course_id"] = "123450000000004321"
+
+        val route = Route("/courses/:course_id")
+        Assert.assertTrue(route.apply("https://mobiledev.instructure.com/courses/12345~4321"))
+        Assert.assertEquals(expectedParams, route.paramsHash)
+    }
+
+    @Test
+    fun testRouteQueryParams_shardCourseAssignment() {
+        val expectedParams = HashMap<String, String>()
+        expectedParams["course_id"] = "123450000000004321"
+        expectedParams["assignment_id"] = "123450000000004321"
+
+        val route = Route("/courses/:course_id/assignments/:assignment_id")
+        Assert.assertTrue(route.apply("https://mobiledev.instructure.com/courses/12345~4321/assignments/12345~4321"))
+        Assert.assertEquals(expectedParams, route.paramsHash)
+    }
+
+    @Test
+    fun testRouteQueryParams_shardPageId() {
+        val expectedParams = HashMap<String, String>()
+        expectedParams["course_id"] = "1234"
+        expectedParams["page_id"] = "12345~4321"
+
+        val route = Route("/courses/:course_id/pages/:page_id")
+        Assert.assertTrue(route.apply("https://mobiledev.instructure.com/courses/1234/pages/12345~4321"))
+        Assert.assertEquals(expectedParams, route.paramsHash)
+    }
+
     //endregion
 
     /*

diff --git a/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/models/StreamItem.kt b/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/models/StreamItem.kt
@@ -20,6 +20,7 @@ package com.instructure.canvasapi2.models
 import android.content.Context
 import com.google.gson.annotations.SerializedName
 import com.instructure.canvasapi2.R
+import com.instructure.canvasapi2.utils.APIHelper
 import com.instructure.canvasapi2.utils.toDate
 import kotlinx.android.parcel.IgnoredOnParcel
 import kotlinx.android.parcel.Parcelize
@@ -267,7 +268,7 @@ data class StreamItem(
             }
             val assignmentId = htmlUrl.substring(start, end)
 
-            return assignmentId.toLong()
+            return APIHelper.expandTildeId(assignmentId).toLong()
         }
         return 0
     }
@@ -284,7 +285,7 @@ data class StreamItem(
 
             val courseIdString = htmlUrl.substring(start, end)
 
-            return courseIdString.toLong()
+            return APIHelper.expandTildeId(courseIdString).toLong()
         }
         return 0
     }
@@ -301,7 +302,7 @@ data class StreamItem(
 
             val groupIdString = htmlUrl.substring(start, end)
 
-            return groupIdString.toLong()
+            return APIHelper.expandTildeId(groupIdString).toLong()
         }
         return 0
     }

diff --git a/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/utils/APIHelper.kt b/libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/utils/APIHelper.kt
@@ -205,4 +205,20 @@ object APIHelper {
         return ApiPrefs.protocol + "://" + ApiPrefs.domain + "/courses/" + courseid + "/quizzes/" + quizId
     }
 
+    /**
+     * Parse an ID that references a shard, replaces "~" with the appropriate 0 padding
+     * i.e., converts a sharded ID '12345~4321' to 123450000000004321
+     *
+     * @param id the ID to convert into a long
+     */
+    fun expandTildeId(id: String): String {
+
+        return if (id.contains("~")) {
+            val parts = id.split("~".toRegex())
+            ((parts[0].toLong() * 10_000_000_000_000L) + parts[1].toLong()).toString()
+        } else {
+            id
+        }
+    }
+
 }
diff --git a/libs/canvas-api-2/src/test/java/com/instructure/canvasapi2/unit/APIHelperTest.kt b/libs/canvas-api-2/src/test/java/com/instructure/canvasapi2/unit/APIHelperTest.kt
@@ -180,4 +180,27 @@ class APIHelperTest {
         assertNotNull(APIHelper.paramsWithDomain(domain, params))
     }
 
+    @Test
+    fun expandTildeId() {
+        val expected = "123450000000001234"
+        val actual = "12345~1234"
+
+        assertEquals(expected, APIHelper.expandTildeId(actual))
+    }
+
+    @Test
+    fun expandTildeIdLongerId() {
+        val expected = "107920000001139989"
+        val actual = "10792~1139989"
+
+        assertEquals(expected, APIHelper.expandTildeId(actual))
+    }
+
+    @Test
+    fun expandTildeIdShortShard() {
+        val expected = "30000012771174"
+        val actual = "3~12771174"
+
+        assertEquals(expected, APIHelper.expandTildeId(actual))
+    }
 }
diff --git a/libs/canvas-api-2/src/test/java/com/instructure/canvasapi2/unit/StreamUnitTest.kt b/libs/canvas-api-2/src/test/java/com/instructure/canvasapi2/unit/StreamUnitTest.kt
@@ -47,6 +47,22 @@ class StreamUnitTest : Assert() {
         Assert.assertTrue(streamItem.id > 0)
     }
 
+    @Test
+    fun testStreamItemCourseId() {
+        val streamItem: StreamItem = courseShardStreamItemJSON.parse()
+
+        Assert.assertNotNull(streamItem)
+        Assert.assertEquals(836350000000001123L, streamItem.courseId)
+    }
+
+    @Test
+    fun testStreamItemAssignmentId() {
+        val streamItem: StreamItem = courseShardStreamItemJSON.parse()
+
+        Assert.assertNotNull(streamItem)
+        Assert.assertEquals(983440000000001212L, streamItem.assignmentId)
+    }
+
     /**
      * personal stream
      * @GET("/users/self/activity_stream")
@@ -212,4 +228,19 @@ class StreamUnitTest : Assert() {
         "root_discussion_entries": []
       }"""
 
+    @Language("JSON")
+    private var courseShardStreamItemJSON = """
+      {
+        "created_at": "2015-02-23T23:41:16Z",
+        "updated_at": "2015-02-23T23:41:16Z",
+        "id": 129486849,
+        "title": "Assignment Created",
+        "message": "hasjdf;lk alksjdfa;k sfal;jdflaksjdflas f;ljaslf kajsfl;ajsf",
+        "type": "Message",
+        "read_state": false,
+        "context_type": "Course",
+        "course_id": -1,
+        "html_url": "https://mobiledev.instructure.com/courses/83635~1123/assignments/98344~1212"
+      }"""
+
 }
diff --git a/libs/interactions/src/main/java/com/instructure/interactions/router/Route.kt b/libs/interactions/src/main/java/com/instructure/interactions/router/Route.kt
@@ -21,6 +21,7 @@ import android.os.Parcel
 import android.os.Parcelable
 import androidx.fragment.app.Fragment
 import com.instructure.canvasapi2.models.CanvasContext
+import com.instructure.canvasapi2.utils.APIHelper
 import kotlinx.android.parcel.Parceler
 import kotlinx.android.parcel.Parcelize
 import kotlinx.android.parcel.WriteWith
@@ -221,7 +222,13 @@ data class Route(
         }
         paramNames.indices
                 .filter { it < paramValues.size }
-                .forEach { params[paramNames[it]] = paramValues[it] }
+                .forEach {
+                    params[paramNames[it]] = if (paramNames[it] in IDS_TO_EXPAND_TILDE) {
+                         APIHelper.expandTildeId(paramValues[it])
+                    } else {
+                        paramValues[it]
+                    }
+                }
 
         return params
     }
@@ -280,6 +287,19 @@ data class Route(
 
     companion object {
         const val ROUTE = "route2.0"
+        val IDS_TO_EXPAND_TILDE = listOf(
+            RouterParams.ASSIGNMENT_ID,
+            RouterParams.COURSE_ID,
+            RouterParams.CONVERSATION_ID,
+            RouterParams.MODULE_ITEM_ID,
+            RouterParams.MODULE_ID,
+            RouterParams.QUIZ_ID,
+            RouterParams.MESSAGE_ID,
+            RouterParams.FILE_ID,
+            RouterParams.USER_ID,
+            RouterParams.EVENT_ID,
+            RouterParams.SUBMISSION_ID
+        )
 
         @JvmStatic
         fun extractCourseId(route: Route?): Long {