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

Allow fetching of the priority of a validation error type #269

Merged
merged 4 commits into from
Jan 27, 2020

Conversation

evansiroky
Copy link
Contributor

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

This PR will expose the priority level of validation errors through the GraphQL schema. This is needed in order to implement ibi-group/datatools-ui#477.

@codecov-io
Copy link

codecov-io commented Jan 22, 2020

Codecov Report

Merging #269 into dev will increase coverage by 0.27%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #269      +/-   ##
============================================
+ Coverage     64.71%   64.99%   +0.27%     
- Complexity      864      866       +2     
============================================
  Files           134      134              
  Lines          6723     6725       +2     
  Branches        789      789              
============================================
+ Hits           4351     4371      +20     
+ Misses         2074     2055      -19     
- Partials        298      299       +1
Impacted Files Coverage Δ Complexity Δ
...veyal/gtfs/graphql/fetchers/ErrorCountFetcher.java 87.5% <100%> (+78.8%) 4 <0> (+2) ⬆️
...a/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java 98.27% <100%> (ø) 1 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d1ae24...7779097. Read the comment docs.

@@ -421,6 +422,7 @@
.field(MapFetcher.field("entity_id"))
.field(MapFetcher.field("entity_sequence", GraphQLInt))
.field(MapFetcher.field("bad_value"))
.field(ErrorPriorityFetcher.field("error_type_priority"))
Copy link
Contributor

@landonreed landonreed Jan 27, 2020

Choose a reason for hiding this comment

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

I think the intent with the errorCountType definition below is to contain high-level information about the error type, including the count, english message and priority fields. I think this error priority fetcher should be removed because it's redundant information. Open to hearing a counter argument though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally thought each error needed this info, but after implementing ibi-group/datatools-ui#527 I realized I didn't really need it. So yeah, I'll remove it.

@evansiroky evansiroky merged commit 40da0fb into dev Jan 27, 2020
@evansiroky evansiroky deleted the error-priority-type-graphql branch January 27, 2020 22:09
@evansiroky evansiroky mentioned this pull request Jan 27, 2020
7 tasks
@landonreed
Copy link
Contributor

🎉 This PR is included in version 5.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants