Skip to content

feat: Web Validator Client Display Errors#1383

Merged
KClough merged 10 commits intomasterfrom
feat/web-handle-errors
Apr 13, 2023
Merged

feat: Web Validator Client Display Errors#1383
KClough merged 10 commits intomasterfrom
feat/web-handle-errors

Conversation

@KClough
Copy link
Copy Markdown
Contributor

@KClough KClough commented Apr 7, 2023

Summary

Closes #1370

  • Refactor upload methods
  • Handle errors better

Expected behavior

  • Unified upload process should work at least as well as it did before for both URL- and file-based uploads
  • The following failures should now show feedback in the UI:

Failed to get a job ID from the server

Screenshot 2023-03-31 at 10 06 49 PM

Failed to PUT the file

Screenshot 2023-03-31 at 10 07 09 PM

Failed to check for a report (other than expected 404 while waiting)

Screenshot 2023-03-31 at 10 07 35 PM

Failed to fetch local RULES.md

Screenshot 2023-03-31 at 10 06 16 PM

Checklist

Notes

This branch is based on https://github.com/MobilityData/gtfs-validator/tree/master which used to be the feat/web branch in the Jarvus fork, hence all the extra commits.

ryon and others added 8 commits April 1, 2023 06:02
@ryon
chore: fix deprecated dependency
9b983b3
@ryon
chore: auto-format
b31fef4
@ryon
feat: handle missing RULES.md
288c543
@ryon
feat: refactor upload process and handle more errors
ca88baa
@bdferris-v2 @KClough
Add a more descriptive test failure message to NoticeFieldsTest. (#1369)
7a48fd8
@isabelle-dr @emmambd @KClough
Docs: Update notice migration file (#1357)
352e65b 
* refactor migration file

* update documentation with new file

* fix wording

* Make easier to read

* remove colors

---------

Co-authored-by: Emma Blue <[email protected]>
@bdferris-v2 @KClough
feat: Fail acceptance tests on significant change in warnings (#1377)
eacca32 
* Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing.  In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds.

* Additional unit tests for ValidationReportComparator.
@KClough
Merge branch 'master' into feat/web-handle-errors
956d5d8
@KClough KClough mentioned this pull request Apr 7, 2023
Closed
4 tasks
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2023

✅ Rule acceptance tests passed.
New Errors: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1425 sources (~0 %) are corrupted.
Commit: 33144a8
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2023

✅ Rule acceptance tests passed.
New Errors: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1425 sources (~0 %) are corrupted.
Commit: 7edc69c
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

davidgamez
davidgamez approved these changes Apr 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@davidgamez davidgamez left a comment

Choose a reason for hiding this comment

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

LGTM!

@isabelle-dr
Copy link
Copy Markdown
Contributor

isabelle-dr commented Apr 12, 2023

Looks like we can merge this one after this suggestion is solved! cc @KClough

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 13, 2023

✅ Rule acceptance tests passed.
New Errors: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1425 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1425 sources (~0 %) are corrupted.
Commit: 10c0e83
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

@KClough KClough merged commit c4cf31f into master Apr 13, 2023
@KClough KClough deleted the feat/web-handle-errors branch April 13, 2023 18:20
bradyhunsaker pushed a commit to bradyhunsaker/gtfs-validator that referenced this pull request Apr 25, 2023
@KClough @ryon
@bdferris-v2 @isabelle-dr @emmambd @davidgamez @bradyhunsaker
feat: Web Validator Client Display Errors (MobilityData#1383)
b00e928 
* chore: fix deprecated dependency

* chore: auto-format

* feat: handle missing RULES.md

* feat: refactor upload process and handle more errors

* Add a more descriptive test failure message to NoticeFieldsTest. (MobilityData#1369)

* Docs: Update notice migration file (MobilityData#1357)

* refactor migration file

* update documentation with new file

* fix wording

* Make easier to read

* remove colors

---------

Co-authored-by: Emma Blue <[email protected]>

* feat: Fail acceptance tests on significant change in warnings (MobilityData#1377)

* Refactor logic out of of outputcomparator/cli/Main and into a new ValidationReportComparator class for easier testing.  In addition, update ValidationReportComparator to trigger a comparison error if changed warnings are above configured thresholds.

* Additional unit tests for ValidationReportComparator.

---------

Co-authored-by: Ryon Coleman <[email protected]>
Co-authored-by: Brian Ferris <[email protected]>
Co-authored-by: isabelle-dr <[email protected]>
Co-authored-by: Emma Blue <[email protected]>
Co-authored-by: David Gamez <[email protected]>
@themightychris themightychris mentioned this pull request Apr 27, 2023
Closed
5 tasks
@mijogu mijogu mentioned this pull request Apr 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidgamez davidgamez davidgamez approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Web Validator: Update web client to show message to the user if there is an issue communicating with the service

5 participants

@KClough @isabelle-dr @davidgamez @ryon @bdferris-v2