feat: [FC-0047] xBlock offline mode #474

IvanStepanok · 2024-06-25T18:47:52Z

🛜 Offline mode – Quick Preview

Downloading

1downloading.1.mov

Working with blocks without an internet connection

Screen.Recording.2024-06-25.at.20.14.22.1.mov

Offline content is synchronized immediately after connecting to the network

offline.content.are.synced.1.mov

Environment for testing:

API_HOST_URL: 'https://axim-mobile.raccoongang.net'
ENVIRONMENT_DISPLAY_NAME: 'lms-axim-stage'
FEEDBACK_EMAIL_ADDRESS: '[email protected]'
OAUTH_CLIENT_ID: 'zP3vPz00c8fTRpYjNbVSlA1fxt9LnCxTM4JK1KQ0'
DISCOVERY:
    TYPE: "native"
    
FIREBASE:
    ENABLED: false

UI_COMPONENTS:
    COURSE_UNIT_PROGRESS_ENABLED: false
    COURSE_NESTED_LIST_ENABLED: false

openedx-webhooks · 2024-06-25T18:47:56Z

Thanks for the pull request, @IvanStepanok!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/openedx-mobile-maintainers. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

rnr · 2024-09-03T13:32:49Z

@IvanStepanok Thank you for your work on this feature - looks cool!
I have a few comments on the UI - will place here

rnr · 2024-09-03T13:35:43Z

While the app is downloading for offline, the user stops the app and starts it again; after navigating to the course -> offline user sees the ‘Download All’ button, but the download continues.

offline_bug.mov

rnr · 2024-09-03T13:47:52Z

All content in course is loaded. User deletes only one block. After tap on "Download all" button it shows like user will get whole course again.

Screen.Recording.2024-09-03.at.14.58.31.mov

rnr · 2024-09-03T13:52:59Z

All content in course is loaded. Some chapter contains youtube videos only (which are not downloadable). When goes to Videos Tab and try to remove such chapter it shows "Removing this content will free up 0MB" - do we need to show 'downloaded' state for chapters in Videos which don't contain downloaded videos?

rnr · 2024-09-04T12:10:50Z

A user has downloaded a course. The user answered a few questions offline, went online, the answers were synchronised, and now we see them in the online version. (it's working right)
When the user goes to that test again offline, he don't see the answers - it looks like the user is seeing the downloaded version BEFORE he gave the answers.

RPReplay_Final1725451531.MP4

rnr · 2024-09-04T12:29:03Z

The user can't select a previously selected option in an offline quiz. For example: select option 3 - submit - change your mind and select option 4 - submit - change your mind again and try to select option 3 - unable to select option 3 again

RPReplay_Final1725452770.MP4

IvanStepanok · 2024-09-05T14:05:08Z

Hi @rnr, and thank you for the review! It is very valuable to me to get a view from another angle. I appreciate it.

✅ Fixed

Screen.Recording.2024-09-05.at.12.23.40.mov

✅Fixed (Pay attention. We don't show download notification, less than 100 MB when connected to Wi-Fi)

RPReplay_Final1725544125.MP4

✅ Fixed

Screen.Recording.2024-09-05.at.16.54.28.mov

I also noticed that if we delete one file, when downloading, the dialog box shows us the size of the entire group instead of a single undownloaded file. This is also fixed.

🤷‍♂️ This is a known issue, the Open EDX team plans to improve this flow in the future. Currently, the backend has IOSBridge only for offline mode. As soon as they finalize the backend, it will work for us immediately.
🤷‍♂️ This is also the behavior that was set on the backend side, on our side we cannot influence it in any way.

rnr · 2024-09-09T12:33:35Z

@IvanStepanok Thank you for the fixes and clarifications
Looks like we have issue for video:
2. It seems that the video file is not counted in the total size, even though the dialogue says that this space will be freed up.

Screen.Recording.2024-09-09.at.14.29.58.mov

IvanStepanok · 2024-09-10T05:39:12Z

Hi, @rnr, and thanks for the question. In this case, we don't know the size of a video until we download it because the server returns 0 megabytes. We can measure the actual size of the videos only after downloading. To prevent this behavior, we need to receive the actual video size in the server response.

rnr

Amazing work done. Please fix some small comments

rnr · 2024-09-12T15:11:21Z

Core/Core/View/Base/FileWebView.swift

+                if let pdfData = try? Data(contentsOf: url) {
+                    webview.load(
+                        pdfData,
+                        mimeType: "application/pdf",


maybe there are some other types of files we may want to open (which webview also supports)? Maybe we need to create some Model (with mimeType and extension parameters, maybe also isWebViewViewable could be determined there) for file and use here and inside WebView->decidePolicyFor while check file extension? WDYT?

I added this part myself, it goes beyond the requirements and out of scope.❤️

rnr · 2024-09-12T15:26:18Z

Course/Course/Presentation/Container/CourseContainerViewModel.swift

            }
        } catch let error {
            if error is NoWiFiError {
                errorMessage = CoreLocalization.Error.wifi
            }
        }
    }
-
+
+    private func presentNoInternetAlert(sequentials: [CourseSequential]) {


Will be this Alert showed anytime? As I see it should be showed if !connectivity.isInternetAvaliable { inside downloadAll() function but downloadAll button (which fires this function) is hiding in case if !connectivity.isInternetAvaliable {. So user has no chance to tap on 'Download All' button in offline. WDYT?

You're right, it's a good observation! However, this alert could still be useful if the internet connection drops during the download process. The user might start downloading with a good connection, but if it's lost midway, this alert would inform them why the download stopped. It's a nice failsafe for unexpected network changes during long operations.

rnr · 2024-09-12T15:39:36Z

Course/Course/Presentation/Offline/OfflineView.swift

+                                            .padding(.top, 8)
+                                            .padding(.bottom, 16)
+                                    }
+                                        downloadAll


looks like redundant space here

rnr · 2024-09-12T15:43:04Z

Course/Course/Presentation/Offline/OfflineView.swift

+    @ViewBuilder
+    private var downloadAll: some View {
+        if viewModel.connectivity.isInternetAvaliable
+            && (viewModel.totalFilesSize - viewModel.downloadedFilesSize != 0)


Shouldn't the second part (after &&) be included in the common brackets?

Thank you for pointing that out; you're absolutely right. I'll make the necessary changes.

rnr · 2024-09-12T15:45:23Z

Course/Course/Presentation/Offline/OfflineView.swift

+                        .stroke(
+                            viewModel.totalFilesSize == 0
+                            ? .clear
+                            : viewModel.downloadAllButtonState.color, lineWidth: 2


please move lineWidth on next line

rnr · 2024-09-12T16:39:50Z

Course/Course/Presentation/Subviews/ActionViews/DownloadErrorAlertView.swift

+                        childs: [],
+                        sequentialProgress: nil,
+                        due: nil
+                    ),


trailing comma violation

rnr · 2024-09-12T16:42:13Z

Course/Course/Presentation/Subviews/CustomDisclosureGroup.swift

+                }.contains(where: { $0.isDownloadable })
+                guard isDownloadable else { return false }
+            }
+            if let _ = viewModel.sequentialsDownloadState[sequential.id] {


change to !=nil please

rnr · 2024-09-12T16:43:17Z

Course/Course/Presentation/Unit/CourseUnitView.swift

@@ -413,7 +429,7 @@ public struct CourseUnitView: View {
                Spacer()
            }
            VStack {
-                if !isHorizontal {
+                if (!isHorizontal) {


Do we need brackets here?

rnr · 2024-09-12T16:44:48Z

Course/Course/Presentation/Unit/Subviews/OfflineContentView.swift

+        }
+        .padding(24)
+    }
+}


Do we need preview for this view?

Sure, why not🫶

rnr · 2024-09-12T16:45:38Z

OpenEdX/AppDelegate.swift

+    // Background progress update
+
+    func registerBackgroundTask() {
+        let isRegistered = BGTaskScheduler.shared.register(forTaskWithIdentifier: Self.bgAppTaskId, using: nil) { task in


line length violation

IvanStepanok · 2024-09-16T09:20:29Z

@rnr Thanks for review! All feedback addressed.

rnr

LGTM! Sorry for delay

feat: course offline mode

180b15d

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jun 25, 2024

IvanStepanok changed the base branch from main to develop June 26, 2024 08:52

IvanStepanok added 2 commits June 26, 2024 12:04

Merge branch 'develop' into feat/course-offline-mode

6a50011

Merge branch 'develop' into feat/course-offline-mode

3f2b941

volodymyr-chekyrta changed the title ~~Feat/course offline mode~~ feat: [FC-0047] xBlock offline mode Jun 26, 2024

IvanStepanok added 12 commits June 26, 2024 19:08

fix: address feedback

552a563

fix: address feedback

761cac0

Merge branch 'develop' into feat/course-offline-mode

3931435

fix: address feedback

28882b5

Merge branch 'develop' into feat/course-offline-mode

7511e8d

fix: resolve merge conflicts

c75cfd9

fix: update mocks

de34954

fix: address feedback

e452818

fix: address feedback

dbb78bc

fix: address feedback

9cab5f8

Merge branch 'develop' into feat/course-offline-mode

5200a44

fix: address feedback

e9cbdda

volodymyr-chekyrta requested review from rnr, saeedbashir and volodymyr-chekyrta July 23, 2024 08:37

volodymyr-chekyrta marked this pull request as ready for review July 23, 2024 08:38

marcotuts mentioned this pull request Jul 24, 2024

[iOS] Fully Offline Courses (Text and Problems) (FC-47) #83

Open

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Jul 30, 2024

Merge branch 'develop' into feat/course-offline-mode

b12d8d7

fix: address feedback

3edf883

rnr reviewed Sep 12, 2024

View reviewed changes

fix: address feedback

b467844

rnr approved these changes Sep 27, 2024

View reviewed changes

volodymyr-chekyrta approved these changes Sep 30, 2024

View reviewed changes

volodymyr-chekyrta merged commit 23c219c into openedx:develop Sep 30, 2024
6 checks passed

IvanStepanok mentioned this pull request Nov 6, 2024

Develop to main. Release v2 #538

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [FC-0047] xBlock offline mode #474

feat: [FC-0047] xBlock offline mode #474

IvanStepanok commented Jun 25, 2024 •

edited

Loading

openedx-webhooks commented Jun 25, 2024 •

edited

Loading

rnr commented Sep 3, 2024

rnr commented Sep 3, 2024 •

edited

Loading

rnr commented Sep 3, 2024 •

edited

Loading

rnr commented Sep 3, 2024

rnr commented Sep 4, 2024

rnr commented Sep 4, 2024

IvanStepanok commented Sep 5, 2024 •

edited

Loading

rnr commented Sep 9, 2024

IvanStepanok commented Sep 10, 2024

rnr left a comment

rnr Sep 12, 2024

IvanStepanok Sep 16, 2024

rnr Sep 12, 2024

IvanStepanok Sep 16, 2024

rnr Sep 12, 2024

rnr Sep 12, 2024

IvanStepanok Sep 16, 2024

rnr Sep 12, 2024

rnr Sep 12, 2024

rnr Sep 12, 2024

rnr Sep 12, 2024

rnr Sep 12, 2024

IvanStepanok Sep 16, 2024

rnr Sep 12, 2024

IvanStepanok Sep 16, 2024

IvanStepanok commented Sep 16, 2024

rnr left a comment

feat: [FC-0047] xBlock offline mode #474

feat: [FC-0047] xBlock offline mode #474

Conversation

IvanStepanok commented Jun 25, 2024 • edited Loading

🛜 Offline mode – Quick Preview

Downloading

Working with blocks without an internet connection

Offline content is synchronized immediately after connecting to the network

Environment for testing:

openedx-webhooks commented Jun 25, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

rnr commented Sep 3, 2024

rnr commented Sep 3, 2024 • edited Loading

rnr commented Sep 3, 2024 • edited Loading

rnr commented Sep 3, 2024

rnr commented Sep 4, 2024

rnr commented Sep 4, 2024

IvanStepanok commented Sep 5, 2024 • edited Loading

rnr commented Sep 9, 2024

IvanStepanok commented Sep 10, 2024

rnr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvanStepanok commented Sep 16, 2024

rnr left a comment

Choose a reason for hiding this comment

IvanStepanok commented Jun 25, 2024 •

edited

Loading

openedx-webhooks commented Jun 25, 2024 •

edited

Loading

rnr commented Sep 3, 2024 •

edited

Loading

rnr commented Sep 3, 2024 •

edited

Loading

IvanStepanok commented Sep 5, 2024 •

edited

Loading