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

[INSPECT-313] FEATURE** Further enhance inspection validation #318

Merged
merged 8 commits into from
Dec 10, 2024
143 changes: 85 additions & 58 deletions README.md
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Important

The linter went wild in this file. So there is a lot more diffs than anticipated.

There is information added about working with XCode 15, and deleting all Realm objects added

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions ipad.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@
path = Shift;
sourceTree = "<group>";
};
2944BCD62433929700826F48 /* Waterfract Inspection */ = {
2944BCD62433929700826F48 /* Watercraft Inspection */ = {
isa = PBXGroup;
children = (
294D9A872379C62F00BD4928 /* WatercraftInspectionModel.swift */,
Expand All @@ -611,7 +611,7 @@
298BF7CD2395D57E0072AA28 /* DestinationWaterBodyModel.swift */,
2944BCD8243392F600826F48 /* Form Fields */,
);
path = "Waterfract Inspection";
path = "Watercraft Inspection";
sourceTree = "<group>";
};
2944BCD8243392F600826F48 /* Form Fields */ = {
Expand Down
5 changes: 5 additions & 0 deletions ipad/Models/Shift/ShiftModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum SyncableItemStatus {
case Draft
case PendingSync
case Completed
case Errors
}

/// Represents a shift model for tracking inspection details and shift information.
Expand Down Expand Up @@ -184,6 +185,8 @@ class ShiftModel: Object, BaseRealmObject {
newStatus = "Pending Sync"
case .Completed:
newStatus = "Completed"
case .Errors:
newStatus = "Contains Errors"
}
do {
let realm = try Realm()
Expand Down Expand Up @@ -217,6 +220,8 @@ class ShiftModel: Object, BaseRealmObject {
return .PendingSync
case "completed":
return .Completed
case "contains errors":
return .Errors
default:
return .Draft
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Realm
import RealmSwift

class WatercraftInspectionModel: Object, BaseRealmObject {
@objc dynamic var formDidValidate: Bool = false
@objc dynamic var userId: String = ""
@objc dynamic var localId: String = {
return UUID().uuidString
Expand Down Expand Up @@ -190,29 +191,34 @@ class WatercraftInspectionModel: Object, BaseRealmObject {
}

func set(shouldSync should: Bool) {
set(status: should ? .PendingSync : .Draft )
do {
let realm = try Realm()
try realm.write {
self.shouldSync = should
self.status = should ? "Pending Sync" : "Draft"
if (formDidValidate) {
do {
let realm = try Realm()
try realm.write {
self.shouldSync = should
self.status = should ? "Pending Sync" : "Draft"
}
} catch let error as NSError {
print("** REALM ERROR")
print(error)
}
} catch let error as NSError {
print("** REALM ERROR")
print(error)
} else {
// If form isn't validated, force status to Errors
set(status: .Errors)
}
set(status: should ? .PendingSync : .Draft )
}

func set(status statusEnum: SyncableItemStatus) {
var newStatus = "\(statusEnum)"
switch statusEnum {
case .Draft:
newStatus = "Draft"
newStatus = formDidValidate ? "Draft" : "Not Validated"
case .PendingSync:
newStatus = "Pending Sync"
newStatus = formDidValidate ? "Pending Sync" : "Not Validated"
case .Completed:
newStatus = "Completed"
case .Errors:
newStatus = "Not Validated"
}
do {
let realm = try Realm()
Expand Down Expand Up @@ -270,6 +276,8 @@ class WatercraftInspectionModel: Object, BaseRealmObject {
return .PendingSync
case "completed":
return .Completed
case "not validated":
return .Errors
default:
return .Draft
}
Expand Down
12 changes: 12 additions & 0 deletions ipad/Services/ShiftService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ class ShiftService {
/// - then: A closure that gets called once the shift has been submitted. It receives a boolean indicating whether the operation was successful.
public func submit(shift: ShiftModel, then: @escaping (_ success: Bool) -> Void) {
let shiftLocalId = shift.localId
if(!shift.inspections.allSatisfy(){item in return item.formDidValidate}) {
shift.set(shouldSync: false)
shift.set(status: .Errors)
shift.inspections.forEach(){inspection in
if (!inspection.formDidValidate){
inspection.set(shouldSync: false)
inspection.set(status: .Errors)
}
}
Alert.show(title: "Submission Error", message: "A shift contains non-validated inspections, please re-visit inspections and correct outstanding issues")
return then(false);
}
// Post call
post(shift: shift) { (shiftId) in
guard let remoteId = shiftId, let refetchedShift = Storage.shared.shift(withLocalId: shiftLocalId) else {
Expand Down
2 changes: 2 additions & 0 deletions ipad/Utilities/Status Color/StatusColor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class StatusColor {
return Colors.Status.Yellow
case "completed":
return Colors.Status.Green
case "contains errors", "not validated":
return Colors.Status.Red
default:
return Colors.Status.LightGray
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ShiftBlowBysHeaderCollectionViewCell: BaseShiftOverviewCollectionViewCell

override func autofill() {
guard let model = self.model else {return}
if model.getStatus() != .Draft {
if [.Draft, .Errors].contains(model.getStatus()) {
addBlowByButton.alpha = 0
addBlowByButton.isEnabled = false
}
Expand Down Expand Up @@ -49,6 +49,8 @@ class ShiftBlowBysHeaderCollectionViewCell: BaseShiftOverviewCollectionViewCell
return Colors.Status.Green
case .Draft:
return Colors.Status.DarkGray
case .Errors:
return Colors.Status.Red
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ShifOverviewHeaderCollectionViewCell: BaseShiftOverviewCollectionViewCell
self.locationLabel.text = model.station
self.statusLabel.text = model.status
self.statusIndicator.backgroundColor = StatusColor.color(for: model.status)
if model.getStatus() != .Draft {
if ![.Draft, .Errors].contains(model.getStatus()) {
addInspectionButton.alpha = 0
addInspectionButton.isEnabled = false
}
Expand Down Expand Up @@ -65,6 +65,8 @@ class ShifOverviewHeaderCollectionViewCell: BaseShiftOverviewCollectionViewCell
return Colors.Status.Green
case .Draft:
return Colors.Status.DarkGray
case .Errors:
return Colors.Status.Red
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class BlowbyTableCollectionViewCell: BaseShiftOverviewCollectionViewCell {
columns.append(TableViewColumnConfig(key: "", header: "Edit", type: .Button, buttonName: "Edit", showHeader: false))

// Disable adding blowbys if not completed and hide delete button
if model.getStatus() != .Draft {
if ![.Draft, .Errors].contains(model.getStatus()) {
columns.removeLast()
blowByButton.alpha = 0
blowByButton.isEnabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class InspectionsTableCollectionViewCell: BaseShiftOverviewCollectionViewCell {
tableHeightConstraint.constant = InspectionsTableCollectionViewCell.getTableHeight(for: model)

var buttonName = "View"
if model.getStatus() == .Draft {
if [.Draft, .Errors].contains(model.getStatus()) {
buttonName = "Edit"
}

Expand Down
53 changes: 41 additions & 12 deletions ipad/ViewControllers/Shift/ShiftViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,32 @@ class ShiftViewController: BaseViewController {
}

override func viewWillDisappear(_ animated: Bool) {
print(self)
NotificationCenter.default.removeObserver(self)
}

override func viewWillAppear(_ animated: Bool) {
self.updateStatuses();
setupCollectionView()
self.collectionView.reloadData()
addListeners()
}

/// Iterates through inspections checking formDidValidate. If any form does validate, modifies all appropraite statuses to `.Errors`
Copy link
Contributor

Choose a reason for hiding this comment

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

optimized updateStatuses so that if model is null, we don't have a crash (use of guard as found elsewhere in the file)

private func updateStatuses() {
guard let model = self.model else { return }
if(model.status != "Completed"){
if (model.inspections.allSatisfy(){$0.formDidValidate}){
model.set(status: .Draft)
model.inspections.forEach { inspection in
inspection.set(status: .Draft)
}
} else {
model.set(status: .Errors)
model.inspections.forEach { inspection in
inspection.set(status: inspection.formDidValidate ? .Draft : .Errors)
}
}
}
}
private func addListeners() {
NotificationCenter.default.removeObserver(self, name: .TableButtonClicked, object: nil)
NotificationCenter.default.removeObserver(self, name: .InputItemValueChanged, object: nil)
Expand All @@ -96,7 +112,7 @@ class ShiftViewController: BaseViewController {
blowbyModal.initialize(shift: currentShiftModel, delegate: self, onStart: { [weak self] (model) in
guard self != nil else { return }
}) {
// Canceled
// Cancelled
}
}

Expand All @@ -107,7 +123,7 @@ class ShiftViewController: BaseViewController {

func setup(model: ShiftModel) {
self.model = model
self.isEditable = model.getStatus() == .Draft || model.getStatus() == .PendingSync
self.isEditable = [.Draft, .PendingSync, .Errors].contains(model.getStatus())
if model.getStatus() == .PendingSync {
model.set(shouldSync: false)
for inspection in model.inspections {
Expand All @@ -116,8 +132,8 @@ class ShiftViewController: BaseViewController {
Alert.show(title: "Changed to draft", message: "Status changed to draft. tap submit when you've made your changes.")
}

if model.getStatus() == .Draft {
// make sure inspections are editable.
if [.Draft, .Errors].contains(model.getStatus()) {
// make sure inspections are editable.
for inspection in model.inspections {
inspection.set(shouldSync: false)
}
Expand Down Expand Up @@ -177,12 +193,14 @@ class ShiftViewController: BaseViewController {
@objc func completeAction(sender: UIBarButtonItem) {
guard let model = self.model else { return }
self.dismissKeyboard()

self.updateStatuses()
Copy link
Contributor

Choose a reason for hiding this comment

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

Added updateStatus before checking canSubmit, so we have the most up to date info on the validation of inspections

// if can submit
var alertMessage = "This shift and the inspections will be uploaded when possible"
if model.shiftStartDate < Calendar.current.startOfDay(for: Date()) {
alertMessage += "\n\n You've entered a date that occurred before today. If this was intentional, no problem! Otherwise, please double-check the entered date: \n\(model.shiftStartDate.stringShort())"
}
if canSubmit() {
if canSubmit() && model.inspections.allSatisfy({ $0.formDidValidate }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This addition of the if statement prevents the user to submit an error containing inspection (avoiding our open loop validation problem)

Alert.show(title: "Are you sure?", message: alertMessage, yes: {[weak self] in
guard let strongSelf = self else { return }
model.set(shouldSync: true)
Expand Down Expand Up @@ -253,7 +271,7 @@ class ShiftViewController: BaseViewController {
navigation.navigationBar.tintColor = .white
navigation.navigationBar.titleTextAttributes = [.foregroundColor: UIColor.white]
setGradiantBackground(navigationBar: navigation.navigationBar)
if let model = self.model, model.getStatus() == .Draft {
if let model = self.model, [.Draft, .Errors].contains(model.getStatus()) {
setRightNavButtons()
}
}
Expand All @@ -268,19 +286,19 @@ class ShiftViewController: BaseViewController {

// MARK: Validation
func canSubmit() -> Bool {
return validationMessage() == ""
return validationMessage().isEmpty
}

func validationMessage() -> String {
var message: String = ""
guard let model = self.model else { return message }
var counter = 1
if model.startTime == "" {
if model.startTime.isEmpty {
message = "\(message)\n\(counter)- Missing Shift Start time."
counter += 1
}

if model.endTime == "" {
if model.endTime.isEmpty {
message = "\(message)\n\(counter)- Missing Shift End time."
counter += 1
}
Expand All @@ -306,7 +324,7 @@ class ShiftViewController: BaseViewController {
}

for inspection in model.inspections {
if inspection.inspectionTime == "" {
if inspection.inspectionTime.isEmpty {
message = "\(message)\n\(counter)- Missing Time of Inspection."
counter += 1
}
Expand Down Expand Up @@ -343,6 +361,17 @@ class ShiftViewController: BaseViewController {
}
}
}

// Check for invalid inspections
Copy link
Contributor

Choose a reason for hiding this comment

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

checks our invalid inspections and let's the user know that the inspections contain validation errors. It is possible to let the users know which validation in particular here too, but that is handled in the inspections page specifically (may be something to review with SO)

let invalidInspections = model.inspections.filter { !$0.formDidValidate }
if !invalidInspections.isEmpty {
message = "\(message)\n\(counter)- One or more inspections contain validation errors. Please review each inspection."
counter += 1
}

if !message.isEmpty {
model.set(status: .Errors)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This if !message.isEmpty block may be redundant and removable with our other checks in place, however I will have to confirm with some testing


return message
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@ class WatercraftInspectionViewController: BaseViewController {

// MARK: Setup
func setup(model: WatercraftInspectionModel) {
model.set(value: false, for: "formDidValidate")
self.model = model
self.isEditable = model.getStatus() == .Draft
self.isEditable = [.Draft, .Errors].contains(model.getStatus())
self.styleNavBar()
if !model.isPassportHolder || model.launchedOutsideBC || model.isNewPassportIssued {
self.showFullInspection = true
Expand Down Expand Up @@ -169,7 +170,7 @@ class WatercraftInspectionViewController: BaseViewController {
navigation.navigationBar.tintColor = .white
navigation.navigationBar.titleTextAttributes = [.foregroundColor: UIColor.white]
setGradiantBackground(navigationBar: navigation.navigationBar)
if let model = self.model, model.getStatus() == .Draft {
if let model = self.model, [.Draft, .Errors].contains(model.getStatus()) {
setRightNavButtons()
setLeftNavButtons()
}
Expand Down Expand Up @@ -243,7 +244,7 @@ class WatercraftInspectionViewController: BaseViewController {
}

func canSubmit() -> Bool {
return validationMessage() == ""
return validationMessage().isEmpty
}

/// Enum error cases grouped by sections for clarity
Expand Down Expand Up @@ -698,7 +699,11 @@ class WatercraftInspectionViewController: BaseViewController {
message += "\n"
}
}

model.set(value: message.isEmpty, for: "formDidValidate")
if (message.isEmpty) {
model.set(status: .Draft)
}

return message
}

Expand Down
2 changes: 1 addition & 1 deletion ipad/Views/Form/Input Cells/InputItem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ class TimeInput: InputItem {

func getValue() -> Time? {
let stringValue: String = self.value.get(type: self.type) as? String ?? ""
if stringValue == "" {return nil}
if stringValue.isEmpty {return nil}
return Time(string: stringValue)
}

Expand Down
2 changes: 1 addition & 1 deletion ipad/Views/Input Modal/InputModal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class InputModal: ModalView, Theme {
}

func validateInput(text: String) -> Bool {
if text.removeWhitespaces() == "" {
if text.removeWhitespaces().isEmpty {
invalidInput(message: "Please enter a value")
return false
} else {
Expand Down
Loading
Loading