-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: api migration #171
feat: api migration #171
Changes from all commits
0459bd6
994496c
5159709
8942ebc
f6cf0bb
c706f76
b266102
5e00139
e3d98f0
7df30f7
0c6b215
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package org.openedx.core.data.model | ||
|
||
import com.google.gson.annotations.SerializedName | ||
|
||
data class CourseEnrollments( | ||
@SerializedName("enrollments") | ||
val enrollments: DashboardCourseList | ||
omerhabib26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,18 +66,14 @@ class CourseContainerFragment : Fragment(R.layout.fragment_course_container) { | |
} | ||
|
||
private fun observe() { | ||
viewModel.dataReady.observe(viewLifecycleOwner) { coursewareAccess -> | ||
if (coursewareAccess != null) { | ||
if (coursewareAccess.hasAccess) { | ||
initViewPager() | ||
} else { | ||
router.navigateToNoAccess( | ||
requireActivity().supportFragmentManager, | ||
courseTitle, | ||
coursewareAccess, | ||
null | ||
) | ||
} | ||
viewModel.dataReady.observe(viewLifecycleOwner) { isReady -> | ||
if (isReady == true) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition doesn't work properly with the expired course and shows an API loading error.
|
||
initViewPager() | ||
} else { | ||
router.navigateToNoAccess( | ||
requireActivity().supportFragmentManager, | ||
courseTitle | ||
) | ||
} | ||
} | ||
viewModel.errorMessage.observe(viewLifecycleOwner) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,6 @@ import androidx.compose.ui.unit.dp | |
import androidx.compose.ui.zIndex | ||
import androidx.core.os.bundleOf | ||
import androidx.fragment.app.Fragment | ||
import org.openedx.core.domain.model.CoursewareAccess | ||
import org.openedx.core.extension.parcelable | ||
import org.openedx.core.ui.* | ||
import org.openedx.core.ui.theme.OpenEdXTheme | ||
|
@@ -36,19 +35,6 @@ import org.openedx.course.R as courseR | |
|
||
class NoAccessCourseContainerFragment : Fragment() { | ||
|
||
private var courseTitle = "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we can revert this change and handle it properly |
||
private var coursewareAccess: CoursewareAccess? = null | ||
private var auditAccessExpires: Date? = null | ||
|
||
override fun onCreate(savedInstanceState: Bundle?) { | ||
super.onCreate(savedInstanceState) | ||
with(requireArguments()) { | ||
courseTitle = getString(ARG_TITLE, "") | ||
coursewareAccess = parcelable(ARG_COURSEWARE_ACCESS) | ||
auditAccessExpires = parcelable(ARG_AUDIT_ACCESS_EXPIRES) | ||
} | ||
} | ||
|
||
override fun onCreateView( | ||
inflater: LayoutInflater, | ||
container: ViewGroup?, | ||
|
@@ -58,12 +44,9 @@ class NoAccessCourseContainerFragment : Fragment() { | |
setContent { | ||
OpenEdXTheme { | ||
val windowSize = rememberWindowSize() | ||
val auditAccessExpired = | ||
auditAccessExpires != null && Date().after(auditAccessExpires) | ||
NoAccessCourseContainerScreen( | ||
windowSize = windowSize, | ||
title = courseTitle, | ||
auditAccessExpired = auditAccessExpired, | ||
title = requireArguments().getString(ARG_TITLE, ""), | ||
onBackClick = { | ||
requireActivity().supportFragmentManager.popBackStack() | ||
} | ||
|
@@ -74,19 +57,13 @@ class NoAccessCourseContainerFragment : Fragment() { | |
|
||
companion object { | ||
private const val ARG_TITLE = "title" | ||
private const val ARG_COURSEWARE_ACCESS = "coursewareAccess" | ||
private const val ARG_AUDIT_ACCESS_EXPIRES = "auditAccessExpires" | ||
|
||
fun newInstance( | ||
title: String, | ||
coursewareAccess: CoursewareAccess, | ||
auditAccessExpires: Date? | ||
): NoAccessCourseContainerFragment { | ||
val fragment = NoAccessCourseContainerFragment() | ||
fragment.arguments = bundleOf( | ||
ARG_TITLE to title, | ||
ARG_COURSEWARE_ACCESS to coursewareAccess, | ||
ARG_AUDIT_ACCESS_EXPIRES to auditAccessExpires | ||
ARG_TITLE to title | ||
) | ||
return fragment | ||
} | ||
|
@@ -99,7 +76,6 @@ class NoAccessCourseContainerFragment : Fragment() { | |
private fun NoAccessCourseContainerScreen( | ||
windowSize: WindowSize, | ||
title: String, | ||
auditAccessExpired: Boolean, | ||
onBackClick: () -> Unit | ||
) { | ||
val scaffoldState = rememberScaffoldState() | ||
|
@@ -165,11 +141,7 @@ private fun NoAccessCourseContainerScreen( | |
) | ||
Spacer(modifier = Modifier.height(10.dp)) | ||
Text( | ||
text = if (auditAccessExpired) { | ||
stringResource(id = courseR.string.course_access_expired) | ||
} else { | ||
stringResource(id = courseR.string.course_not_started) | ||
}, | ||
text = stringResource(id = courseR.string.course_not_started), | ||
color = MaterialTheme.appColors.textPrimary, | ||
style = MaterialTheme.appTypography.bodyLarge | ||
) | ||
|
@@ -189,8 +161,7 @@ fun NoAccessCourseContainerScreenPreview() { | |
OpenEdXTheme { | ||
NoAccessCourseContainerScreen( | ||
windowSize = WindowSize(WindowType.Compact, WindowType.Compact), | ||
title = "Example title", | ||
auditAccessExpired = false, | ||
title = "Course title", | ||
onBackClick = {} | ||
) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can also pass the
api_version
in paramsi.e,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it's a great suggestion 👍👍👍
It might be beneficial to handle this improvement in a separate pull request to include a user API version to the config as well.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure,
@moiz994 can you please create a ticket for these improvements ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to create the ticket for now, we will handle it later