Performance: Optimized data loading of course#416
Performance: Optimized data loading of course#416volodymyr-chekyrta merged 5 commits intoopenedx:developfrom
Conversation
Sh-AhsanArif
commented
Jan 16, 2025
- Optimized data loading on app dashboard screen (cached data first)
- Optimized data loading on course outline screen (cached data first)
- Minor code refactoring
|
Thanks for the pull request, @Sh-AhsanArif! This repository is currently maintained by @openedx/openedx-mobile-maintainers. Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.
|
|
@Sh-AhsanArif as you are covered by the Arbisoft entity CLA, I've added you to the triage team. This will allow tests to run automatically on your PRs. Please accept the invite. |
course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
Fixed
Show fixed
Hide fixed
cd9e651 to
e816e86
Compare
course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
Fixed
Show fixed
Hide fixed
e816e86 to
e86d49f
Compare
course/src/test/java/org/openedx/course/presentation/outline/CourseOutlineViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/outline/CourseOutlineViewModelTest.kt
Fixed
Show fixed
Hide fixed
d2c5156 to
44803f0
Compare
|
@Sh-AhsanArif |
|
Thank you for your feedback! That’s a great suggestion, and I completely agree that moving all relevant methods to return Let me know what you think! |
|
@Sh-AhsanArif |
44803f0 to
d28d821
Compare
|
Hi, @Sh-AhsanArif I’ve reviewed the app again and have a few questions: Could you clarify why we are using |
|
Hey @Sh-AhsanArif, thank you so much for this improvement! I had a chance to take a quick look, and I noticed a couple of small UI issues in your repo, as shown in the video:
OpenEdxPR.webm |
course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/data/repository/CourseRepository.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt
Fixed
Show fixed
Hide fixed
|
Hey @PavloNetrebchuk, @omerhabib26 Thank you for reviewing the PR and providing valuable feedback. I’ve incorporated the suggested changes and fixed the mentioned issues. Please have a look at the latest updates and let me know if any further adjustments are needed. |
course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt
Outdated
Show resolved
Hide resolved
course/src/main/java/org/openedx/course/presentation/container/CourseContainerViewModel.kt
Outdated
Show resolved
Hide resolved
|
@Sh-AhsanArif Hello. It looks good to me. Just 2 question ⬆️ |
course/src/test/java/org/openedx/course/presentation/container/CourseContainerViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/outline/CourseOutlineViewModelTest.kt
Fixed
Show fixed
Hide fixed
f9b8ad1 to
aaee6ff
Compare
|
Thank you @Sh-AhsanArif for the update. I'll start its review on monday. |
|
@volodymyr-chekyrta are we good to merge this PR? Pavlo & Omer both have approved this PR. |
merging 👍 |