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

feat: Alert for outdated course calendars on the course home screen #416

Merged
merged 4 commits into from
May 7, 2024
Merged

feat: Alert for outdated course calendars on the course home screen #416

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8887034
feat: Alert for outdated course calendars on the course home screen
shafqat-muneer Apr 26, 2024
c2e5a41
Merge remote-tracking branch 'origin/develop' into Shafqat/LEARNER-9955
shafqat-muneer May 6, 2024
fbbcade
Merge remote-tracking branch 'upstream/develop' into Shafqat/LEARNER-…
shafqat-muneer May 6, 2024
631dab4
chore: address review feedback
shafqat-muneer May 6, 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
39 changes: 38 additions & 1 deletion Course/Course/Presentation/Container/CourseContainerView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public struct CourseContainerView: View {

@ObservedObject
public var viewModel: CourseContainerViewModel
@ObservedObject
public var courseDatesViewModel: CourseDatesViewModel
@State private var isAnimatingForTap: Bool = false
public var courseID: String
private var title: String
Expand Down Expand Up @@ -55,6 +57,7 @@ public struct CourseContainerView: View {
}
self.courseID = courseID
self.title = title
self.courseDatesViewModel = viewModel.courseDatesViewModel
}

public var body: some View {
Expand Down Expand Up @@ -112,8 +115,32 @@ public struct CourseContainerView: View {
}
}
}

switch courseDatesViewModel.eventState {
case .removedCalendar:
showDatesSuccessView(
title: CourseLocalization.CourseDates.calendarEvents,
message: CourseLocalization.CourseDates.calendarEventsRemoved
)
case .updatedCalendar:
showDatesSuccessView(
title: CourseLocalization.CourseDates.calendarEvents,
message: CourseLocalization.CourseDates.calendarEventsUpdated
)
default:
EmptyView()
}
}

private func showDatesSuccessView(title: String, message: String) -> some View {
return DatesSuccessView(
title: title,
message: message
) {
courseDatesViewModel.resetEventState()
}
}

private func backButton(containerWidth: CGFloat) -> some View {
ZStack(alignment: .topLeading) {
if !collapsed {
Expand Down Expand Up @@ -311,7 +338,17 @@ struct CourseScreensView_Previews: PreviewProvider {
courseEnd: nil,
enrollmentStart: nil,
enrollmentEnd: nil,
coreAnalytics: CoreAnalyticsMock()
coreAnalytics: CoreAnalyticsMock(),
courseDatesViewModel: CourseDatesViewModel(
interactor: CourseInteractor.mock,
router: CourseRouterMock(),
cssInjector: CSSInjectorMock(),
connectivity: Connectivity(),
config: ConfigMock(),
courseID: "1",
courseName: "a",
analytics: CourseAnalyticsMock()
)
),
courseID: "", title: "Title of Course")
}
Expand Down
8 changes: 7 additions & 1 deletion Course/Course/Presentation/Container/CourseContainerViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public class CourseContainerViewModel: BaseCourseViewModel {
private let authInteractor: AuthInteractorProtocol
let analytics: CourseAnalytics
let coreAnalytics: CoreAnalytics
var courseDatesViewModel: CourseDatesViewModel
private(set) var storage: CourseStorage

public init(
Expand All @@ -109,7 +110,8 @@ public class CourseContainerViewModel: BaseCourseViewModel {
courseEnd: Date?,
enrollmentStart: Date?,
enrollmentEnd: Date?,
coreAnalytics: CoreAnalytics
coreAnalytics: CoreAnalytics,
courseDatesViewModel: CourseDatesViewModel
) {
self.interactor = interactor
self.authInteractor = authInteractor
Expand All @@ -126,6 +128,7 @@ public class CourseContainerViewModel: BaseCourseViewModel {
self.userSettings = storage.userSettings
self.isInternetAvaliable = connectivity.isInternetAvaliable
self.coreAnalytics = coreAnalytics
self.courseDatesViewModel = courseDatesViewModel

super.init(manager: manager)
addObservers()
Expand All @@ -140,6 +143,9 @@ public class CourseContainerViewModel: BaseCourseViewModel {
do {
if isInternetAvaliable {
courseStructure = try await interactor.getCourseBlocks(courseID: courseID)
Task {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCourseBlocks is async function, why are you using Task there? Don't we need to wait for dates before will set isShowProgress and isShowRefresh to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the course home screen, we are executing the getCourseDates API call solely to determine whether to display an outdated calendar alert or not. There is no UI population involving dates at this stage. Hence, we can optimize the loading time of this screen by running the getCourseDates API call on a separate thread. Once we receive the response regarding the dates, we can then decide whether to present the outdated alert or not.

Additionally, it's essential to populate getCourseBlocks before invoking getCourseDates because the getCourseDates method involves offline population of courseStructure.

await courseDatesViewModel.getCourseDates(courseID: courseID)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shafqat-muneer, I don't know if this can be tested. We can't check what's going on inside the Task.
And this dependency on CourseDatesViewViewModel 🤔.
But I can't offer a good solution right now. Only we could send notification to CourseDatesViewModel instead of calling it directly.
What do you think?

In any case, the calendar flow will be heavily modified in FC-0047, so we can probably make do with this approach for now.

isShowProgress = false
isShowRefresh = false
if let courseStructure {
Expand Down
3 changes: 1 addition & 2 deletions Course/Course/Presentation/Dates/CourseDatesView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ public struct CourseDatesView: View {
} else {
return DatesSuccessView(
title: title,
message: message,
selectedTab: .dates
message: message
) {
viewModel.resetEventState()
}
Expand Down
12 changes: 11 additions & 1 deletion Course/Course/Presentation/Outline/CourseOutlineView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,17 @@ struct CourseOutlineView_Previews: PreviewProvider {
courseEnd: nil,
enrollmentStart: Date(),
enrollmentEnd: nil,
coreAnalytics: CoreAnalyticsMock()
coreAnalytics: CoreAnalyticsMock(),
courseDatesViewModel: CourseDatesViewModel(
interactor: CourseInteractor.mock,
router: CourseRouterMock(),
cssInjector: CSSInjectorMock(),
connectivity: Connectivity(),
config: ConfigMock(),
courseID: "1",
courseName: "a",
analytics: CourseAnalyticsMock()
)
)
Task {
await withTaskGroup(of: Void.self) { group in
Expand Down
7 changes: 2 additions & 5 deletions Course/Course/Views/DatesSuccessView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public struct DatesSuccessView: View {

private var title: String
private var message: String
var selectedTab: Tab
var selectedTab: Tab?
var courseDatesViewModel: CourseDatesViewModel?
var courseContainerViewModel: CourseContainerViewModel?
var action: () -> Void = {}
Expand All @@ -29,12 +29,10 @@ public struct DatesSuccessView: View {
init (
title: String,
message: String,
selectedTab: Tab,
dismissAction: @escaping () -> Void
) {
self.title = title
self.message = message
self.selectedTab = selectedTab
self.dismissAction = dismissAction
}

Expand Down Expand Up @@ -149,8 +147,7 @@ struct DatesSuccessView_Previews: PreviewProvider {
static var previews: some View {
DatesSuccessView(
title: CourseLocalization.CourseDates.toastSuccessTitle,
message: CourseLocalization.CourseDates.toastSuccessMessage,
selectedTab: .course
message: CourseLocalization.CourseDates.toastSuccessMessage
) {}
}
}
Expand Down
Loading
Loading