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.

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

Fix issue with assignments without grading period not being shown on Assignment List #2969

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
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 @@ -57,7 +57,7 @@ public class AssignmentListViewModel: ObservableObject {
@Published public private(set) var isShowingGradingPeriods: Bool = false

public var isFilterIconSolid: Bool = false
public let defaultGradingPeriod: GradingPeriod?
public var defaultGradingPeriod: GradingPeriod?
public let defaultSortingOption: AssignmentArrangementOptions = .dueDate
public var selectedGradingPeriod: GradingPeriod?
public var selectedSortingOption: AssignmentArrangementOptions = .dueDate
Expand All @@ -67,48 +67,69 @@ public class AssignmentListViewModel: ObservableObject {
private let sortingOptions = AssignmentArrangementOptions.allCases
private var initialFilterOptions: [AssignmentFilterOption] = AssignmentFilterOption.allCases
private var selectedFilterOptions: [AssignmentFilterOption] = AssignmentFilterOption.allCases

private var isFilteringCustom: Bool {
// all filters selected is the same as no filter selected
!selectedFilterOptions.isEmpty && selectedFilterOptions.count != AssignmentFilterOption.allCases.count
}

private let env = AppEnvironment.shared
private var userDefaults: SessionDefaults?
let courseID: String

public private(set) lazy var gradingPeriods: Store<LocalUseCase<GradingPeriod>> = {
let scope: Scope = .where(
#keyPath(GradingPeriod.courseID),
equals: courseID,
orderBy: #keyPath(GradingPeriod.startDate)
)
return env.subscribe(LocalUseCase(scope: scope))
}()
private lazy var course = env.subscribe(GetCourse(courseID: courseID)) { [weak self] in
self?.courseDidUpdate()
}

private lazy var assignmentGroups = env.subscribe(
GetAssignmentsByGroup(courseID: courseID, gradingPeriodID: defaultGradingPeriod?.id)
) { [weak self] in
self?.assignmentGroupsDidUpdate()
private lazy var gradingPeriods = env.subscribe(GetGradingPeriods(courseID: courseID)) { [weak self] in
guard let self else { return }
if selectedGradingPeriod == nil {
defaultGradingPeriod = currentGradingPeriod
selectedGradingPeriod = defaultGradingPeriod
}
assignmentGroups.refresh()
}

private lazy var course = env.subscribe(GetCourse(courseID: courseID)) { [weak self] in
self?.courseDidUpdate()
private lazy var assignmentGroups = env.subscribe(GetAssignmentGroups(courseID: courseID)) { [weak self] in
self?.assignmentGroupsDidUpdate()
}

/** This is required for the router to help decide if the hybrid discussion details or the native one should be launched. */
private lazy var featureFlags = env.subscribe(GetEnabledFeatureFlags(context: .course(courseID)))

private var currentGradingPeriod: GradingPeriod? {
gradingPeriods.filter {
let rightNow = Clock.now
if let start = $0.startDate, let end = $0.endDate {
return start < rightNow && end > rightNow
}
return false
}
.first
}

// MARK: - Init
public init(
context: Context,
userDefaults: SessionDefaults? = AppEnvironment.shared.userDefaults,
defaultGradingPeriod: GradingPeriod? = nil
userDefaults: SessionDefaults? = AppEnvironment.shared.userDefaults
) {
self.userDefaults = userDefaults
self.courseID = context.id
self.defaultGradingPeriod = defaultGradingPeriod
self.selectedGradingPeriod = self.defaultGradingPeriod

featureFlags.refresh()
}

// MARK: - Functions

public func viewDidAppear() {
loadAssignmentListPreferences()
course.refresh(force: true)
gradingPeriods.refresh(force: true)
filterOptionsDidUpdate(filterOptions: selectedFilterOptions, gradingPeriod: selectedGradingPeriod)

isFilterIconSolid = isFilteringCustom || selectedGradingPeriod != defaultGradingPeriod
}

public func filterOptionsDidUpdate(
filterOptions: [AssignmentFilterOption]? = nil,
sortingOption: AssignmentArrangementOptions? = nil,
Expand All @@ -124,46 +145,33 @@ public class AssignmentListViewModel: ObservableObject {
selectedSortingOption = sortingOption ?? selectedSortingOption
selectedFilterOptions = filterOptions ?? selectedFilterOptions

isFilterIconSolid = gradingPeriod != defaultGradingPeriod
|| ![0, AssignmentFilterOption.allCases.count].contains(selectedFilterOptions.count)
&& selectedFilterOptions != initialFilterOptions
isFilterIconSolid = gradingPeriod != defaultGradingPeriod || isFilteringCustom && selectedFilterOptions != initialFilterOptions

assignmentGroups = env.subscribe(GetAssignmentsByGroup(courseID: courseID, gradingPeriodID: gradingPeriod?.id)) { [weak self] in
self?.assignmentGroupsDidUpdate()
}
assignmentGroups.refresh()
}

public func viewDidAppear() {
loadAssignmentListPreferences()
gradingPeriods.refresh()
filterOptionsDidUpdate(filterOptions: selectedFilterOptions, gradingPeriod: defaultGradingPeriod)
course.refresh()
assignmentGroups.refresh(force: true)

isFilterIconSolid = ![0, AssignmentFilterOption.allCases.count].contains(selectedFilterOptions.count)
assignmentGroupsDidUpdate()
}

private func assignmentGroupsDidUpdate() {
if !assignmentGroups.requested || assignmentGroups.pending { return }
if !gradingPeriods.requested || gradingPeriods.pending { return }

isShowingGradingPeriods = gradingPeriods.count > 1
var assignmentGroupViewModels: [AssignmentGroupViewModel] = []
let assignments: [Assignment] = filterAssignments(assignmentGroups.compactMap { $0 })
let compactAssignmentGroups = assignmentGroups
.compactMap { $0.assignments }
.map { Array($0) }
.flatMap { $0 }
let assignments: [Assignment] = filterAssignments(compactAssignmentGroups)

switch selectedSortingOption {
case .groupName:
for section in 0..<(assignmentGroups.sections?.count ?? 0) {
if let group = assignmentGroups[IndexPath(row: 0, section: section)]?.assignmentGroup {
let groupAssignments: [Assignment] = assignments.filter { $0.assignmentGroup == group }
if !groupAssignments.isEmpty {
assignmentGroupViewModels.append(AssignmentGroupViewModel(
assignmentGroup: group,
assignments: groupAssignments,
courseColor: courseColor
))
}
}
assignmentGroups.forEach { group in
let groupAssignments: [Assignment] = assignments.filter { $0.assignmentGroup == group }
guard !groupAssignments.isEmpty else { return }
assignmentGroupViewModels.append(AssignmentGroupViewModel(
assignmentGroup: group,
assignments: groupAssignments,
courseColor: courseColor
))
}
case .dueDate:
let rightNow = Clock.now
Expand All @@ -189,10 +197,16 @@ public class AssignmentListViewModel: ObservableObject {
}

private func filterAssignments(_ assignments: [Assignment]) -> [Assignment] {
// Filtering by grading period except if "All" is selected (nil)
var assignments = assignments
if let selectedGradingPeriod {
assignments = assignments.filter { $0.submission?.gradingPeriodId == selectedGradingPeriod.id }
}

var filteredAssignments: [Assignment] = []

// all filter selected is the same as no filter selected
if selectedFilterOptions.count == AssignmentFilterOption.allCases.count || selectedFilterOptions.isEmpty {
guard isFilteringCustom else {
return assignments
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="23231" systemVersion="23G93" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier="">
<model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="23507" systemVersion="23G80" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier="">
<entity name="AccountNotification" representedClassName="Core.AccountNotification" syncable="YES">
<attribute name="endAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="iconRaw" attributeType="String"/>
Expand Down Expand Up @@ -997,6 +997,7 @@
<attribute name="grade" optional="YES" attributeType="String"/>
<attribute name="gradedAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="gradeMatchesCurrentSubmission" attributeType="Boolean" usesScalarValueType="YES"/>
<attribute name="gradingPeriodId" optional="YES" attributeType="String"/>
<attribute name="groupID" optional="YES" attributeType="String"/>
<attribute name="groupName" optional="YES" attributeType="String"/>
<attribute name="id" attributeType="String"/>
Expand Down
4 changes: 4 additions & 0 deletions Core/Core/Grades/Model/UseCase/GetAssignmentsByGroup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public class GetAssignmentsByGroup: UseCase {
.map { AssignmentGroupsByGradingPeriod(gradingPeriod: gradingPeriod, assignmentGroups: $0) }
}
.collect()
.flatMap { assignmentsByGradingPeriods in
getAssignmentGroups(nil)
.map { [AssignmentGroupsByGradingPeriod(gradingPeriod: nil, assignmentGroups: $0)] + assignmentsByGradingPeriods }
}
.eraseToAnyPublisher()
}
}
Expand Down
3 changes: 3 additions & 0 deletions Core/Core/Submissions/APISubmission.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public struct APISubmission: Codable, Equatable {
var grade: String?
var graded_at: Date?
let grade_matches_current_submission: Bool
let grading_period_id: ID?
let group: APISubmissionGroup?
let id: ID
let late: Bool?
Expand Down Expand Up @@ -152,6 +153,7 @@ extension APISubmission {
grade: String? = nil,
graded_at: Date? = nil,
grade_matches_current_submission: Bool = true,
grading_period_id: String? = nil,
group: APISubmissionGroup? = nil,
id: String = "1",
late: Bool = false,
Expand Down Expand Up @@ -187,6 +189,7 @@ extension APISubmission {
grade: grade,
graded_at: graded_at,
grade_matches_current_submission: grade_matches_current_submission,
grading_period_id: ID(grading_period_id),
group: group,
id: ID(id),
late: late,
Expand Down
2 changes: 2 additions & 0 deletions Core/Core/Submissions/Submission.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ final public class Submission: NSManagedObject, Identifiable {
@NSManaged public var grade: String?
@NSManaged public var gradedAt: Date?
@NSManaged public var gradeMatchesCurrentSubmission: Bool
@NSManaged public var gradingPeriodId: String?
@NSManaged public var groupID: String?
@NSManaged public var groupName: String?
@NSManaged public var id: String
Expand Down Expand Up @@ -161,6 +162,7 @@ extension Submission: WriteableModel {
model.grade = item.grade
model.gradedAt = item.graded_at
model.gradeMatchesCurrentSubmission = item.grade_matches_current_submission
model.gradingPeriodId = item.grading_period_id?.value
model.groupID = item.group?.id?.value
model.groupName = item.group?.name
model.id = item.id.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
//

import XCTest
import TestsFoundation
@testable import Core

class AssignmentListViewModelTests: CoreTestCase {
Expand All @@ -26,7 +26,7 @@ class AssignmentListViewModelTests: CoreTestCase {
XCTAssertEqual(testee.state, .loading)
XCTAssertNil(testee.courseName)
XCTAssertNil(testee.courseColor)
XCTAssertEqual(testee.gradingPeriods.count, 0)
XCTAssertNil(testee.defaultGradingPeriod)
}

func testCoursePropertiesUpdate() {
Expand All @@ -45,7 +45,13 @@ class AssignmentListViewModelTests: CoreTestCase {
}

func testFilterButtonVisibleWhenTwoGradingPeriodsAvailable() {
api.mock(GetGradingPeriods(courseID: "1"), value: [.make(id: "1", title: "GP1"), .make(id: "2", title: "GP2")])
api.mock(
GetGradingPeriods(courseID: "1"),
value: [
.make(id: "1", title: "GP1", start_date: .now.addMonths(-9), end_date: .now.addMonths(-3)),
.make(id: "2", title: "GP2", start_date: .now.addMonths(-3), end_date: .now.addMonths(3))
]
)

// GetAssignmentsByGroup iterates through all grading periods so we have to mock it not to make the whole fetch fail
var assignmentGroupRequest = GetAssignmentGroupsRequest(
Expand All @@ -66,7 +72,7 @@ class AssignmentListViewModelTests: CoreTestCase {

testee.viewDidAppear()

XCTAssertTrue(testee.gradingPeriods.count > 1)
XCTAssertEqual(testee.defaultGradingPeriod?.id, "2")
}

func testAssignmentsPopulate() {
Expand All @@ -92,13 +98,24 @@ class AssignmentListViewModelTests: CoreTestCase {
XCTFail("State doesn't contain any view models.")
return
}

guard let firstGroupViewModel = groupViewModels.filter({ $0.name == "AGroup1" }).first else {
XCTFail("AssignmentGroup1 View Model is not available.")
return
}

guard let secondGroupViewModel = groupViewModels.filter({ $0.name == "AGroup2" }).first else {
XCTFail("AssignmentGroup2 View Model is not available.")
return
}

XCTAssertEqual(groupViewModels.count, 2)
XCTAssertEqual(groupViewModels[0].name, "AGroup1")
XCTAssertEqual(groupViewModels[0].assignments.count, 1)
XCTAssertEqual(groupViewModels[0].assignments[0].name, "Assignment1")
XCTAssertEqual(groupViewModels[1].name, "AGroup2")
XCTAssertEqual(groupViewModels[1].assignments.count, 1)
XCTAssertEqual(groupViewModels[1].assignments[0].name, "Assignment2")
XCTAssertEqual(firstGroupViewModel.name, "AGroup1")
XCTAssertEqual(firstGroupViewModel.assignments.count, 1)
XCTAssertEqual(firstGroupViewModel.assignments[0].name, "Assignment1")
XCTAssertEqual(secondGroupViewModel.name, "AGroup2")
XCTAssertEqual(secondGroupViewModel.assignments.count, 1)
XCTAssertEqual(secondGroupViewModel.assignments[0].name, "Assignment2")
}

func testEmptyStateIfNoAssignments() {
Expand All @@ -120,39 +137,19 @@ class AssignmentListViewModelTests: CoreTestCase {
}

func testGradingPeriodFilterChange() {
// we modify the first grading period's start_date to make sure it is the first in the picker
api.mock(GetGradingPeriods(courseID: "1"), value: [.make(id: "1", title: "GP1", start_date: Clock.now.addMonths(-1)), .make(id: "2", title: "GP2")])
api.mock(
GetGradingPeriods(courseID: "1"),
value: [
.make(id: "1", title: "Past GP", start_date: Clock.now.addMonths(-2), end_date: Clock.now.addMonths(-1)),
.make(id: "2", title: "Current GP", start_date: Clock.now.addMonths(-1), end_date: Clock.now.addMonths(1))
]
)
let gradingPeriods = AppEnvironment.shared.subscribe(GetGradingPeriods(courseID: "1"))
let testee = AssignmentListViewModel(context: .course("1"))
testee.selectedSortingOption = .groupName
testee.viewDidAppear()
XCTAssertEqual(testee.state, .empty)
// GetAssignmentsByGroup iterates through all grading periods so we have to mock it not to make the whole fetch fail
var assignmentGroupRequest = GetAssignmentGroupsRequest(
courseID: "1",
gradingPeriodID: "1",
perPage: 100
)
api.mock(assignmentGroupRequest, value: [])
XCTAssertEqual(testee.selectedGradingPeriod, gradingPeriods[1])

assignmentGroupRequest = GetAssignmentGroupsRequest(
courseID: "1",
gradingPeriodID: "2",
perPage: 100
)

api.mock(assignmentGroupRequest, value: [
.make(id: "AG1", name: "AGroup1", position: 1, assignments: [.make(assignment_group_id: "AG1", id: "1", name: "Assignment1")])
])

testee.filterOptionsDidUpdate(gradingPeriod: testee.gradingPeriods[1])

guard case .data(let groupViewModels) = testee.state else {
XCTFail("State doesn't contain any view models.")
return
}
XCTAssertEqual(groupViewModels.count, 1)
XCTAssertEqual(groupViewModels[0].name, "AGroup1")
XCTAssertEqual(groupViewModels[0].assignments.count, 1)
XCTAssertEqual(groupViewModels[0].assignments[0].name, "Assignment1")
testee.filterOptionsDidUpdate(gradingPeriod: gradingPeriods[0])
XCTAssertEqual(testee.selectedGradingPeriod, gradingPeriods[0])
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class CourseSyncSyllabusInteractorLiveTests: CoreTestCase {
api.mock(GetCalendarEvents(context: .course("testCourse"),
type: .event),
value: [.make(id: "2")])
api.mock(GetCalendarEvents(context: .course("testCourse")),
value: [.make(id: "1"), .make(id: "2")])
}

private func getHTMLParser() -> HTMLParser {
Expand Down
15 changes: 15 additions & 0 deletions Core/CoreTests/Grades/GetAssignmentsByGroupTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@ class GetAssignmentsByGroupTests: CoreTestCase {
return (groups2, nil, nil)
}

// Grading period nil assignment and group
let groups3: [APIAssignmentGroup] = [
.make(id: "3", name: "TestGroup3", position: 3, assignments: [])
]
let groupsRequest3 = GetAssignmentGroupsRequest(
courseID: "tc",
gradingPeriodID: nil,
perPage: 100
)
let group3Called = expectation(description: "group3Called")
api.mock(groupsRequest3) { _ in
group3Called.fulfill()
return (groups3, nil, nil)
}

let gradingPeriodsCalled = expectation(description: "gradingPeriodsCalled")
api.mock(GetGradingPeriodsRequest(courseID: "tc")) { _ in
gradingPeriodsCalled.fulfill()
Expand Down