feat: Added offline support for CourseEnrollmentDetails API #69
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omerhabib26
requested review from
k1rill,
dixidroid,
farhan-arshad-dev and
HamzaIsrar12
dixidroid
approved these changes
Nov 25, 2024
- Store API response in DB for offline mode fix: LEARNER-10282
omerhabib26
force-pushed
the
2U/omer/LEARNER-10282
branch
from
a59532a to
a01f356
Compare
farhan-arshad-dev
requested changes
Dec 3, 2024
| @@ -12,4 +12,69 @@ object Migrations { | |||
| db.execSQL("ALTER TABLE download_model ADD COLUMN transcriptDownloadedStatus TEXT NOT NULL DEFAULT 'NOT_DOWNLOADED'") | |||
| } | |||
| } | |||
|
|
|||
| val MIGRATION_2_3 = object : Migration(2, 3) { | |||
| override fun migrate(database: SupportSQLiteDatabase) { | |||
There was a problem hiding this comment.
use db instead of database to make the naming convention consistent.
| val MIGRATION_2_3 = object : Migration(2, 3) { | ||
| override fun migrate(database: SupportSQLiteDatabase) { | ||
|
|
||
| database.execSQL("DROP TABLE IF EXISTS course_enrollment_details_table") |
There was a problem hiding this comment.
Specific use-case to delete the old table?
cuz this will delete the data along with the table structure. also already using CREATE TABLE IF NOT EXISTS in the next statement.
Comment on lines
63
to
79
| database.execSQL( | ||
| """ | ||
| CREATE TABLE IF NOT EXISTS course_modes_table ( | ||
| course_id TEXT NOT NULL, | ||
| slug TEXT, | ||
| sku TEXT, | ||
| androidSku TEXT, | ||
| iosSku TEXT, | ||
| minPrice REAL, | ||
| storeSku TEXT, | ||
| PRIMARY KEY(course_id, slug), | ||
| FOREIGN KEY(course_id) REFERENCES course_enrollment_details_table(id) ON DELETE CASCADE | ||
| ) | ||
| """ | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
I believe it can be removed, as the course_modes_table is not being used elsewhere.
also using CourseModeDB as Column.
Comment on lines
269
to
270
| @Embedded | ||
| val coursewareAccess: CoursewareAccessDb?, |
There was a problem hiding this comment.
The code change in this file can be reverted.
Comment on lines
6
to
8
| import androidx.room.migration.Migration | ||
| import androidx.sqlite.db.SupportSQLiteDatabase | ||
| import org.openedx.core.data.model.room.CourseEnrollmentDetailsEntity |
farhan-arshad-dev
approved these changes
Dec 6, 2024
| val response = api.getEnrollmentDetails(courseId = courseId) | ||
| courseDao.insertCourseEnrollmentDetails(response.mapToRoomEntity()) | ||
| courseEnrollmentDetails[courseId] = response.mapToDomain() | ||
|
|
There was a problem hiding this comment.
extra line can be removed.
| @@ -21,6 +21,7 @@ class CourseRepository( | |||
| private val networkConnection: NetworkConnection, | |||
| ) { | |||
| private var courseStructure = mutableMapOf<String, CourseStructure>() | |||
| private var courseEnrollmentDetails = mutableMapOf<String, CourseEnrollmentDetails>() | |||
There was a problem hiding this comment.
courseEnrollmentDetails can local.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
fix: LEARNER-10282