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

Add MobilityData Validator #916

Merged
merged 30 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e145922
add initial framework for displaying mobility data
miles-grant-ibigroup Feb 21, 2023
a46ca8f
refactor(MobilityDataValidationResult): show rules from rules.json
miles-grant-ibigroup Feb 24, 2023
cf5fdad
refactor: lint
miles-grant-ibigroup Feb 24, 2023
e526aae
refactor(MobilityDataValidationResult): draw tables
miles-grant-ibigroup Feb 24, 2023
7b084f0
Merge branch 'dev' into mobility-data-validator
miles-grant-ibigroup Feb 28, 2023
c07756c
refactor(MobilityDataValidationResult): new validator matches old val…
miles-grant-ibigroup Mar 1, 2023
d9dd263
chore(MobilityDataValidationResult): add key events
miles-grant-ibigroup Mar 1, 2023
3809343
chore: correct `mobilityDataResult` type
miles-grant-ibigroup Mar 1, 2023
ffafb0e
refactor(MobilityDataValidationResult): update strings
miles-grant-ibigroup Mar 1, 2023
095d60d
test: update snapshots
miles-grant-ibigroup Mar 1, 2023
a46cb70
clean up table cells
miles-grant-ibigroup Mar 7, 2023
539dc07
rough version of dual labels
miles-grant-ibigroup Mar 7, 2023
b2115b0
include link to mobility data docs
miles-grant-ibigroup Mar 7, 2023
504c534
add initial mobility data rule import script
miles-grant-ibigroup Mar 7, 2023
a04e332
Merge branch 'dev' into mobility-data-validator
miles-grant-ibigroup Mar 7, 2023
7b99947
lint
miles-grant-ibigroup Mar 8, 2023
177a106
render MB rules as html
miles-grant-ibigroup Mar 9, 2023
7907ced
import-mobility-data-rules: support edge cases
miles-grant-ibigroup Mar 9, 2023
56262cf
use number summaries for validation error count
miles-grant-ibigroup Mar 9, 2023
c3e8ec8
lint
miles-grant-ibigroup Mar 9, 2023
4597d6c
repair link rel
miles-grant-ibigroup Mar 10, 2023
bee31de
render complete info for missing_recommended_field
miles-grant-ibigroup Mar 10, 2023
d1438a4
e2e tests: use correct java version
miles-grant-ibigroup Mar 10, 2023
195c2ec
add wget to e2e image
miles-grant-ibigroup Mar 10, 2023
2abfd0a
refactor: address pr feedback
miles-grant-ibigroup Mar 10, 2023
81b1f60
refactor: sticky headers
miles-grant-ibigroup Mar 17, 2023
1faf20f
attempt to shorten problematically wide table
miles-grant-ibigroup Mar 17, 2023
5500c45
refactor: address pr feedback
miles-grant-ibigroup Mar 22, 2023
7725b9e
refactor: address pr feedback
miles-grant-ibigroup Mar 29, 2023
8ec815b
chore: update imports
miles-grant-ibigroup Mar 29, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion __tests__/e2e/server/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# syntax=docker/dockerfile:1
FROM maven:3.8.6-openjdk-8
FROM maven:3.8.7-openjdk-18

WORKDIR /datatools

ARG E2E_AUTH0_USERNAME
Expand Down Expand Up @@ -32,6 +33,7 @@ ARG AWS_SECRET_ACCESS_KEY

# Grab latest dev build of Datatools Server
RUN git clone https://github.com/ibi-group/datatools-server.git
RUN microdnf install wget
WORKDIR /datatools/datatools-server

RUN mvn package -DskipTests
Expand Down
1 change: 1 addition & 0 deletions __tests__/test-utils/mock-data/manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ export const mockFeedVersion = {
feedVersionId: 'mock-feed-version-id',
loadFailureReason: null,
loadStatus: 'SUCCESS',
mobilityDataResult: {},
routeCount: 10,
startDate: '20180801',
stopCount: 237,
Expand Down
15 changes: 14 additions & 1 deletion lib/manager/components/validation/GtfsValidationViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import type {Props as FeedVersionViewerProps} from '../version/FeedVersionViewer
import type {ValidationResult} from '../../../types'

import ValidationErrorItem from './ValidationErrorItem'
import MobilityDataValidationResult from './MobilityDataValidationResult'

const DEFAULT_LIMIT = 10

Expand Down Expand Up @@ -266,9 +267,21 @@ export default class GtfsValidationViewer extends Component<Props, State> {
</Alert>
}
<Panel>
<Panel.Heading><Panel.Title componentClass='h2'>{this.messages('title')}</Panel.Title></Panel.Heading>
<Panel.Heading>
<Panel.Title componentClass='h2'>{this.messages('title')}</Panel.Title>
</Panel.Heading>
{validationContent}
</Panel>
<Panel><Panel.Heading>Mobility Data Validation Issues</Panel.Heading>
<ListGroup>
{version.mobilityDataResult && version.mobilityDataResult.notices.map(notice => (
<MobilityDataValidationResult notice={notice} />
))}
{(!version.mobilityDataResult || version.mobilityDataResult.notices.length === 0) && <ListGroupItem className='validation-item'>
The MobilityData validator has not produced any errors or warnings. This may be because the validator is still running.
</ListGroupItem>}
</ListGroup>
</Panel>
</div>
)
}
Expand Down
154 changes: 154 additions & 0 deletions lib/manager/components/validation/MobilityDataValidationResult.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/* eslint-disable jsx-a11y/no-noninteractive-element-interactions */
/* eslint-disable no-fallthrough */
import React, { useState } from 'react'
import Icon from '@conveyal/woonerf/components/icon'
import { ListGroupItem, Table } from 'react-bootstrap'
import Markdown from 'markdown-to-jsx'

import toSentenceCase from '../../../common/util/to-sentence-case'
import {
mobilityDataValidationErrorMapping,
validationErrorIconLookup
} from '../../util/version'

import rules from './rules.json'

// from https://stackoverflow.com/a/4149671
function unCamelCase (s) {
return s
.split(/(?=[A-Z])/)
.join(' ')
.toLowerCase()
}

const NoticeTable = ({ headerOverides = {'stopSequence1': 'Stop seq-uence 1', 'stopSequence2': 'Stop seq-uence 2'}, notices }) => {
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
if (notices.length === 0) return null

const headers = Object.keys(notices[0])

return (
<Table bordered className='table-fixed' fill hover striped style={{display: 'relative', borderCollapse: 'collapse'}}>
<thead>
<tr>
{headers.map((header) => (
<th style={{position: 'sticky', top: 0, background: '#fefefe'}}>
{headerOverides[header] || toSentenceCase(unCamelCase(header))}
</th>
))}
</tr>
</thead>
<tbody>
{notices.map((notice) => (
<tr>
{headers.map((header, index) => {
const FieldWrapper =
(header === 'fieldValue' || header === 'message') ? 'pre' : React.Fragment

let field = notice[header]
if (header.endsWith('Km') || header.endsWith('Kph')) {
field = Math.round(field)
}

return (
<td key={`${header}-${index}`}>
<FieldWrapper>{field}</FieldWrapper>
</td>
)
})}
</tr>
))}
</tbody>
</Table>
)
}

// eslint-disable-next-line complexity
const renderNoticeDetail = (notice) => {
switch (notice.code) {
case 'too_many_rows':
notice.csvRowNumber = notice.rowNumber
case 'fare_transfer_rule_duration_limit_type_without_duration_limit':
case 'fare_transfer_rule_duration_limit_without_type':
case 'fare_transfer_rule_missing_transfer_count':
case 'fare_transfer_rule_with_forbidden_transfer_count':
notice.filename = 'fare_transfer_rules.txt'
case 'empty_file':
case 'emtpy_row':
case 'missing_timepoint_column':
case 'missing_required_file':
case 'missing_recommended_file':
case 'unknown_file':
return (
<ul>
{notice.sampleNotices.map((notice) => (
<li>
{notice.filename}
{notice.csvRowNumber && `: row ${notice.csvRowNumber}`}
</li>
))}
</ul>
)
default:
return (
<NoticeTable notices={notice.sampleNotices} />
)
}
}

const MobilityDataValidationResult = (props) => {
const { notice } = props
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
const rule = rules.find((rd) => rd.rule === notice.code)
Copy link
Contributor

Choose a reason for hiding this comment

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

would indexOf be more efficient here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be but I think it would be dirtier code wouldn't it

if (!rule) return null

const errorClass = `gtfs-error-${mobilityDataValidationErrorMapping[notice.severity]}`
const [expanded, setExpanded] = useState(notice.totalNotices < 2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this...

Copy link
Contributor

Choose a reason for hiding this comment

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

What are you not sure about? If 2 is a good number for when to show the notices?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and if we should expand by default at all?


const onRowSelect = () => setExpanded(!expanded)

return (
<ListGroupItem style={{ padding: 0 }}>
<div style={{ padding: '10px 15px' }}>
<h4
onClick={onRowSelect}
onKeyDown={onRowSelect}
style={{ cursor: 'pointer' }}
>
<span
className={`buffer-icon ${errorClass}`}
title={`${toSentenceCase(notice.severity)} priority`}
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
>
<Icon
type={validationErrorIconLookup[mobilityDataValidationErrorMapping[notice.severity]]}
/>
</span>
{toSentenceCase(notice.code.replaceAll('_', ' ').toLowerCase())}
<span className={errorClass}>
{' '}
&mdash; {notice.totalNotices} case
{notice.totalNotices > 1 ? 's' : ''} found
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
</span>
<span className={`pull-right`}>
<Icon type={expanded ? 'caret-up' : 'caret-down'} />
</span>
</h4>
{expanded && (
<>
<p><Markdown>{rule.description}</Markdown></p>
<p>
<a
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
href={`https://github.com/MobilityData/gtfs-validator/blob/master/RULES.md#${notice.code}`}
target='_blank'
rel='noreferrer'
>
More details
</a>
</p>
</>
)}
</div>
<div>{expanded && renderNoticeDetail(notice)}</div>
</ListGroupItem>
)
}

export default MobilityDataValidationResult
1 change: 1 addition & 0 deletions lib/manager/components/validation/rules.json

Large diffs are not rendered by default.

33 changes: 29 additions & 4 deletions lib/manager/components/version/FeedVersionViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import * as plusActions from '../../../gtfsplus/actions/gtfsplus'
import ActiveGtfsPlusVersionSummary from '../../../gtfsplus/containers/ActiveGtfsPlusVersionSummary'
import NotesViewer from '../NotesViewer'
import TransformationsViewer from '../TransformationsViewer'
import { errorPriorityLevels, getTableFatalExceptions } from '../../util/version'
import { errorPriorityLevels, getTableFatalExceptions, mobilityDataValidationErrorMapping } from '../../util/version'
import GtfsValidationViewer from '../validation/GtfsValidationViewer'
import type { Feed, FeedVersion, GtfsPlusValidation, Note, Project } from '../../../types'
import type { GtfsState, ManagerUserState } from '../../../types/reducers'
Expand All @@ -32,6 +32,7 @@ import VersionButtonToolbar from './VersionButtonToolbar'
import VersionDateLabel from './VersionDateLabel'
import FeedVersionReport from './FeedVersionReport'
import DeltaStat from './DeltaStat'
import numeral from 'numeral'

export type Props = {
comparedVersion?: FeedVersion,
Expand Down Expand Up @@ -150,6 +151,7 @@ type SelectorProps = {
}

class VersionSectionSelector extends Component<SelectorProps> {
// eslint-disable-next-line complexity
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
_renderIssuesLabel = () => {
const { comparedVersion, version } = this.props
const { feedLoadResult, validationResult, validationSummary } = version
Expand All @@ -171,9 +173,29 @@ class VersionSectionSelector extends Component<SelectorProps> {
})
}

const text = hasCriticalError
// Do the same for the mobility data results
let highestMBPriority = 'UNKNOWN'
if (version && version.mobilityDataResult) {
version.mobilityDataResult.notices.forEach(notice => {
const level = errorPriorityLevels[mobilityDataValidationErrorMapping[notice.severity]]
if (level < errorPriorityLevels[highestMBPriority]) {
highestMBPriority = mobilityDataValidationErrorMapping[notice.severity]
}
})
}

const dtValidationCount = hasCriticalError
? 'critical error'
: validationSummary.errorCount
: numeral(validationSummary.errorCount).format('0a')

const mbValidationCount =
version.mobilityDataResult &&
version.mobilityDataResult.notices &&
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
numeral(
version.mobilityDataResult.notices.reduce((prev, cur) => {
return prev + cur.totalNotices
}, 0)
).format('0a')

let diffLabel
if (comparedVersion) {
Expand All @@ -190,8 +212,11 @@ class VersionSectionSelector extends Component<SelectorProps> {
return (
<div style={{ display: 'inline' }}>
<BsLabel className={`gtfs-error-${highestPriority}-label-solid`}>
{text}
DT {dtValidationCount}
</BsLabel>
{mbValidationCount && <BsLabel style={{marginLeft: '1ch'}} className={`gtfs-error-${highestMBPriority}-label-solid`}>
MD {mbValidationCount}
</BsLabel> }
{diffLabel}
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down Expand Up @@ -344,6 +345,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down Expand Up @@ -637,6 +639,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down Expand Up @@ -801,6 +804,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down Expand Up @@ -1153,6 +1157,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down Expand Up @@ -1282,6 +1287,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down Expand Up @@ -1690,6 +1696,7 @@ exports[`lib > manager > DeploymentsPanel should render with the list of deploym
"feedVersionId": "mock-feed-version-id",
"loadFailureReason": null,
"loadStatus": "SUCCESS",
"mobilityDataResult": Object {},
"routeCount": 10,
"startDate": "20180801",
"stopCount": 237,
Expand Down
Loading