[Horizon] Feature/clx 165 notebook list

[reabbotted]

• Showing and filtering a list of courses that have notes by text search
• Showing and filtering a list of notes for an individual course

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[inst-danger]

Horizon Build QR Code:

[inst-danger]

	Warnings
⚠️	One or more files are below the minimum test coverage 50%
⚠️	The total test coverage is below the minimum 90%

Coverage	New %	Master %	Delta
Canvas iOS	89.99%	90.05%	-0.07%
Core/Core/Courses/CourseProgression/CDCourseProgression.swift	0%	--	--
Core/Core/Planner/CalendarMain/ViewModel/PlannerViewModel.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/Model/CalendarWeek.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/View/Calendar/CalendarCardInteractionState.swift	0%	0%	0%
Core/Core/Courses/CourseProgression/GetCoursesProgressionRequest.swift	0%	--	--
Core/Core/Planner/CalendarMain/Model/PlannablesInteractor.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/View/Calendar/ViewPreferences.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/Model/CalendarDay.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/Model/CalendarMonth.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/View/Calendar/CalendarUtils.swift	0%	0%	0%

Generated by 🚫 dangerJS against f165134

[szabinst]

Congrats on your first Horizon PR!

Functionality wise, it works well! I had a some remark about our coding conventions, combine usage.

Some general observations:

• In Horizon (as opposed to Canvas) let's follow the View, Domain, Data folder structure for features.
• When declaring properties, we usually keep them "close" without new lines. We do separate dependencies, private properties and such by marking them with a comment but each section of properties are listed without new lines.
• Not set in stone but if a function signature is getting long (e.g 3 ore more parameters) we divide it by new lines.
• When adding view modifiers, reactive operators we add them on new lines.
• I added a few comments about new lines, but not everywhere.

[Ahmed-Naguib93]

congratulation for your first PR,
I set some comments

[Ahmed-Naguib93]

I'm sorry approved by mistake

[szabinst]

Thanks for the changes!

Last round of suggestions below.

[szabinst]

Missing indentation.

[szabinst]

Missing indentation.

[szabinst]

Instead of passing the whole router we could reduce the coupling by passing a closure e.g onBackButtonTap or didTap and then NotebookCourseViewModel and NotebookViewModel could handle the back logic.

[szabinst]

Suggested change

	let notebookNoteLabel = courseNote.labels.map({ label in
	NotebookNoteLabel.allCases.first { $0.rawValue.lowercased() == label.lowercased() }
	}).filter({$0 != nil}).map({$0!}).first
	let notebookNoteLabel = courseNote.labels
	.map { label in
	NotebookNoteLabel.allCases.first {
	$0.rawValue.lowercased() == label.lowercased()
	}
	}
	.filter {$0 != nil}
	.map {$0!}
	.first

[szabinst]

Please use the name subscriptions as that's the project wide standard for storing AnyCancellables.

[szabinst]

Suggested change

	.map({ notes in notes.map({ NotebookCourse(from: $0) }) })
	.map { notes in notes.map({ NotebookCourse(from: $0) }) }

[szabinst]

Suggested change

	return courses.filter({ course in
	term.isEmpty ||
	course.course.lowercased().contains(term.lowercased()) ||
	course.institution.lowercased().contains(term.lowercased())
	})
	return courses.filter { course in
	term.isEmpty ||
	course.course.lowercased().contains(term.lowercased()) ||
	course.institution.lowercased().contains(term.lowercased())
	}

[szabinst]

Please group the router and the interactor together as they are outside dependencies and use subscriptions instead of cancellables.

[szabinst]

Suggested change

	getCoursesInteractor.get().sink { _ in } receiveValue: { [weak self] courses in
	self?.listItems = courses.map {
	NotebookListItem(id: $0.id, course: $0.course, institution: $0.institution)
	}
	}.store(in: &cancellables)
	getCoursesInteractor.get()
	.flatMap {
	$0.publisher
	.map { course in
	NotebookListItem(
	id: course.id,
	course: course.course,
	institution: course.institution
	)
	}
	.collect()
	}
	.replaceError(with: [])
	.sink { [weak self] in
	self?.listItems = $0
	}
	.store(in: &cancellables)