Fix table quiz presented like theory (#607)

ivan-magda · web-flow · commit ad84dd2115dd · 2019-12-18T21:22:34.000+03:00
* Make BlockType optional

* Update can edit step text determination

* Remove fill blanks from supported quizzes list

* Hide step statistics for unsopported quizzes

* Extend BlockType

* Test all block types

* Fix analytics

* Review linter violations
diff --git a/Stepic/Analytics/Events/AnalyticsEvents.swift b/Stepic/Analytics/Events/AnalyticsEvents.swift
@@ -135,14 +135,6 @@ struct AnalyticsEvents {
         struct Edit {
             static let opened = "step_edit_opened"
             static let completed = "step_edit_completed"
-
-            static func makeParams(stepID: Int, type: String, position: Int) -> [String: Any] {
-                return [
-                    "step": stepID,
-                    "type": type,
-                    "number": position
-                ]
-            }
         }
     }
 
diff --git a/Stepic/Block+CoreDataProperties.swift b/Stepic/Block+CoreDataProperties.swift
@@ -68,8 +68,8 @@ extension Block {
         }
     }
 
-    var type: BlockType {
-        BlockType(rawValue: self.name) ?? .text
+    var type: BlockType? {
+        BlockType(rawValue: self.name)
     }
 
     // MARK: - Types -
@@ -80,9 +80,6 @@ extension Block {
         case choice
         case code
         case dataset
-        case fillBlanks = "fill-blanks"
-        case freeAnswer = "free-answer"
-        case linuxCode = "linux-code"
         case matching
         case math
         case number
@@ -94,13 +91,17 @@ extension Block {
         case text
         case video
         case admin
+        case table
+        case html
+        case schulte
+        case fillBlanks = "fill-blanks"
+        case freeAnswer = "free-answer"
+        case linuxCode = "linux-code"
+        case randomTasks = "random-tasks"
+        case manualScore = "manual-score"
 
         var isTheory: Bool {
             return [BlockType.text, BlockType.video].contains(self)
         }
-
-        var isQuiz: Bool {
-            return !self.isTheory
-        }
     }
 }
diff --git a/Stepic/Sources/Controllers/StyledNavigationController.swift b/Stepic/Sources/Controllers/StyledNavigationController.swift
@@ -331,6 +331,7 @@ class StyledNavigationController: UINavigationController {
         self.navigationBarAppearanceForController.removeValue(forKey: viewController.hashValue)
     }
 
+    // swiftlint:disable:next cyclomatic_complexity
     private func animateShadowView(transitionCoordinator: UIViewControllerTransitionCoordinator) {
         guard let fromViewController = self.transitionCoordinator?.viewController(forKey: .from),
               let toViewController = self.transitionCoordinator?.viewController(forKey: .to) else {
diff --git a/Stepic/Sources/Modules/CourseInfoSubmodules/CourseInfoTabSyllabus/Provider/SyllabusDownloadsService.swift b/Stepic/Sources/Modules/CourseInfoSubmodules/CourseInfoTabSyllabus/Provider/SyllabusDownloadsService.swift
@@ -421,6 +421,7 @@ final class SyllabusDownloadsService: SyllabusDownloadsServiceProtocol {
         return .cached(bytesTotal: unitSizeInBytes)
     }
 
+    // swiftlint:disable:next cyclomatic_complexity
     func getDownloadingStateForSection(_ section: Section) -> CourseInfoTabSyllabus.DownloadState {
         let units = section.units
 
diff --git a/Stepic/Sources/Modules/Discussions/DiscussionsInteractor.swift b/Stepic/Sources/Modules/Discussions/DiscussionsInteractor.swift
@@ -474,6 +474,7 @@ final class DiscussionsInteractor: DiscussionsInteractorProtocol {
         return max(leftToLoad, 0)
     }
 
+    // TODO: Fix cyclomatic_complexity
     private func getLoadedDiscussionsWindow() -> (startIndex: Int, endIndex: Int) {
         let loadedDiscussionsIDs = Set(self.currentDiscussions.map({ $0.id }))
 
diff --git a/Stepic/Sources/Modules/Discussions/Views/DiscussionsTableViewDataSource.swift b/Stepic/Sources/Modules/Discussions/Views/DiscussionsTableViewDataSource.swift
@@ -128,6 +128,7 @@ extension DiscussionsTableViewDataSource: UITableViewDataSource {
         cell.isUpdating = viewModel.isFetchingMoreReplies
     }
 
+    // TODO: Refactor
     private func configureDiscussionCell(
         _ cell: DiscussionsTableViewCell,
         at indexPath: IndexPath,
diff --git a/Stepic/Sources/Modules/EditStep/EditStepInteractor.swift b/Stepic/Sources/Modules/EditStep/EditStepInteractor.swift
@@ -109,22 +109,24 @@ final class EditStepInteractor: EditStepInteractorProtocol {
                 return
             }
 
-            let params = AnalyticsEvents.Step.Edit.makeParams(
-                stepID: step.id, type: step.block.type.rawValue, position: step.position
-            )
+            let params: [String: Any] = [
+                "step": step.id,
+                "type": step.block.name,
+                "number": step.position
+            ]
 
             switch event {
             case .opened:
                 AmplitudeAnalyticsEvents.Steps.stepEditOpened(
                     stepID: step.id,
-                    type: step.block.type.rawValue,
+                    type: step.block.name,
                     position: step.position
                 ).send()
                 AnalyticsReporter.reportEvent(AnalyticsEvents.Step.Edit.opened, parameters: params)
             case .completed:
                 AmplitudeAnalyticsEvents.Steps.stepEditCompleted(
                     stepID: step.id,
-                    type: step.block.type.rawValue,
+                    type: step.block.name,
                     position: step.position
                 ).send()
                 AnalyticsReporter.reportEvent(AnalyticsEvents.Step.Edit.completed, parameters: params)
diff --git a/Stepic/Sources/Modules/NewLesson/NewLessonPresenter.swift b/Stepic/Sources/Modules/NewLesson/NewLessonPresenter.swift
@@ -112,9 +112,9 @@ final class NewLessonPresenter: NewLessonPresenterProtocol {
             }()
             return .init(
                 id: step.id,
-                type: step.block.type,
                 iconImage: iconImage ?? UIImage(),
-                isPassed: progresses[safe: index]?.isPassed ?? false
+                isPassed: progresses[safe: index]?.isPassed ?? false,
+                canEdit: canEdit && step.block.type != .video
             )
         }
         return NewLessonViewModel(
@@ -123,8 +123,7 @@ final class NewLessonPresenter: NewLessonPresenterProtocol {
             stepLinkMaker: {
                 "\(StepicApplicationsInfo.stepicURL)/lesson/\(lesson.id)/step/\($0)?from_mobile_app=true"
             },
-            startStepIndex: startStepIndex,
-            canEdit: canEdit
+            startStepIndex: startStepIndex
         )
     }
 
diff --git a/Stepic/Sources/Modules/NewLesson/NewLessonViewController.swift b/Stepic/Sources/Modules/NewLesson/NewLessonViewController.swift
@@ -322,9 +322,7 @@ final class NewLessonViewController: TabmanViewController, ControllerWithStepikP
             return
         }
 
-        let showEditStep = data.canEdit && step.type != .video
-
-        self.navigationItem.rightBarButtonItems = showEditStep
+        self.navigationItem.rightBarButtonItems = step.canEdit
             ? self.teacherRightBarButtonItems
             : self.studentRightBarButtonItems
     }
diff --git a/Stepic/Sources/Modules/NewLesson/NewLessonViewModel.swift b/Stepic/Sources/Modules/NewLesson/NewLessonViewModel.swift
@@ -3,14 +3,13 @@ import Foundation
 struct NewLessonViewModel {
     struct StepDescription {
         let id: Step.IdType
-        let type: Block.BlockType
         let iconImage: UIImage
         let isPassed: Bool
+        let canEdit: Bool
     }
 
     let lessonTitle: String
     let steps: [StepDescription]
     let stepLinkMaker: (String) -> String
     let startStepIndex: Int
-    let canEdit: Bool
 }
diff --git a/Stepic/Sources/Modules/NewStep/NewStepDataFlow.swift b/Stepic/Sources/Modules/NewStep/NewStepDataFlow.swift
@@ -125,12 +125,10 @@ enum NewStep {
         case math
         case sorting
         case matching
-        case fillBlanks
         case code
         case sql
         case unknown(blockName: String)
 
-        // swiftlint:disable:next cyclomatic_complexity
         init(blockName: String) {
             switch blockName {
             case "choice":
@@ -147,8 +145,6 @@ enum NewStep {
                 self = .sorting
             case "matching":
                 self = .matching
-            case "fill-blanks":
-                self = .fillBlanks
             case "code":
                 self = .code
             case "sql":
@@ -174,8 +170,6 @@ enum NewStep {
                 return "sorting"
             case .matching:
                 return "matching"
-            case .fillBlanks:
-                return "fill-blanks"
             case .code:
                 return "code"
             case .sql:
@@ -188,7 +182,7 @@ enum NewStep {
         static func == (lhs: QuizType, rhs: QuizType) -> Bool {
             switch (lhs, rhs) {
             case (.choice, .choice), (.string, .string), (.number, .number), (.math, .math), (.freeAnswer, .freeAnswer),
-                 (.sorting, .sorting), (.matching, .matching), (.fillBlanks, .fillBlanks), (.code, .code), (.sql, .sql):
+                 (.sorting, .sorting), (.matching, .matching), (.code, .code), (.sql, .sql):
                 return true
             case (.unknown(let lhsName), .unknown(let rhsName)):
                 return lhsName == rhsName
diff --git a/Stepic/Sources/Modules/NewStep/NewStepPresenter.swift b/Stepic/Sources/Modules/NewStep/NewStepPresenter.swift
@@ -107,6 +107,16 @@ final class NewStepPresenter: NewStepPresenterProtocol {
                 quizType = NewStep.QuizType(blockName: step.block.name)
             }
 
+            let shouldShowStepStatistics: Bool = {
+                if quizType == nil {
+                    return false
+                }
+                if case .unknown = quizType {
+                    return false
+                }
+                return true
+            }()
+
             let discussionsLabelTitle = self.makeDiscussionsLabelTitle(step: step)
             let urlPath = "\(StepicApplicationsInfo.stepicURL)/lesson/\(step.lessonID)/step/\(step.position)?from_mobile_app=true"
 
@@ -118,8 +128,8 @@ final class NewStepPresenter: NewStepPresenterProtocol {
                 discussionProxyID: step.discussionProxyID,
                 stepURLPath: urlPath,
                 lessonID: step.lessonID,
-                passedByCount: step.block.type.isQuiz ? step.passedByCount : nil,
-                correctRatio: step.block.type.isQuiz ? step.correctRatio : nil,
+                passedByCount: shouldShowStepStatistics ? step.passedByCount : nil,
+                correctRatio: shouldShowStepStatistics ? step.correctRatio : nil,
                 step: step
             )
 
diff --git a/StepicTests/NewStepTests/NewStepViewControllerMock.swift b/StepicTests/NewStepTests/NewStepViewControllerMock.swift
@@ -148,7 +148,6 @@ class NewStepViewControllerSpec: QuickSpec {
             let blockNameWithQuizTypePairs: [(String, NewStep.QuizType)] = [
                 ("choice", .choice),
                 ("code", .code),
-                ("fill-blanks", .fillBlanks),
                 ("free-answer", .freeAnswer),
                 ("matching", .matching),
                 ("math", .math),
@@ -162,7 +161,13 @@ class NewStepViewControllerSpec: QuickSpec {
                 ("linux-code", .unknown(blockName: "linux-code")),
                 ("puzzle", .unknown(blockName: "puzzle")),
                 ("pycharm", .unknown(blockName: "pycharm")),
-                ("admin", .unknown(blockName: "admin"))
+                ("admin", .unknown(blockName: "admin")),
+                ("table", .unknown(blockName: "table")),
+                ("html", .unknown(blockName: "html")),
+                ("fill-blanks", .unknown(blockName: "fill-blanks")),
+                ("random-tasks", .unknown(blockName: "random-tasks")),
+                ("schulte", .unknown(blockName: "schulte")),
+                ("manual-score", .unknown(blockName: "manual-score"))
             ]
             let steps = blockNameWithQuizTypePairs.map { pair -> Step in
                 let json = JSON(

Original file line number	Diff line number	Diff line change
`@@ -135,14 +135,6 @@ struct AnalyticsEvents {`
`135`	`135`	`struct Edit {`
`136`	`136`	`static let opened = "step_edit_opened"`
`137`	`137`	`static let completed = "step_edit_completed"`
`138`		`-`
`139`		`- static func makeParams(stepID: Int, type: String, position: Int) -> [String: Any] {`
`140`		`- return [`
`141`		`- "step": stepID,`
`142`		`- "type": type,`
`143`		`- "number": position`
`144`		`- ]`
`145`		`- }`
`146`	`138`	`}`
`147`	`139`	`}`
`148`	`140`
Original file line number	Diff line number	Diff line change
`@@ -331,6 +331,7 @@ class StyledNavigationController: UINavigationController {`
`331`	`331`	`self.navigationBarAppearanceForController.removeValue(forKey: viewController.hashValue)`
`332`	`332`	`}`
`333`	`333`
	`334`	`+ // swiftlint:disable:next cyclomatic_complexity`
`334`	`335`	`private func animateShadowView(transitionCoordinator: UIViewControllerTransitionCoordinator) {`
`335`	`336`	`guard let fromViewController = self.transitionCoordinator?.viewController(forKey: .from),`
`336`	`337`	`let toViewController = self.transitionCoordinator?.viewController(forKey: .to) else {`
Original file line number	Diff line number	Diff line change
`@@ -421,6 +421,7 @@ final class SyllabusDownloadsService: SyllabusDownloadsServiceProtocol {`
`421`	`421`	`return .cached(bytesTotal: unitSizeInBytes)`
`422`	`422`	`}`
`423`	`423`
	`424`	`+ // swiftlint:disable:next cyclomatic_complexity`
`424`	`425`	`func getDownloadingStateForSection(_ section: Section) -> CourseInfoTabSyllabus.DownloadState {`
`425`	`426`	`let units = section.units`
`426`	`427`
Original file line number	Diff line number	Diff line change
`@@ -474,6 +474,7 @@ final class DiscussionsInteractor: DiscussionsInteractorProtocol {`
`474`	`474`	`return max(leftToLoad, 0)`
`475`	`475`	`}`
`476`	`476`
	`477`	`+ // TODO: Fix cyclomatic_complexity`
`477`	`478`	`private func getLoadedDiscussionsWindow() -> (startIndex: Int, endIndex: Int) {`
`478`	`479`	`let loadedDiscussionsIDs = Set(self.currentDiscussions.map({ $0.id }))`
`479`	`480`
Original file line number	Diff line number	Diff line change
`@@ -128,6 +128,7 @@ extension DiscussionsTableViewDataSource: UITableViewDataSource {`
`128`	`128`	`cell.isUpdating = viewModel.isFetchingMoreReplies`
`129`	`129`	`}`
`130`	`130`
	`131`	`+ // TODO: Refactor`
`131`	`132`	`private func configureDiscussionCell(`
`132`	`133`	`_ cell: DiscussionsTableViewCell,`
`133`	`134`	`at indexPath: IndexPath,`
Original file line number	Diff line number	Diff line change
`@@ -112,9 +112,9 @@ final class NewLessonPresenter: NewLessonPresenterProtocol {`
`112`	`112`	`}()`
`113`	`113`	`return .init(`
`114`	`114`	`id: step.id,`
`115`		`- type: step.block.type,`
`116`	`115`	`iconImage: iconImage ?? UIImage(),`
`117`		`- isPassed: progresses[safe: index]?.isPassed ?? false`
	`116`	`+ isPassed: progresses[safe: index]?.isPassed ?? false,`
	`117`	`+ canEdit: canEdit && step.block.type != .video`
`118`	`118`	`)`
`119`	`119`	`}`
`120`	`120`	`return NewLessonViewModel(`
`@@ -123,8 +123,7 @@ final class NewLessonPresenter: NewLessonPresenterProtocol {`
`123`	`123`	`stepLinkMaker: {`
`124`	`124`	`"\(StepicApplicationsInfo.stepicURL)/lesson/\(lesson.id)/step/\($0)?from_mobile_app=true"`
`125`	`125`	`},`
`126`		`- startStepIndex: startStepIndex,`
`127`		`- canEdit: canEdit`
	`126`	`+ startStepIndex: startStepIndex`
`128`	`127`	`)`
`129`	`128`	`}`
`130`	`129`
Original file line number	Diff line number	Diff line change
`@@ -322,9 +322,7 @@ final class NewLessonViewController: TabmanViewController, ControllerWithStepikP`
`322`	`322`	`return`
`323`	`323`	`}`
`324`	`324`
`325`		`- let showEditStep = data.canEdit && step.type != .video`
`326`		`-`
`327`		`- self.navigationItem.rightBarButtonItems = showEditStep`
	`325`	`+ self.navigationItem.rightBarButtonItems = step.canEdit`
`328`	`326`	`? self.teacherRightBarButtonItems`
`329`	`327`	`: self.studentRightBarButtonItems`
`330`	`328`	`}`