Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: api migration #171

Merged
merged 11 commits into from
Jan 5, 2024
Merged

feat: api migration #171

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0459bd6
API changes. TODO: find solution for courseware_access and commented …
PavloNetrebchuk Sep 21, 2023
994496c
Chenged "dataReady" logic in CourseContainerFragment
PavloNetrebchuk Sep 22, 2023
5159709
Merge remote-tracking branch 'origin/develop' into feature/api_changes
PavloNetrebchuk Sep 29, 2023
8942ebc
Merge branch 'develop' into feat/api_migration
volodymyr-chekyrta Dec 19, 2023
f6cf0bb
Added mobile_search argument to the course search
volodymyr-chekyrta Dec 19, 2023
c706f76
Fix CourseContainerViewModelTest
volodymyr-chekyrta Dec 19, 2023
b266102
Merge remote-tracking branch 'upstream/develop' into feat/api_migration
volodymyr-chekyrta Dec 27, 2023
5e00139
fix: code improvements
volodymyr-chekyrta Dec 28, 2023
e3d98f0
Removed unused auditAccessExpires from the NoAccessCourseContainerFra…
volodymyr-chekyrta Jan 2, 2024
7df30f7
Merge remote-tracking branch 'origin/develop' into feat/api_migration
volodymyr-chekyrta Jan 2, 2024
0c6b215
Change CourseApi.getCourseDetail endpoint
volodymyr-chekyrta Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions app/src/main/java/org/openedx/app/AppRouter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.openedx.auth.presentation.restore.RestorePasswordFragment
import org.openedx.auth.presentation.signin.SignInFragment
import org.openedx.auth.presentation.signup.SignUpFragment
import org.openedx.core.FragmentViewType
import org.openedx.core.domain.model.CoursewareAccess
import org.openedx.core.presentation.course.CourseViewMode
import org.openedx.core.presentation.global.app_upgrade.AppUpgradeRouter
import org.openedx.core.presentation.global.app_upgrade.UpgradeRequiredFragment
Expand Down Expand Up @@ -113,13 +112,9 @@ class AppRouter : AuthRouter, DiscoveryRouter, DashboardRouter, CourseRouter, Di
override fun navigateToNoAccess(
fm: FragmentManager,
title: String,
coursewareAccess: CoursewareAccess,
auditAccessExpires: Date?
) {
replaceFragment(
fm,
NoAccessCourseContainerFragment.newInstance(title, coursewareAccess, auditAccessExpires)
)
replaceFragment(fm, NoAccessCourseContainerFragment.newInstance(title, auditAccessExpires))
volodymyr-chekyrta marked this conversation as resolved.
Show resolved Hide resolved
}
//endregion

Expand Down
11 changes: 6 additions & 5 deletions core/src/main/java/org/openedx/core/data/api/CourseApi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@ import retrofit2.http.*

interface CourseApi {

@GET("/mobile_api_extensions/v1/users/{username}/course_enrollments")
@GET("/api/mobile/v3/users/{username}/course_enrollments/")
Copy link
Contributor

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 params
i.e,

    @GET("/api/mobile/{api_version}/users/{username}/course_enrollments/")
    suspend fun getEnrolledCourses(
        @Header("Cache-Control") cacheControlHeaderParam: String? = null,
        @Path("api_version") apiVersion: String,
        @Path("username") username: String,
        @Query("org") org: String? = null,
        @Query("page") page: Int
    ): CourseEnrollments

Copy link
Contributor Author

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?

Copy link
Contributor

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 ?

Copy link
Contributor

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

suspend fun getEnrolledCourses(
@Header("Cache-Control") cacheControlHeaderParam: String? = null,
@Path("username") username: String,
@Query("org") org: String? = null,
@Query("page") page: Int
): DashboardCourseList
): CourseEnrollments

@GET("/mobile_api_extensions/courses/v1/courses/")
@GET("/api/courses/v1/courses/")
suspend fun getCourseList(
@Query("search_term") searchQuery: String? = null,
@Query("page") page: Int,
@Query("mobile") mobile: Boolean,
@Query("mobile_search") mobileSearch: Boolean,
@Query("username") username: String? = null,
@Query("org") org: String? = null,
@Query("permissions") permission: List<String> = listOf(
Expand All @@ -28,15 +29,15 @@ interface CourseApi {
)
): CourseList

@GET("/mobile_api_extensions/v1/courses/{course_id}")
@GET("/api/mobile/v3/course_info/{course_id}/info")
volodymyr-chekyrta marked this conversation as resolved.
Show resolved Hide resolved
suspend fun getCourseDetail(
@Path("course_id") courseId: String?,
@Query("username") username: String? = null,
@Query("is_enrolled") isEnrolled: Boolean = true,
): CourseDetails

@GET(
"/mobile_api_extensions/{api_version}/blocks/?" +
"/api/mobile/{api_version}/course_info/blocks/?" +
omerhabib26 marked this conversation as resolved.
Show resolved Hide resolved
"depth=all&" +
"requested_fields=contains_gated_content,show_gated_sections,special_exam_info,graded,format,student_view_multi_device,due,completion&" +
"student_view_data=video,discussion&" +
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/java/org/openedx/core/data/model/CourseEnrollments.kt
View file
Open in desktop
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
)
omerhabib26 marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions core/src/main/java/org/openedx/core/data/model/CourseStructureModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ data class CourseStructureModel(
startDisplay = startDisplay ?: "",
startType = startType ?: "",
end = TimeUtils.iso8601ToDate(end ?: ""),
coursewareAccess = coursewareAccess?.mapToDomain()!!,
coursewareAccess = coursewareAccess?.mapToDomain(),
media = media?.mapToDomain(),
certificate = certificate?.mapToDomain(),
isSelfPaced = isSelfPaced ?: false
Expand All @@ -70,7 +70,7 @@ data class CourseStructureModel(
startDisplay = startDisplay ?: "",
startType = startType ?: "",
end = end ?: "",
coursewareAccess = coursewareAccess?.mapToRoomEntity()!!,
coursewareAccess = coursewareAccess?.mapToRoomEntity(),
media = MediaDb.createFrom(media),
certificate = certificate?.mapToRoomEntity(),
isSelfPaced = isSelfPaced ?: false
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/openedx/core/data/model/EnrolledCourseData.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ data class EnrolledCourseData(
end = TimeUtils.iso8601ToDate(end ?: ""),
dynamicUpgradeDeadline = dynamicUpgradeDeadline ?: "",
subscriptionId = subscriptionId ?: "",
coursewareAccess = coursewareAccess?.mapToDomain()!!,
coursewareAccess = coursewareAccess?.mapToDomain(),
media = media?.mapToDomain(),
courseImage = courseImage ?: "",
courseAbout = courseAbout ?: "",
Expand All @@ -86,7 +86,7 @@ data class EnrolledCourseData(
end = end ?: "",
dynamicUpgradeDeadline = dynamicUpgradeDeadline ?: "",
subscriptionId = subscriptionId ?: "",
coursewareAccess = coursewareAccess?.mapToRoomEntity()!!,
coursewareAccess = coursewareAccess?.mapToRoomEntity(),
media = MediaDb.createFrom(media),
courseImage = courseImage ?: "",
courseAbout = courseAbout ?: "",
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/openedx/core/data/model/room/CourseStructureEntity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ data class CourseStructureEntity(
@ColumnInfo("end")
val end: String?,
@Embedded
val coursewareAccess: CoursewareAccessDb,
val coursewareAccess: CoursewareAccessDb?,
@Embedded
val media: MediaDb?,
@Embedded
Expand All @@ -54,7 +54,7 @@ data class CourseStructureEntity(
startDisplay,
startType,
TimeUtils.iso8601ToDate(end ?: ""),
coursewareAccess.mapToDomain(),
coursewareAccess?.mapToDomain(),
media?.mapToDomain(),
certificate?.mapToDomain(),
isSelfPaced
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/openedx/core/data/model/room/discovery/EnrolledCourseEntity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ data class EnrolledCourseDataDb(
@ColumnInfo("subscriptionId")
val subscriptionId: String,
@Embedded
val coursewareAccess: CoursewareAccessDb,
val coursewareAccess: CoursewareAccessDb?,
@Embedded
val media: MediaDb?,
@ColumnInfo(name = "course_image_link")
Expand Down Expand Up @@ -93,7 +93,7 @@ data class EnrolledCourseDataDb(
TimeUtils.iso8601ToDate(end),
dynamicUpgradeDeadline,
subscriptionId,
coursewareAccess.mapToDomain(),
coursewareAccess?.mapToDomain(),
media?.mapToDomain(),
courseImage,
courseAbout,
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/openedx/core/domain/model/CourseStructure.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ data class CourseStructure(
val startDisplay: String,
val startType: String,
val end: Date?,
val coursewareAccess: CoursewareAccess,
val coursewareAccess: CoursewareAccess?,
val media: Media?,
val certificate: Certificate?,
val isSelfPaced: Boolean
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/openedx/core/domain/model/EnrolledCourseData.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ data class EnrolledCourseData(
val end: Date?,
val dynamicUpgradeDeadline: String,
val subscriptionId: String,
val coursewareAccess: CoursewareAccess,
val coursewareAccess: CoursewareAccess?,
val media: Media?,
val courseImage: String,
val courseAbout: String,
Expand Down
2 changes: 1 addition & 1 deletion course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class CourseRepository(
suspend fun preloadCourseStructure(courseId: String) {
val response = api.getCourseStructure(
"stale-if-error=0",
"v1",
"v3",
omerhabib26 marked this conversation as resolved.
Show resolved Hide resolved
preferencesManager.user?.username,
courseId
)
Expand Down
2 changes: 0 additions & 2 deletions course/src/main/java/org/openedx/course/presentation/CourseRouter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.openedx.course.presentation

import androidx.fragment.app.FragmentManager
import org.openedx.core.domain.model.CoursewareAccess
import org.openedx.core.presentation.course.CourseViewMode
import org.openedx.course.presentation.handouts.HandoutsType
import java.util.Date
Expand All @@ -17,7 +16,6 @@ interface CourseRouter {
fun navigateToNoAccess(
fm: FragmentManager,
title: String,
coursewareAccess: CoursewareAccess,
auditAccessExpires: Date?
)

Expand Down
21 changes: 9 additions & 12 deletions course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,15 @@ 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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.
CourseStatusInfo, CourseDates, and Topis API are failing due to course expiry. I guess we can update this change with the following for now. We also need to update the message on NoAccessScreen accordingly. Right now it showing The Course hasn't started yet for every case.

        viewModel.dataReady.observe(viewLifecycleOwner) { coursewareAccess ->
            if (coursewareAccess != null && coursewareAccess.hasAccess) {
                initViewPager()
            } else {
                router.navigateToNoAccess(
                    requireActivity().supportFragmentManager,
                    courseTitle
                )
            }
        }

initViewPager()
} else {
router.navigateToNoAccess(
requireActivity().supportFragmentManager,
courseTitle,
null
)
}
}
viewModel.errorMessage.observe(viewLifecycleOwner) {
Expand Down
11 changes: 7 additions & 4 deletions course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package org.openedx.course.presentation.container
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.launch
import org.openedx.core.BaseViewModel
import org.openedx.core.R
import org.openedx.core.SingleEventLiveData
import org.openedx.core.domain.model.CoursewareAccess
import org.openedx.core.exception.NoCachedDataException
import org.openedx.core.extension.isInternetError
import org.openedx.core.system.ResourceManager
Expand All @@ -15,6 +15,7 @@ import org.openedx.core.system.notifier.CourseNotifier
import org.openedx.core.system.notifier.CourseStructureUpdated
import org.openedx.course.domain.interactor.CourseInteractor
import org.openedx.course.presentation.CourseAnalytics
import java.util.Date
import kotlinx.coroutines.launch
import org.openedx.core.config.Config
import org.openedx.core.system.notifier.CourseCompletionSet
Expand All @@ -31,8 +32,8 @@ class CourseContainerViewModel(

val isCourseTopTabBarEnabled get() = config.isCourseTopTabBarEnabled()

private val _dataReady = MutableLiveData<CoursewareAccess>()
val dataReady: LiveData<CoursewareAccess>
private val _dataReady = MutableLiveData<Boolean?>()
val dataReady: LiveData<Boolean?>
get() = _dataReady

private val _errorMessage = SingleEventLiveData<String>()
Expand Down Expand Up @@ -70,7 +71,9 @@ class CourseContainerViewModel(
}
val courseStructure = interactor.getCourseStructureFromCache()
courseName = courseStructure.name
_dataReady.value = courseStructure.coursewareAccess
_dataReady.value = courseStructure.start?.let { start ->
start < Date()
}
} catch (e: Exception) {
if (e.isInternetError() || e is NoCachedDataException) {
_errorMessage.value =
Expand Down
6 changes: 0 additions & 6 deletions ...rc/main/java/org/openedx/course/presentation/container/NoAccessCourseContainerFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -37,14 +36,12 @@ import org.openedx.course.R as courseR
class NoAccessCourseContainerFragment : Fragment() {

private var courseTitle = ""
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
}
}
Expand Down Expand Up @@ -74,18 +71,15 @@ 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
)
return fragment
Expand Down
3 changes: 2 additions & 1 deletion ...e/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.junit.Test
import org.junit.rules.TestRule
import org.openedx.core.config.Config
import java.net.UnknownHostException
import java.util.Date

@OptIn(ExperimentalCoroutinesApi::class)
class CourseContainerViewModelTest {
Expand All @@ -48,7 +49,7 @@ class CourseContainerViewModelTest {
name = "Course name",
number = "",
org = "Org",
start = null,
start = Date(0),
startDisplay = "",
startType = "",
end = null,
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/main/java/org/openedx/dashboard/data/repository/DashboardRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class DashboardRepository(
page = page
)
if (page == 1) dao.clearCachedData()
dao.insertEnrolledCourseEntity(*result.results.map { it.mapToRoomEntity() }.toTypedArray())
return result.mapToDomain()
dao.insertEnrolledCourseEntity(*result.enrollments.results.map { it.mapToRoomEntity() }.toTypedArray())
return result.enrollments.mapToDomain()
}

suspend fun getEnrolledCoursesFromCache(): List<EnrolledCourse> {
Expand Down
8 changes: 7 additions & 1 deletion discovery/src/main/java/org/openedx/discovery/data/repository/DiscoveryRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class DiscoveryRepository(
val pageResponse = api.getCourseList(
page = pageNumber,
mobile = true,
mobileSearch = false,
username = username,
org = organization
)
Expand All @@ -42,7 +43,12 @@ class DiscoveryRepository(
query: String,
pageNumber: Int,
): CourseList {
val pageResponse = api.getCourseList(searchQuery = query, page = pageNumber, mobile = true)
val pageResponse = api.getCourseList(
searchQuery = query,
page = pageNumber,
mobile = true,
mobileSearch = true
)
return CourseList(
pageResponse.pagination.mapToDomain(),
pageResponse.results?.map { it.mapToDomain() } ?: emptyList()
Expand Down
2 changes: 1 addition & 1 deletion discussion/src/main/java/org/openedx/discussion/data/api/DiscussionApi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ interface DiscussionApi {
@Query("requested_fields") requestedFields: List<String> = listOf("profile_image")
): CommentsResponse

@POST("/mobile_api_extensions/discussion/v1/comments/")
@POST("/api/discussion/v1/comments/")
suspend fun createComment(
@Body commentBody: CommentBody
) : CommentResult
Expand Down
2 changes: 1 addition & 1 deletion profile/src/main/java/org/openedx/profile/data/api/ProfileApi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ interface ProfileApi {
suspend fun deleteProfileImage(@Path("username") username: String?): Response<Unit>

@FormUrlEncoded
@POST("/mobile_api_extensions/user/v1/accounts/deactivate_logout/")
@POST("/api/user/v1/accounts/deactivate_logout/")
suspend fun deactivateAccount(
@Field("password") password: String
): Response<Unit>
Expand Down
Loading