Repository navigation
fix: resolve confirmed bugs from the open iOS issues - #688
IvanStepanok wants to merge 13 commits into
Conversation
Date(iso8601:) parsed fixed-format API timestamps with the user's locale. When the 24-Hour Time switch is overridden in iOS settings, DateFormatter reads `HH` as a 12-hour field, so any timestamp after 12:59 failed to parse and silently became "now". Due dates showed as "Today" and past-due work was sorted as due today. Parse with en_US_POSIX, as Apple QA1480 recommends and as Date(subtitleTime:) already does.
Fixes openedx#438. Opening a course on the Dates tab (for example from "Past due assignments" on the dashboard) loaded the dates twice: once when the tab appeared and again when the course outline finished loading and posted .getCourseDates. The second load swapped the whole list for a spinner. Every load also read the cached outline into a property nothing used, and on a course opened for the first time that read failed and showed "Course component not found, please reload". The tab now skips a refresh while its own request is still running, refreshes loaded dates in the background without blanking them, ignores refresh requests for other courses, and no longer reads the outline.
Fixes openedx#161. Addresses openedx#419. "Shift due dates" on a course's Dates tab moved the deadlines on the server but the tab kept showing "Missed some deadlines?" and the old past-due list, with no confirmation, so learners tapped it again. The tab posts .shiftCourseDates with a (courseID, courseName) tuple, like the app-level Dates screen and the calendar sync expect, but the tab and the course container only listened for a plain course ID and dropped it. The container also posted a plain ID, so calendar events never updated after a shift there. All senders now post the tuple, the listeners read it and react only to their own course, and the tab reloads its dates and shows the success message. While the request runs, the banner now shows a spinner in place of its button, as the app-level Dates screen does. Before, the whole list was replaced by a full-screen spinner, which on a slow connection looked like the screen had reset (openedx#419). Now that the list refreshes in place, the Dates list also takes its section order from the dates it is drawing. It read the order from the view model, whose dates a refresh can replace while the previous value is still on screen; after a shift the order could name a section (Next Week) that the drawn dates did not have, and the force-unwrapped lookup crashed.
Fixes openedx#271. Links added to synced calendar events carry a course_id that the app uses to open the course (DeepLinkManager.showCourseScreen). Since the calendar synchronization rework in openedx#466, that field held the calendar's name instead of the course ID, so tapping a link in an event could not find the course. Pass the course ID down to where the link is built, as before openedx#466.
Fixes openedx#531. Dark mode inverts course HTML and inverts images back, so photos keep their colours. Figures exported as PNGs with dark text and lines on a transparent background then show that dark ink straight on the dark page and become nearly invisible. Give images a white backing in dark mode; the double inversion keeps it white, so the figure looks as it does on the web. Opaque images are unchanged.
Fixes openedx#650. The Assignments views treated a graded subsection as done only when the whole subsection was complete, so an unwatched video or unread text page kept an assignment "incomplete" after the learner had submitted every problem in it. Count the assignment as done when each graded problem in it has been attempted. The server marks a problem complete as soon as it is submitted, whatever the score, so block completion is the attempt signal.
Fixes openedx#455. While a section's videos were downloading, tapping its progress indicator on the Home, Content and Videos screens cancelled every download in the section at once, with no way to follow progress per video. The course-level download bar already opens the downloads list, where each video shows its progress and can be cancelled on its own. Section indicators now open the same list while downloading, as the previous app did.
Fixes openedx#337. The enrollments API reports courseware_access.has_access = false, for example when an audit learner's access has expired, and the dashboard already shows a lock on such courses, but tapping one still opened the course and loaded its content and videos. The course screen now shows a "You don't currently have access to this course" state with a back button and doesn't request the course content. A course that hasn't started yet keeps its existing "hasn't started" screen. Note: on current edx-platform master the blocks API answers 500 for such a learner (get_assignments_grades returns [] and the view unpacks two values), so before this change the screen spun forever instead.
|
Thanks for the pull request, @IvanStepanok! This repository is currently maintained by 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. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Fixes openedx#576. CourseDownloadHelper refreshed its value from a Task(priority: .background), added in openedx#562. A background task can wait seconds for a thread while the device or the CI runner is busy, so the download buttons lagged behind the download manager, and the tests waiting for that refresh went over the time limit on GitHub Actions. They were commented out in openedx#575 and deleted in openedx#652. On this PR's first CI run the restored tests showed it: awaiting the async refresh directly took 0.004 s, the sync refresh through the background task 7.9 s, and the refresh triggered by a download event did not start within 15 s. The refresh now runs at the priority of whoever asks for it. The tests are back in a form that does not block the main thread: every test is async and waits with fulfillment(of:) instead of wait(for:), so the helper's main-actor hop can always run, and the checks use the Mockolo call counters that replaced SwiftyMocky's Verify.
Fixes openedx#581. On a network that connects but doesn't carry traffic (captive portal, in-flight Wi-Fi) the Learn screen showed a spinner until its request timed out after 60 s and then "No Courses" with "Something went wrong", even though the courses and their outlines were saved from the last visit. The connectivity check from openedx#635 notices such a network after about 30 s and shows the offline banner, but the screens only looked at it when a request started. - Both dashboards now show the saved courses as soon as the app knows it is offline while a request is still hanging, and fall back to them when a request fails instead of emptying the list. A late successful response still replaces them. The update-required (426) path is unchanged. - The course screen opens the outline saved on the last visit when loading it fails on the network, instead of an empty course. Saved data is only used where the app already uses it in offline mode; nothing is shown ahead of a working network.
Fixes openedx#262. After the last block of a section was completed, the app reloaded the course outline and the server already reported the section complete, but the outline kept the old marks and progress bar until the course was closed and opened again. CourseStructure, CourseChapter and CourseSequential compared equal by id alone, and SwiftUI skips redrawing a view whose inputs compare equal, so the refreshed outline was never drawn. They now use memberwise equality, as CourseVertical already did, so new completion, due dates and assignment progress redraw the outline right away.
bd0734e to
5465f0c
Compare
The card at the top of a course's Dates tab always said "Offline", also for a course synced to the calendar, while Settings > Dates & Calendar said "Synced". Its status was a @State declared as a local variable inside body. SwiftUI keeps state only for a view's stored properties, so the value the .task wrote was dropped and the card kept its initial .offline. The status is now a property of the view.
… downloaded Downloads did not check the HTTP status. When the server answered with an error, such as a 403 from a CDN for an expired or removed link, the error page was saved under the video's file name and the video showed as downloaded; playing it offline then failed. Downloads are now validated. A download that fails for a reason other than the connection is dropped from the queue, its file removed, and reported with the other failed downloads once the queue is done, the way a download that started offline already is. Before, it stayed in the queue and the next pass picked it up again straight away. The Alamofire session can be passed in, so a test can answer with an error.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #688 +/- ##
==============================
==============================
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
RawanMatar89
left a comment
There was a problem hiding this comment.
really nice PR great work 👏🏻 LGTM
Description
A pass over every open issue in this repository (88 issues on 2026-10-07 01:37 UTC). Each report was checked on the current
develop(18ea0c2) against a local Open edX backend. The confirmed iOS defects are fixed here, one commit per issue. Reports that no longer reproduce are listed with the PR that fixed them, so the issues can be closed.Fixed here: #262, #438, #161, #419, #271, #531, #650, #455, #337, #576, #581, plus three bugs found on the way that had no issue: due dates shown as "Today", a calendar sync status stuck at "Offline", and video downloads the server refused shown as downloaded.
Already fixed on
develop: #359, #449, #568, #571, #355, #425, and #640 does not reproduce on iOS 26 / 27 (details below).Needs a decision rather than code: #555 (design), #569 (translation tooling), #383 and #489 (edX-specific / IAP).
How this was tested
master@0f57cf2b(Verawood-dev, Tutor dev), the Open edX demo course plus fixtures added for these reports:discussion_urlisnull([iOS] JSON parsing error is showing up in log when openeing the Dasbhoard viewmodel #359);AppVersionConfigrows ([iOS] Soft and Force upgrade not working as expected #449);default_config/devpointed at the local backend, and on the LMSVIDEO_CDN_URLpointed at the local clip server, so the video sizes come from VAL. The example instance's light and dark accent colours were left out, so the app's own palette applies (see "Found on the way" for why).developat 18ea0c2. After: a build of this branch. Same simulators, same accounts and data, a fresh install for every run.Each fix below has a close-up of the part of the screen the report is about, before on the left and after on the right. Where the bug plays out over time there is also a recording (iPhone, portrait, light), which plays in place. The full set of eight configurations, iPhone and iPad in portrait and landscape, light and dark, is under "All configurations".
Dates: due dates shown as "Today" (no issue filed)
Date(iso8601:)parsed server timestamps with the user's locale. When the 24-Hour Time switch is overridden in iOS Settings,DateFormatterreadsHHas a 12-hour field, so every timestamp after 12:59 failed to parse and silently became "now". Due dates showed as "Today" and were grouped wrongly. It now parses withen_US_POSIX(Apple QA1480), asDate(subtitleTime:)already did. This was found while validating #438 and #161; it affects the Dates screens in the grids below.Test:
CourseDateViewModelTests.testDateWithFractionalSecondsAfterNoon_isParsedExactlyDates: calendar sync status always "Offline" (no issue filed)
The card at the top of a course's Dates tab always said "Offline", also for a course synced to the calendar, while Settings > Dates & Calendar said "Synced". Its status was a
@Statedeclared as a local variable insidebody. SwiftUI keeps state only for a view's stored properties, so the value the.taskwrote was dropped and the card kept its initial.offline. The status is now a property of the view. In the other Dates captures calendar sync is off, and there "Offline" is the right label.There is no unit test for this one: the bug is in how SwiftUI stores view state, so the captures are the check.
Evidence
Course Dates tab with calendar sync on. Settings says "Synced" in both builds (next image).
Settings > Dates & Calendar, in each build
All configurations
Before:
develop@ 18ea0c2After: this branch
#438 Double refresh of the "Dates" tab when redirecting to it
Closes #438
Opening a course on the Dates tab (for example from "Past due assignments" on the dashboard) loaded the dates twice: once when the tab appeared and again when the course outline finished loading and posted
.getCourseDates. The second load replaced the list with a spinner. Each load also read the cached outline into a property nothing used; on a course opened for the first time that read failed and showed "Course component not found, please reload".The tab now skips a refresh while its own request runs, refreshes loaded dates quietly, ignores refresh requests for other courses, and no longer reads the outline.
Tests:
testGetCourseDates_whenCourseOutlineIsNotCachedYet_doesNotShowError,testRefreshRequestedByCourse_whenDatesAreShown_keepsThemOnScreenEvidence
Three seconds after tapping "View Assignments" on the Learn tab. The notifications prompt is iOS asking on the first course open, in both builds.
The past-due list once loaded (the date fix)
Recording, iPhone, portrait, light. The clock is recording time.
All configurations
Before:
develop@ 18ea0c2After: this branch
#161 Option "Shift due date" and #419 loading indicator over "Shift due dates"
Closes #161
Closes #419
"Shift due dates" on a course's Dates tab moved the deadlines on the server, but the tab kept showing "Missed some deadlines?" and the old past-due list with no confirmation. The tab posts
.shiftCourseDateswith a(courseID, courseName)tuple, as the app-level Dates screen and calendar sync expect, but the tab and the course container only listened for a plain course ID and dropped it. The container also posted a plain ID, so calendar events never updated after a shift started there. All senders now post the tuple, listeners react only to their own course, and the tab reloads and shows the success message.The overlap in #419 belonged to the 2024 course home layout, which has since been replaced. On the current Dates tab, a shift replaced the whole list with a full-screen spinner, which on a slow connection looked like the screen had reset. The banner now shows a spinner in place of its button while the request runs, as the app-level Dates screen does (a 3 s server delay was added for these captures).
Tests:
testShiftDueDates_reloadsDatesAndConfirmsTheShift,testShiftDueDatesInAnotherCourse_doesNotTouchThisCourseEvidence
While the shift request runs (3 s server delay)
Once it has finished
Recording, iPhone, portrait, light. The clock is recording time.
All configurations
Before:
develop@ 18ea0c2After: this branch
#262 Section completion checkmark appears only after re-entering the course
Closes #262
After the last block of a section was completed, the app reloaded the outline and the server already reported the section complete, but the outline kept the old marks and progress bar until the course was closed and opened again.
CourseStructure,CourseChapterandCourseSequentialcompared equal by id alone, and SwiftUI skips redrawing a view whose inputs compare equal, so the refreshed outline was never drawn (the "1/7 Sections Completed" counter, which reads the view model directly, did update). They now use memberwise equality, asCourseVerticalalready did. The same change also redraws new due dates and assignment progress after a shift.The captures were taken right after "Good job! You've completed Conclusion" → Back to outline, without leaving the course. Rotating the device rebuilds the outline and brings the mark back even without the fix, so portrait and landscape were captured in separate runs, each completing the section in that orientation.
Test:
testOutlineInputs_withNewCompletion_doNotCompareEqualEvidence
Back on the outline after finishing Conclusion, portrait
The same in landscape, in its own run
Recording, iPhone, portrait, light. The clock is recording time.
All configurations
Before:
develop@ 18ea0c2After: this branch
#271 Calendar integration deep links
Closes #271
Links added to synced calendar events carry a
course_id, which the app uses to open the course (DeepLinkManager.showCourseScreen). Since the calendar sync rework in #466 that field held the calendar's name instead of the course ID, so a link from an event could not find its course. The course ID is passed down again, as before #466. Branch is not configured in local builds, so this is covered by a unit test rather than a capture.Test:
CalendarManagerTests.testCalendarEventLink_opensTheCourseTheEventBelongsTo#531 Text on picture is not visible in Dark Mode
Closes #531
Dark mode inverts course HTML and inverts images back, so photos keep their colours. Figures exported as PNGs with dark ink on a transparent background then show that ink straight on the dark page. Images now get a white backing in dark mode; the double inversion keeps it white, so the figure reads as it does on the web. Opaque images are unchanged.
Evidence
Dark mode. In light mode the two builds are the same.
All configurations
Before:
develop@ 18ea0c2After: this branch
#650 Completion logic for assignments
Closes #650
An assignment counted as done only when the whole subsection was complete, so an unwatched video kept it "incomplete" or "past due" after every problem was submitted. It is now done once each graded problem in it has been attempted. The server marks a problem complete when it is submitted, whatever the score, so block completion is the attempt signal. In the captures, "Graded homework" is BAS2 under Basic Assessment Tools: both of its problems are attempted and its clip is not watched. Before, it stays open (0 of 2 assignments done); after, it is done (1 of 2).
Tests:
testAssignmentWithEveryGradedProblemAttempted_isCompleted_evenWithAnUnwatchedVideo,testAssignmentWithAnUnattemptedGradedProblem_isNotCompletedEvidence
Content > Assignments, Basic Assessment Tools
All configurations
Before:
develop@ 18ea0c2After: this branch
#455 Section level video download progress visibility
Closes #455
While a section's videos were downloading, tapping its progress indicator (Home, Content, Videos) cancelled every download in the section at once. The course-level download bar already opens the downloads list, where each video shows its progress and can be cancelled on its own; section indicators now open the same list while downloading. The close-up shows the section while it downloads and what tapping its indicator does; the recording shows the tap.
Test:
testSectionIndicatorWhileDownloading_opensDownloadsListInsteadOfCancellingEvidence
After tapping the section's indicator while its videos download. Before, every download in the section is cancelled and the plain download icon is back; after, the downloads list opens and the downloads go on.
Recording, iPhone, portrait, light. The clock is recording time.
All configurations
Before:
develop@ 18ea0c2After: this branch
Downloads: a video the server refused showed as downloaded (no issue filed)
Video downloads didn't check the HTTP status. When the server answered with an error, such as a 403 from a CDN for an expired or removed link, the error page was saved under the video's file name and the video showed as downloaded. Tapping its indicator again offered to free up the full size of the video, and playing it offline failed.
Downloads are now validated. A download that fails for a reason other than the connection is dropped from the queue, its file removed, and listed in the existing "download failed" prompt once the queue is done, the way a download that started offline already is. Before, such a download stayed in the queue and the next pass picked it up again straight away. The Alamofire session can now be passed in, so a test can answer with an error.
This turned up in the first captures for #455: the LMS rewrites VAL video URLs to
edx-video.net, which answers 403 to anything it doesn't host, and every video "downloaded" in under a second.Test:
DownloadManagerTests.testNewDownload_WhenServerRefusesTheFile_ReportsItAsFailedNotDownloaded. It fails without.validate(), and also when the failed download stays in the queue.Evidence
Six seconds after tapping the download button of a section whose only video the server no longer has. Before, the green check marks the section as downloaded; after, the download failed prompt.
Recording, iPhone, portrait, light. The clock is recording time.
All configurations
Before:
develop@ 18ea0c2After: this branch
#337 Blocked course can be opened in the app
Closes #337
The enrollments API reports
courseware_access.has_access = false, for example when an audit learner's access has expired, and the dashboard already shows a lock on such courses. Tapping one still opened the course and loaded its content and videos. The course screen now shows "You don't currently have access to this course" with a back button and doesn't request the content. A course that hasn't started yet keeps its existing screen.On current
master, the blocks API answers 500 for such a learner (get_assignments_gradesreturns[]and the view unpacks two values), which is why the "before" capture spins instead of showing content.Evidence
Opening a course whose audit access has expired
All configurations
Before:
develop@ 18ea0c2After: this branch
#576 CourseDownloadHelperTests exceed time limit on GitHub
Closes #576
The timeouts came from the helper, not from the tests. Since #562,
CourseDownloadHelper.refreshValue()has refreshed its value insideTask(priority: .background). A background task can wait seconds for a thread when the machine is busy, so on CI the tests waiting for that refresh ran out of time, and on a busy device the download buttons lag behind the downloads. The tests were commented out in #575 and deleted in #652.The refresh now runs at the caller's priority. The tests are back as async tests that wait with
fulfillment(of:)and check the Mockolo call counters.The CI runs of this PR show the difference. With only the tests restored (run), awaiting the async
refreshValue()took 0.004 s, the syncrefreshValue()took 7.9 s, and the refresh triggered by a download event had not started after 15 s, so that test failed. With the priority fixed (run), all three finished in 0.013 s or less.#581 App blocked by a spinner on a network that doesn't work
Closes #581
On a network that connects but doesn't carry traffic, the Learn screen spun until its request timed out after 60 s and then showed "No Courses" with "Something went wrong", although the courses were saved from the last visit. The connectivity check from #635 notices such a network after about 30 s and shows the offline banner, but screens only looked at it when a request started.
Saved data is only used where offline mode already uses it, and never ahead of a working network. The dashboard cache (
CDMyEnrollments) is not scoped to a user or an instance, so showing it before the network answers could show another account's courses.Every API request was held for 180 s. The captures were taken 72 s after launch, once the stuck request had timed out; the recording shows the whole wait.
Tests:
testGetEnrollments_whenTheRequestFails_showsTheSavedCourses,testGoingOfflineWhileLoading_showsTheSavedCoursesBeforeTheRequestTimesOut,ListDashboardViewModelTests.testGetMyCourses_whenTheRequestFails_showsTheSavedCourses,testCourseStructure_whenTheNetworkFails_opensTheSavedCopyEvidence
Learn tab 72 s after launch, every request held for 180 s
Recording, iPhone, portrait, light. The clock is recording time.
All configurations
Before:
develop@ 18ea0c2After: this branch
Already fixed on
develop: these issues can be closedEach was re-run on 18ea0c2 in the same 8 configurations.
discussion_urlisnull.MOBILE_APP_USER_AGENT_REGEXES; the LMS default only matchesedX/org.edx.mobile, and without it the upgrade middleware never sees the app.rendered_bodyfrom edx-platform's Markdown, which merges the two lists into one<ol>.SearchViewhas hidden the navigation bar with.navigationBarHidden(true)since 2023, so the duplicate seen in June 2026 was probably OS-version specific. An iOS 18 runtime isn't available for Xcode 27, so it couldn't be re-checked there.Captures for the issues above, on 18ea0c2
#359: Learn tab for a course whose
discussion_urlisnullAll configurations
#449: soft upgrade, the banner on Learn and the version row in Settings
All configurations
#449: force upgrade after a 426
All configurations
#568 / #571: Edit Profile for a learner without a year of birth
All configurations
#355: formatted discussion post
All configurations
#425: course details while signed out
All configurations
#640: Discover search while signed in
All configurations
#555: Learn header (still as reported)
All configurations
Needs a decision rather than code
Localizable.stringsdictin Course, Discovery and Discussion), buti18n_scripts/translation.pyonly reads and writesLocalizable.strings, so plural keys never reach Transifex. The comments propose a custom plural function instead of supporting another file format. That is a tooling decision for the maintainers.Other open issues
The other 69 open issues are mostly epics, design tasks, features, backend or process items. Two of them, #271 and #650, turned out to be bugs and are fixed above. The table records each one's state on 18ea0c2 and, where it was built, the merged PR. Many can be closed.
All 69 other open issues
develop(18ea0c2)sourceScreen: .discoveryand never returns to the course (DiscoveryWebviewViewModel.swift:72).BROWSER_LOGIN, and Kelketek's 2024-12 question was never answered.course_id; see the #271 section above.sequential.due < now) or in the LMS.Found on the way, not changed here
CourseDownloadHelper.enumeratereturns from itswithCheckedContinuationblock without resuming when there isn't enough disk space to re-download an updated video, sorefreshValue()never returns. It needs a decision on whether to skip that block or stop the whole pass.master:/api/mobile/v4/course_info/blocks/answers 500 for a learner whose audit access has expired, and/api/mobile/v3/course_dates/<username>/(app-level Dates) is 404.default_config/dev/config.jsongives the example instance a dark accent colour of#1C355E. The app draws links and accent text in that colour, which is 1.3:1 against the dark page background (#18202E) and 1.05:1 against a card, so in a dev build every link in dark mode is close to invisible ("Tap to update", "Switch to full profile", links in posts). The app's own dark accent,#5378F8, gives 4.2:1.AI assistance
This PR was prepared with Bulava, a Mac app that runs Claude Code and Codex as a pair, in line with the Open edX AI contribution policy. Claude Code checked each issue against a local backend, wrote the fixes and tests, and drove the simulators for the captures. Codex reviewed the plan for the sweep and was consulted on the #581 fallback and the #576 task priority. Every claim above was checked on a local build, and the tests named in each section fail without their fix. The PR stays a draft until a maintainer has reviewed and understood each change.