-
Notifications
You must be signed in to change notification settings - Fork 81
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
Changes from all commits
e145922
a46ca8f
cf5fdad
e526aae
7b084f0
c07756c
d9dd263
3809343
ffafb0e
095d60d
a46cb70
539dc07
b2115b0
504c534
a04e332
7b99947
177a106
7907ced
56262cf
c3e8ec8
4597d6c
bee31de
d1438a4
195c2ec
2abfd0a
81b1f60
1faf20f
5500c45
7725b9e
8ec815b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// @flow | ||
|
||
import toLower from 'lodash/toLower' | ||
import upperFirst from 'lodash/upperFirst' | ||
|
||
export default function toSentenceCase (s: string): string { | ||
return upperFirst(toLower(s)) | ||
} | ||
|
||
/** | ||
* This method takes a string like expires_in_7days and ensures | ||
* that 7days is replaced with 7 days | ||
*/ | ||
// $FlowFixMe flow needs to learn about new es2021 features! | ||
export function spaceOutNumbers (s: string): string { | ||
return s.replaceAll('_', ' ') | ||
.split(/(?=[1-9])/) | ||
.join(' ') | ||
.toLowerCase() | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
/* 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, { spaceOutNumbers } from '../../../common/util/text' | ||
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 }) => { | ||
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 = ({notice}) => { | ||
const rule = rules.find((rd) => rd.rule === notice.code) | ||
if (!rule) return null | ||
|
||
const errorClass = `gtfs-error-${mobilityDataValidationErrorMapping[notice.severity]}` | ||
const [expanded, setExpanded] = useState(notice.totalNotices < 2) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about this... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(spaceOutNumbers(notice.code))} | ||
<span className={errorClass}> | ||
{' '} | ||
— {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 |
Large diffs are not rendered by default.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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