Skip to content

Grade list grading period filter auto selects the current period instead of reloading the last selection #3314

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,6 @@ public struct SessionDefaults: Equatable {
}

// MARK: - Grades
public var selectedGradingPeriodIdsByCourseIDs: [String: String]? {
get { self["selectedGradingPeriodIdsByCourseIDs"] as? [String: String] }
set { self["selectedGradingPeriodIdsByCourseIDs"] = newValue }
}

public var selectedSortByOptionIDs: [String: String]? {
get { self["selectedSortByOptionIDs"] as? [String: String] }
set { self["selectedSortByOptionIDs"] = newValue }
Expand Down
15 changes: 0 additions & 15 deletions Core/Core/Features/Grades/Model/GradeFilterInteractor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ import Foundation

public protocol GradeFilterInteractor {
var gradingShowAllId: String { get }
var selectedGradingId: String? { get }
var selectedSortById: String? { get }
var isParentApp: Bool { get }
func saveSelectedGradingPeriod(id: String?)
func saveSortByOption(type: GradeArrangementOptions)
}

Expand Down Expand Up @@ -53,23 +51,10 @@ extension GradeFilterInteractorLive: GradeFilterInteractor {
appEnvironment.app == .parent
}

public var selectedGradingId: String? {
appEnvironment.userDefaults?.selectedGradingPeriodIdsByCourseIDs?[courseId]
}

public var selectedSortById: String? {
appEnvironment.userDefaults?.selectedSortByOptionIDs?[courseId]
}

public func saveSelectedGradingPeriod(id: String?) {
let gradingId = id ?? gradingShowAllId
if appEnvironment.userDefaults?.selectedGradingPeriodIdsByCourseIDs == nil {
appEnvironment.userDefaults?.selectedGradingPeriodIdsByCourseIDs = [courseId: gradingId]
} else {
appEnvironment.userDefaults?.selectedGradingPeriodIdsByCourseIDs?[courseId] = gradingId
}
}

public func saveSortByOption(type: GradeArrangementOptions) {
let sortById = type.rawValue
if appEnvironment.userDefaults?.selectedSortByOptionIDs == nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public final class GradeListInteractorLive: GradeListInteractor {
loadAllPages: true
),
enrollmentListStore.getEntities(
ignoreCache: ignoreCache,
ignoreCache: true,
loadAllPages: true
)
)
Expand Down
1 change: 1 addition & 0 deletions Core/Core/Features/Grades/View/GradeFilterView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public struct GradeFilterView: View {
dependency: .init(
router: AppEnvironment.shared.router,
isShowGradingPeriod: false,
initialGradingPeriodID: nil,
sortByOptions: GradeArrangementOptions.allCases
),
gradeFilterInteractor: GradeFilterInteractorLive(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public final class GradeFilterViewModel: ObservableObject {
return
}

let initialGradingPeriod = gradingPeriods.first { $0.id == gradeFilterInteractor.selectedGradingId }
let initialGradingPeriod = gradingPeriods.first { $0.id == dependency.initialGradingPeriodID }
gradingPeriodOptions = .init(
all: [GradingPeriod.optionItemAll] + gradingPeriods.map { $0.optionItem },
initial: initialGradingPeriod?.optionItem ?? GradingPeriod.optionItemAll
Expand Down Expand Up @@ -101,7 +101,6 @@ public final class GradeFilterViewModel: ObservableObject {
let optionId = gradingPeriodOptions.selected.value?.id
let selectedGradingPeriodId = optionId == OptionItem.allId ? nil : optionId
dependency.selectedGradingPeriodPublisher.accept(selectedGradingPeriodId)
gradeFilterInteractor.saveSelectedGradingPeriod(id: selectedGradingPeriodId)

dependency.router.dismiss(viewController) {
UIAccessibility.announce(String(localized: "Filter applied successfully", bundle: .core))
Expand All @@ -118,6 +117,7 @@ extension GradeFilterViewModel {
public struct Dependency {
let router: Router
let isShowGradingPeriod: Bool
let initialGradingPeriodID: String?
var courseName: String?
var selectedGradingPeriodPublisher = PassthroughRelay<String?>()
var selectedSortByPublisher = CurrentValueRelay<GradeArrangementOptions>(.dueDate)
Expand Down
52 changes: 10 additions & 42 deletions Core/Core/Features/Grades/ViewModel/GradeListViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,38 +142,31 @@ public final class GradeListViewModel: ObservableObject {
.store(in: &subscriptions)

loadSortPreferences()
loadBaseDataAndGrades(ignoreCache: false)
loadBaseDataAndGrades(ignoreCache: false, isInitialLoad: true)
}

private func loadBaseDataAndGrades(ignoreCache: Bool, completionBlock: (() -> Void)? = nil) {
private func loadBaseDataAndGrades(ignoreCache: Bool, isInitialLoad: Bool = false, completionBlock: (() -> Void)? = nil) {
interactor
.loadBaseData(ignoreCache: ignoreCache)
.map { [weak self] gradingPeriodData in
self?.calculateGradingPeriodToShow(gradingPeriodData)
// Initial loading selects the currently active grading period
if isInitialLoad {
self?.selectedGradingPeriod = gradingPeriodData.currentlyActiveGradingPeriodID
}
}
.mapToVoid()
.receive(on: scheduler)
.sink(receiveCompletion: { [weak self] completion in
.sink { [weak self] completion in
if case .failure = completion {
self?.state = .error
completionBlock?()
}
}, receiveValue: { [triggerGradeRefresh] in
} receiveValue: { [triggerGradeRefresh] in
triggerGradeRefresh.accept((ignoreCache, completionBlock))
})
}
.store(in: &subscriptions)
}

private func calculateGradingPeriodToShow(_ gradingPeriodData: GradeListGradingPeriodData) {
let id = Self.getSelectedGradingPeriodId(
gradeFilterInteractor: gradeFilterInteractor,
currentGradingPeriodID: gradingPeriodData.currentlyActiveGradingPeriodID,
gradingPeriods: gradingPeriodData.gradingPeriods
)

selectedGradingPeriod = id
}

private func refreshGrades(ignoreCache: Bool) -> AnyPublisher<Void, Never> {
interactor.getGrades(
arrangeBy: selectedGroupByOption.value,
Expand Down Expand Up @@ -213,32 +206,6 @@ public final class GradeListViewModel: ObservableObject {
selectedGroupByOption.accept(option)
}

private static func getSelectedGradingPeriodId(
gradeFilterInteractor: GradeFilterInteractor,
currentGradingPeriodID: String?,
gradingPeriods: [GradingPeriod]
) -> String? {
let currentId = gradeFilterInteractor.selectedGradingId
guard !gradingPeriods.isEmpty else {
gradeFilterInteractor.saveSelectedGradingPeriod(id: currentGradingPeriodID)
return currentGradingPeriodID
}

if let currentId {
if currentId == gradeFilterInteractor.gradingShowAllId {
return nil
} else if gradingPeriods.contains(where: { $0.id == currentId }) {
return currentId
} else {
gradeFilterInteractor.saveSelectedGradingPeriod(id: gradingPeriods.first?.id)
gradeFilterInteractor.saveSortByOption(type: .dueDate)
return gradingPeriods.first?.id
}
}
gradeFilterInteractor.saveSelectedGradingPeriod(id: currentGradingPeriodID)
return currentGradingPeriodID
}

func navigateToFilter(viewController: WeakViewController) {
let gradeData: GradeListData? = {
switch state {
Expand All @@ -254,6 +221,7 @@ public final class GradeListViewModel: ObservableObject {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: isShowGradingPeriod,
initialGradingPeriodID: selectedGradingPeriod,
courseName: courseName,
selectedGradingPeriodPublisher: didSelectGradingPeriod,
selectedSortByPublisher: selectedGroupByOption,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,38 +43,6 @@ final class GradeFilterInteractorLiveTests: CoreTestCase {
XCTAssertEqual(testee.gradingShowAllId, "-1")
}

func test_saveGradingForFistTime() {
// Given
let id = "10"
environment.userDefaults?.selectedGradingPeriodIdsByCourseIDs = nil
// When
testee.saveSelectedGradingPeriod(id: id)
// Then
XCTAssertEqual(testee.selectedGradingId, id)
}

func test_saveGradingChangeValue() {
// Given
let oldValue = "10"
let newValue = "20"
environment.userDefaults?.selectedGradingPeriodIdsByCourseIDs = [courseId: oldValue]
// When
testee.saveSelectedGradingPeriod(id: newValue)
// Then
XCTAssertEqual(testee.selectedGradingId, newValue)
}

func test_saveGrading_whenSaveShowAll() {
// Given
let oldValue = "10"
let newValue: String? = nil
environment.userDefaults?.selectedGradingPeriodIdsByCourseIDs = [courseId: oldValue]
// When
testee.saveSelectedGradingPeriod(id: newValue)
// Then
XCTAssertEqual(testee.selectedGradingId, "-1")
}

func test_saveSortByOptionForFirstTime() {
// Given
environment.userDefaults?.selectedSortByOptionIDs = nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ final class GradeFilterViewModelTests: CoreTestCase {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: true,
initialGradingPeriodID: nil,
sortByOptions: GradeArrangementOptions.allCases
)
testee = GradeFilterViewModel(
Expand All @@ -55,11 +56,11 @@ final class GradeFilterViewModelTests: CoreTestCase {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: true,
initialGradingPeriodID: nil,
gradingPeriods: listGradingPeriods,
sortByOptions: GradeArrangementOptions.allCases
)
// When
environment.userDefaults?.selectedGradingPeriodIdsByCourseIDs = nil
let testee = GradeFilterViewModel(
dependency: dependency,
gradeFilterInteractor: gradeFilterInteractor
Expand All @@ -72,13 +73,14 @@ final class GradeFilterViewModelTests: CoreTestCase {
XCTAssertEqual(testee.isShowGradingPeriodsView, true)
}

func test_mapGradingPeriod_hasSelectedGradingPeriods() {
func test_mapGradingPeriod_selectsInitialGradingPeriod() {
// Given
let listGradingPeriods = getListGradingPeriods()
gradeFilterInteractor.currentGradingId = "4"
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: true,
initialGradingPeriodID: "1",
gradingPeriods: listGradingPeriods,
sortByOptions: GradeArrangementOptions.allCases
)
Expand All @@ -90,14 +92,15 @@ final class GradeFilterViewModelTests: CoreTestCase {
// Then
XCTAssertEqual(testee.gradingPeriodOptions.all.count, 5)
XCTAssertEqual(testee.isShowGradingPeriodsView, true)
XCTAssertEqual(testee.gradingPeriodOptions.selected.value?.id, "4")
XCTAssertEqual(testee.gradingPeriodOptions.selected.value?.id, "1")
}

func test_mapGradingPeriod_notHasGradingPeriods_hideGradingPeriodSection() {
// Given
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: false,
initialGradingPeriodID: nil,
gradingPeriods: nil,
sortByOptions: GradeArrangementOptions.allCases
)
Expand All @@ -119,6 +122,7 @@ final class GradeFilterViewModelTests: CoreTestCase {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: false,
initialGradingPeriodID: nil,
gradingPeriods: nil,
sortByOptions: sortByOptions
)
Expand All @@ -137,6 +141,7 @@ final class GradeFilterViewModelTests: CoreTestCase {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: true,
initialGradingPeriodID: nil,
gradingPeriods: listGradingPeriods,
sortByOptions: GradeArrangementOptions.allCases
)
Expand All @@ -156,6 +161,7 @@ final class GradeFilterViewModelTests: CoreTestCase {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: true,
initialGradingPeriodID: nil,
gradingPeriods: listGradingPeriods,
sortByOptions: GradeArrangementOptions.allCases
)
Expand All @@ -181,6 +187,7 @@ final class GradeFilterViewModelTests: CoreTestCase {
let dependency = GradeFilterViewModel.Dependency(
router: router,
isShowGradingPeriod: true,
initialGradingPeriodID: nil,
selectedGradingPeriodPublisher: selectedGradingPeriodPublisher,
selectedSortByPublisher: selectedSortByPublisher,
gradingPeriods: listGradingPeriods,
Expand Down