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

Conversation

LocalNewsTV
Copy link
Collaborator

@LocalNewsTV LocalNewsTV commented Jun 13, 2024

Warning

This work was not approved for the on-season (as of 06/14/24) and should not be merged in without the SO verifying they want it. It should remain saved until the off-season and made part of the normal bundle of work.

Caution

This work was off of the existing production branch that had its targets pointing to dev. This may require the branch being updated to point point at .dev again if this has been changed to .prod

Pull Request Standards

  • The title of the PR is accurate
  • The title includes the type of change [HOTFIX, FEATURE, etc]
  • The PR title includes the ticket number in format of [INSPECT-###]
  • Documentation is updated to reflect change

Description

This PR includes the following proposed change(s):

  • Add new boolean flag to Inspections formDidValidate

    • boolean only triggers when user exits inspection, and validation passed by conventional method
    • if user bypasses validation, the boolean remains false :)
  • When user goes to submit a shift, the shift checks that all inspections have formDidValidate == true

    • If this fails, it marks all failing inspections with new status enum .Errors
      • .Errors enum displays red on screen
      • User is alerted their inspection fails due to unvalidated inspections
    • Submission is cancelled, no data is sent, no duplicate entries made
  • Converted several string == "" checks to the swift built in flag string.isEmpty

  • Added check of inspection validation errors on reboot when user selects submit

  • Renamed Watercraft Inspection folder

  • Current error message when a user reboots the app with the validation errors and attempts to submit the invalidated shift:

Screenshot 2024-12-09 at 10 54 04 AM

  • Screen that appears when looking at an error containing shift on reboot:

Screenshot 2024-12-09 at 10 55 05 AM

README.md Outdated
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

@LocalNewsTV LocalNewsTV linked an issue Jun 18, 2024 that may be closed by this pull request
5 tasks
@PaulGarewal PaulGarewal force-pushed the 313-further-enforce-validation branch from 6e8fdce to d520884 Compare December 6, 2024 18:17
@PaulGarewal PaulGarewal self-assigned this Dec 6, 2024
- Add formDidValidate to prevent submission of unvalidated inspection at restart
- renamed water craft inspection folder
- optimized updateStatuses() function
- removed print statement in viewWillDisappear function
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)

@@ -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)


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

@@ -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)

@PaulGarewal PaulGarewal marked this pull request as ready for review December 9, 2024 19:40
@PaulGarewal PaulGarewal requested a review from a team December 9, 2024 19:40
@PaulGarewal PaulGarewal merged commit 94bf150 into master Dec 10, 2024
3 checks passed
@PaulGarewal PaulGarewal deleted the 313-further-enforce-validation branch December 10, 2024 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tighten Validation Loophole in Inspections
3 participants