Skip to content

Commit 9f41e62

Browse files
kristofnemerekristofnemere
authored
[MBL-19515][Student] Fix cross-shard user submissions
refs: MBL-19515 affects: Student release note: Fixed errors when viewing submissions in courses across different Canvas shards.
1 parent bf39849 commit 9f41e62

File tree

9 files changed

+291
-17
lines changed

9 files changed

+291
-17
lines changed

‎apps/student/src/main/java/com/instructure/student/mobius/assignmentDetails/submissionDetails/SubmissionDetailsEffectHandler.kt‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package com.instructure.student.mobius.assignmentDetails.submissionDetails
1919

2020
import com.instructure.canvasapi2.models.Assignment
21+
import com.instructure.canvasapi2.utils.APIHelper
2122
import com.instructure.canvasapi2.utils.ApiPrefs
2223
import com.instructure.canvasapi2.utils.DataResult
2324
import com.instructure.canvasapi2.utils.exhaustive
@@ -32,7 +33,8 @@ import kotlinx.coroutines.launch
3233
import java.io.File
3334

3435
class SubmissionDetailsEffectHandler(
35-
private val repository: SubmissionDetailsRepository
36+
private val repository: SubmissionDetailsRepository,
37+
private val apiPrefs: ApiPrefs
3638
) : EffectHandler<SubmissionDetailsView, SubmissionDetailsEvent, SubmissionDetailsEffect>() {
3739

3840
override fun accept(effect: SubmissionDetailsEffect) {
@@ -70,9 +72,16 @@ class SubmissionDetailsEffectHandler(
7072
// If the user is an observer, get the id of the first observee that comes back, otherwise use the user's id
7173
val enrollments = repository.getObserveeEnrollments(true).dataOrNull.orEmpty()
7274
val observeeId = enrollments.firstOrNull { it.isObserver && it.courseId == effect.courseId }?.associatedUserId
73-
val userId = observeeId ?: ApiPrefs.user!!.id
75+
val userId = observeeId ?: apiPrefs.user!!.id
7476

75-
val submissionResult = repository.getSingleSubmission(effect.courseId, effect.assignmentId, userId, true)
77+
val finalUserId = APIHelper.getUserIdForCourse(
78+
effect.courseId,
79+
userId,
80+
apiPrefs.shardIds,
81+
apiPrefs.accessToken
82+
)
83+
84+
val submissionResult = repository.getSingleSubmission(effect.courseId, effect.assignmentId, finalUserId, true)
7685
val assignmentResult = repository.getAssignment(effect.assignmentId, effect.courseId, true)
7786

7887
val studioLTIToolResult = if (repository.isOnline() && assignmentResult.containsSubmissionType(Assignment.SubmissionType.ONLINE_UPLOAD)) {

‎apps/student/src/main/java/com/instructure/student/mobius/assignmentDetails/submissionDetails/content/DiscussionSubmissionViewFragment.kt‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,11 @@ import android.view.View
2222
import android.view.ViewGroup
2323
import android.webkit.WebChromeClient
2424
import android.webkit.WebView
25-
import com.instructure.pandautils.base.BaseCanvasFragment
2625
import com.instructure.canvasapi2.managers.OAuthManager
27-
import com.instructure.canvasapi2.models.AuthenticatedSession
2826
import com.instructure.canvasapi2.utils.ApiPrefs
2927
import com.instructure.canvasapi2.utils.weave.StatusCallbackError
3028
import com.instructure.canvasapi2.utils.weave.awaitApi
29+
import com.instructure.pandautils.base.BaseCanvasFragment
3130
import com.instructure.pandautils.binding.viewBinding
3231
import com.instructure.pandautils.utils.StringArg
3332
import com.instructure.pandautils.utils.enableAlgorithmicDarkening
@@ -50,6 +49,29 @@ class DiscussionSubmissionViewFragment : BaseCanvasFragment() {
5049
private var discussionUrl: String by StringArg()
5150
private var authJob: Job? = null
5251

52+
/**
53+
* Check if a URL belongs to any of the valid Canvas domains (main domain or override domains)
54+
*/
55+
private fun isValidCanvasDomain(url: String): Boolean {
56+
if (url.contains(ApiPrefs.domain)) return true
57+
return ApiPrefs.overrideDomains.values.any { overrideDomain ->
58+
!overrideDomain.isNullOrEmpty() && url.contains(overrideDomain)
59+
}
60+
}
61+
62+
/**
63+
* Get the appropriate domain for a given URL (main domain or matching override domain)
64+
*/
65+
private fun getDomainForUrl(url: String): String {
66+
if (url.contains(ApiPrefs.domain)) return ApiPrefs.domain
67+
ApiPrefs.overrideDomains.values.forEach { overrideDomain ->
68+
if (!overrideDomain.isNullOrEmpty() && url.contains(overrideDomain)) {
69+
return overrideDomain
70+
}
71+
}
72+
return ApiPrefs.domain
73+
}
74+
5375
override fun onCreateView(
5476
inflater: LayoutInflater,
5577
container: ViewGroup?,
@@ -86,12 +108,12 @@ class DiscussionSubmissionViewFragment : BaseCanvasFragment() {
86108
(url != discussionUrl && !url.contains("root_discussion_topic_id")) && RouteMatcher.canRouteInternally(
87109
requireActivity(),
88110
url,
89-
ApiPrefs.domain,
111+
getDomainForUrl(url),
90112
false
91113
)
92114

93115
override fun routeInternallyCallback(url: String) {
94-
RouteMatcher.canRouteInternally(requireActivity(), url, ApiPrefs.domain, true)
116+
RouteMatcher.canRouteInternally(requireActivity(), url, getDomainForUrl(url), true)
95117
}
96118
}
97119

@@ -103,18 +125,19 @@ class DiscussionSubmissionViewFragment : BaseCanvasFragment() {
103125
)
104126

105127
override fun shouldLaunchInternalWebViewFragment(url: String): Boolean =
106-
!url.contains(ApiPrefs.domain)
128+
!isValidCanvasDomain(url)
107129
}
108130

109131
binding.discussionSubmissionWebView.setInitialScale(100)
110132

111133
authJob = GlobalScope.launch(Dispatchers.Main) {
112-
val authenticatedUrl = if (ApiPrefs.domain in discussionUrl)
134+
val authenticatedUrl = if (isValidCanvasDomain(discussionUrl))
113135
try {
114-
awaitApi<AuthenticatedSession> {
136+
awaitApi {
115137
OAuthManager.getAuthenticatedSession(
116138
discussionUrl,
117-
it
139+
it,
140+
getDomainForUrl(discussionUrl).takeIf { it != ApiPrefs.domain }
118141
)
119142
}.sessionUrl
120143
} catch (e: StatusCallbackError) {

‎apps/student/src/main/java/com/instructure/student/mobius/assignmentDetails/submissionDetails/ui/SubmissionDetailsFragment.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package com.instructure.student.mobius.assignmentDetails.submissionDetails.ui
1919
import android.view.LayoutInflater
2020
import android.view.ViewGroup
2121
import com.instructure.canvasapi2.models.Course
22+
import com.instructure.canvasapi2.utils.ApiPrefs
2223
import com.instructure.canvasapi2.utils.pageview.PageViewUrlParam
2324
import com.instructure.pandautils.utils.BooleanArg
2425
import com.instructure.pandautils.utils.Const
@@ -45,7 +46,7 @@ abstract class SubmissionDetailsFragment : MobiusFragment<SubmissionDetailsModel
4546
val isObserver by BooleanArg(key = Const.IS_OBSERVER, default = false)
4647
private val initialSelectedSubmissionAttempt by LongArg(key = Const.SUBMISSION_ATTEMPT)
4748

48-
override fun makeEffectHandler() = SubmissionDetailsEffectHandler(getRepository())
49+
override fun makeEffectHandler() = SubmissionDetailsEffectHandler(getRepository(), getApiPreferences())
4950

5051
override fun makeUpdate() = SubmissionDetailsUpdate()
5152

@@ -62,4 +63,6 @@ abstract class SubmissionDetailsFragment : MobiusFragment<SubmissionDetailsModel
6263
)
6364

6465
abstract fun getRepository(): SubmissionDetailsRepository
66+
67+
abstract fun getApiPreferences(): ApiPrefs
6568
}

‎apps/student/src/main/java/com/instructure/student/mobius/assignmentDetails/submissionDetails/ui/SubmissionDetailsRepositoryFragment.kt‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import com.instructure.interactions.router.Route
2525
import com.instructure.interactions.router.RouterParams
2626
import com.instructure.pandautils.analytics.SCREEN_VIEW_SUBMISSION_DETAILS
2727
import com.instructure.pandautils.analytics.ScreenView
28+
import com.instructure.pandautils.room.studentdb.StudentDb
29+
import com.instructure.pandautils.room.studentdb.entities.CreateSubmissionEntity
2830
import com.instructure.pandautils.utils.Const
2931
import com.instructure.pandautils.utils.makeBundle
3032
import com.instructure.pandautils.utils.withArgs
@@ -33,8 +35,6 @@ import com.instructure.student.mobius.assignmentDetails.submissionDetails.Submis
3335
import com.instructure.student.mobius.assignmentDetails.submissionDetails.SubmissionDetailsSharedEvent
3436
import com.instructure.student.mobius.common.FlowSource
3537
import com.instructure.student.mobius.common.LiveDataSource
36-
import com.instructure.pandautils.room.studentdb.StudentDb
37-
import com.instructure.pandautils.room.studentdb.entities.CreateSubmissionEntity
3838
import dagger.hilt.android.AndroidEntryPoint
3939
import javax.inject.Inject
4040

@@ -49,8 +49,13 @@ class SubmissionDetailsRepositoryFragment : SubmissionDetailsFragment() {
4949
@Inject
5050
lateinit var studentDb: StudentDb
5151

52+
@Inject
53+
lateinit var apiPrefs: ApiPrefs
54+
5255
override fun getRepository() = submissionDetailsRepository
5356

57+
override fun getApiPreferences() = apiPrefs
58+
5459
override fun onCreate(savedInstanceState: Bundle?) {
5560
super.onCreate(savedInstanceState)
5661
retainInstance = false

‎apps/student/src/main/java/com/instructure/student/router/EnabledTabs.kt‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import com.instructure.canvasapi2.models.Tab
88
import com.instructure.canvasapi2.utils.ApiPrefs
99
import com.instructure.canvasapi2.utils.depaginate
1010
import com.instructure.interactions.router.Route
11+
import androidx.core.net.toUri
1112

1213
interface EnabledTabs {
1314
fun isPathTabNotEnabled(route: Route?): Boolean
@@ -66,8 +67,22 @@ class EnabledTabsImpl: EnabledTabs {
6667
api.next(it, RestParams(usePerPageQueryParam = true))
6768
}.dataOrNull?.associate { it.id to (it.tabs ?: emptyList()) } ?: emptyMap()
6869
enabledTabs?.forEach { entry ->
69-
entry.value.find { tab -> tab.tabId == Tab.ASSIGNMENTS_ID }?.domain?.let { domain ->
70-
ApiPrefs.overrideDomains[entry.key] = domain
70+
entry.value.find { tab -> tab.tabId == Tab.ASSIGNMENTS_ID }?.let { tab ->
71+
tab.domain?.let { domain ->
72+
ApiPrefs.overrideDomains[entry.key] = domain
73+
}
74+
75+
// Parse shard ID from full_url if it contains a tilde
76+
// Example: "https://example.com/courses/7053~2848/assignments" -> "7053"
77+
tab.externalUrl?.let { externalUrl ->
78+
val uri = externalUrl.toUri()
79+
val courseIdIndex = uri.pathSegments.indexOf("courses") + 1
80+
val courseIdSegment = uri.pathSegments.getOrNull(courseIdIndex) ?: return@let
81+
if (courseIdSegment.contains("~")) {
82+
val shardId = courseIdSegment.substringBefore("~")
83+
ApiPrefs.shardIds[entry.key] = shardId
84+
}
85+
}
7186
}
7287
}
7388
}

‎apps/student/src/test/java/com/instructure/student/test/assignment/details/submissionDetails/SubmissionDetailsEffectHandlerTest.kt‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import com.instructure.student.mobius.assignmentDetails.submissionDetails.ui.Sub
3939
import com.instructure.student.mobius.common.FlowSource
4040
import com.spotify.mobius.functions.Consumer
4141
import io.mockk.coEvery
42+
import io.mockk.coVerify
4243
import io.mockk.confirmVerified
4344
import io.mockk.every
4445
import io.mockk.mockk
@@ -62,7 +63,8 @@ import java.io.File
6263
class SubmissionDetailsEffectHandlerTest : Assert() {
6364
private val view: SubmissionDetailsView = mockk(relaxed = true)
6465
private val repository: SubmissionDetailsRepository = mockk(relaxed = true)
65-
private val effectHandler = SubmissionDetailsEffectHandler(repository).apply { view = this@SubmissionDetailsEffectHandlerTest.view }
66+
private val apiPrefs: ApiPrefs = mockk(relaxed = true)
67+
private val effectHandler = SubmissionDetailsEffectHandler(repository, apiPrefs).apply { view = this@SubmissionDetailsEffectHandlerTest.view }
6668
private val eventConsumer: Consumer<SubmissionDetailsEvent> = mockk(relaxed = true)
6769
private val connection = effectHandler.connect(eventConsumer)
6870
private val testDispatcher = UnconfinedTestDispatcher()
@@ -492,4 +494,61 @@ class SubmissionDetailsEffectHandlerTest : Assert() {
492494
assertEquals(expectedEvent, deferred.await())
493495
}
494496

497+
@Test
498+
fun `loadData uses global user ID for cross-shard course`() {
499+
val courseId = 1234L
500+
val userId = 5678L
501+
val courseShardId = "7053"
502+
val tokenShardId = "8000"
503+
val expectedGlobalUserId = 80000000000005678L
504+
505+
val assignment = Assignment().copy(submissionTypesRaw = listOf(Assignment.SubmissionType.ONLINE_QUIZ.apiString))
506+
val submission = Submission()
507+
val user = User(id = userId)
508+
509+
// Mock apiPrefs to return different shard IDs
510+
every { apiPrefs.user } returns user
511+
every { apiPrefs.shardIds } returns mutableMapOf(courseId to courseShardId)
512+
every { apiPrefs.accessToken } returns "$tokenShardId~abcdef1234567890"
513+
514+
coEvery { repository.getObserveeEnrollments(any()) } returns DataResult.Success(listOf())
515+
coEvery { repository.getAssignment(any(), any(), any()) } returns DataResult.Success(assignment)
516+
coEvery { repository.getSingleSubmission(any(), any(), any(), any()) } returns DataResult.Success(submission)
517+
coEvery { repository.getCourseFeatures(any(), any()) } returns DataResult.Success(listOf("assignments_2_student"))
518+
519+
connection.accept(SubmissionDetailsEffect.LoadData(courseId, assignment.id))
520+
521+
// Verify that getSingleSubmission was called with the global user ID
522+
coVerify(timeout = 100) {
523+
repository.getSingleSubmission(courseId, assignment.id, expectedGlobalUserId, true)
524+
}
525+
}
526+
527+
@Test
528+
fun `loadData uses original user ID for same-shard course`() {
529+
val courseId = 1234L
530+
val userId = 5678L
531+
val shardId = "7053"
532+
533+
val assignment = Assignment().copy(submissionTypesRaw = listOf(Assignment.SubmissionType.ONLINE_QUIZ.apiString))
534+
val submission = Submission()
535+
val user = User(id = userId)
536+
537+
// Mock apiPrefs to return same shard ID
538+
every { apiPrefs.user } returns user
539+
every { apiPrefs.shardIds } returns mutableMapOf(courseId to shardId)
540+
every { apiPrefs.accessToken } returns "$shardId~abcdef1234567890"
541+
542+
coEvery { repository.getObserveeEnrollments(any()) } returns DataResult.Success(listOf())
543+
coEvery { repository.getAssignment(any(), any(), any()) } returns DataResult.Success(assignment)
544+
coEvery { repository.getSingleSubmission(any(), any(), any(), any()) } returns DataResult.Success(submission)
545+
coEvery { repository.getCourseFeatures(any(), any()) } returns DataResult.Success(listOf("assignments_2_student"))
546+
547+
connection.accept(SubmissionDetailsEffect.LoadData(courseId, assignment.id))
548+
549+
// Verify that getSingleSubmission was called with the original user ID (not converted)
550+
coVerify(timeout = 100) {
551+
repository.getSingleSubmission(courseId, assignment.id, userId, true)
552+
}
553+
}
495554
}

‎libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/utils/APIHelper.kt‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,4 +229,57 @@ object APIHelper {
229229
}
230230
}
231231

232+
/**
233+
* Extract shard ID from Canvas access token if it contains one
234+
* Canvas tokens can be in the format: shardId~token
235+
*
236+
* @param token the access token to parse
237+
* @return the shard ID if present, null otherwise
238+
*/
239+
fun getShardIdFromToken(token: String): String? {
240+
return if (token.contains("~")) {
241+
token.substringBefore("~")
242+
} else {
243+
null
244+
}
245+
}
246+
247+
/**
248+
* Create a global user ID from a shard ID and user ID, then expand it
249+
* i.e., converts shardId "7053" and userId 2848 to 70530000000002848
250+
*
251+
* @param shardId the shard ID
252+
* @param userId the user ID
253+
* @return the expanded global user ID as a Long
254+
*/
255+
fun createGlobalUserId(shardId: String, userId: Long): Long {
256+
val tildeId = "$shardId~$userId"
257+
return expandTildeId(tildeId).toLongOrNull()
258+
?: throw IllegalArgumentException("Invalid tilde ID: $tildeId")
259+
}
260+
261+
/**
262+
* Get the appropriate user ID for a course, converting to global user ID if the course is on a different shard
263+
*
264+
* @param courseId the course ID
265+
* @param userId the user ID
266+
* @param shardIds map of course IDs to shard IDs
267+
* @param accessToken the access token to extract the user's shard ID from
268+
* @return the user ID to use (either original or global)
269+
*/
270+
fun getUserIdForCourse(
271+
courseId: Long,
272+
userId: Long,
273+
shardIds: Map<Long, String?>,
274+
accessToken: String
275+
): Long {
276+
val courseShardId = shardIds[courseId]
277+
val tokenShardId = getShardIdFromToken(accessToken)
278+
279+
return if (courseShardId != null && tokenShardId != null && courseShardId != tokenShardId) {
280+
createGlobalUserId(tokenShardId, userId)
281+
} else {
282+
userId
283+
}
284+
}
232285
}

‎libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/utils/ApiPrefs.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ object ApiPrefs : PrefManager(PREFERENCE_FILE_NAME) {
9797

9898
var overrideDomains: MutableMap<Long, String?> = mutableMapOf()
9999

100+
var shardIds: MutableMap<Long, String?> = mutableMapOf()
101+
100102
val fullDomain: String
101103
get() = if (isMasquerading || isStudentView) {
102104
when {

0 commit comments

Comments
 (0)