-
Notifications
You must be signed in to change notification settings - Fork 88
Updates to gtfs spec changes - front end UI fields only #668
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
Changes from 2 commits
b299745
976b5c7
b1341cf
8a9c383
baaba8b
acd651a
93b1f53
5397f4a
ad6bfe6
6a9e2ed
b30a941
c4cf115
36a759d
79073f7
094efc7
7717c28
f1cb21e
ff05ac7
5d17fbb
c728b3d
212cf37
3b97719
826088d
4039af7
0255029
ae5eaae
cb588be
a95a02c
5696ff1
2eda50c
e692199
f567077
589b6c6
ef69da3
00ea4d4
e180c43
1c5fd78
59d429a
548fb24
c48bb18
cda4f65
be6b088
a5f9c88
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 |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ export function doesNotExist (value: any): boolean { | |
| * Returns false if the value is ok. | ||
| * Returns an EditorValidationIssue object if the value is not ok. | ||
| */ | ||
| // eslint-disable-next-line complexity | ||
| export function validate ( | ||
| field: GtfsSpecField, | ||
| value: any, | ||
|
|
@@ -36,8 +37,8 @@ export function validate ( | |
| const valueDoesNotExist = doesNotExist(value) | ||
| const isRequiredButEmpty = required && valueDoesNotExist | ||
| const isOptionalAndEmpty = !required && valueDoesNotExist | ||
| let reason = 'Required field must not be empty' | ||
| const agencies = getTableById(tableData, 'agency') | ||
| let reason = 'Required field must not be empty' | ||
robertgregg3 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // setting as a variable here because of eslint bug | ||
| type CheckPositiveOutput = { | ||
|
|
@@ -100,6 +101,12 @@ export function validate ( | |
| (indices.length > 1 || | ||
| (indices.length > 0 && entities[indices[0]].id !== entity.id)) | ||
| ) | ||
| console.log(idList.length) | ||
| console.log({entity}) | ||
|
||
| if (agencies.length > 1 && entity.agency_id === null) { | ||
|
||
| reason = 'Identifier is required if more than one agency exists' | ||
| return {field: name, invalid: isRequiredButEmpty, reason} | ||
robertgregg3 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| if (isRequiredButEmpty || isNotUnique) { | ||
| if (isNotUnique) { | ||
| reason = 'Identifier must be unique' | ||
|
|
@@ -175,21 +182,29 @@ export function validate ( | |
| } | ||
| case 'LATITUDE': | ||
| const isNotLat = value > 90 || value < -90 | ||
| if (isRequiredButEmpty || isNotLat) { | ||
| if (isNotLat) { | ||
| reason = 'Field must be valid latitude.' | ||
| if (isNotLat) { | ||
| reason = 'Field must be valid latitude.' | ||
| return {field: name, invalid: isOptionalAndEmpty || isNotLat, reason} | ||
| } | ||
|
Contributor
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. Should the range check from
Author
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. @binh-dam-ibigroup it is being enforced regardless. Line 189 is the conditional for the location_type: |
||
| if (entity && entity.location_type >= 2) { | ||
|
||
| if (isOptionalAndEmpty) { | ||
| reason = '1 Latitude and Longitude are required for your current location type' | ||
|
||
| return {field: name, invalid: isOptionalAndEmpty || isNotLat, reason} | ||
| } | ||
| return {field: name, invalid: isRequiredButEmpty || isNotLat, reason} | ||
| } else { | ||
| return false | ||
| } | ||
| case 'LONGITUDE': | ||
| const isNotLng = value > 180 || value < -180 | ||
| if (isRequiredButEmpty || isNotLng) { | ||
| if (isNotLng) { | ||
| reason = 'Field must be valid longitude.' | ||
| if (isNotLng) { | ||
robertgregg3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| reason = 'Field must be valid longitude.' | ||
| return {field: name, invalid: isOptionalAndEmpty || isNotLng, reason} | ||
| } | ||
| if (entity && entity.location_type >= 2) { | ||
|
||
| if (isOptionalAndEmpty) { | ||
| reason = '1 Latitude and Longitude are required for your current location type' | ||
|
||
| return {field: name, invalid: isOptionalAndEmpty || isNotLng, reason} | ||
| } | ||
| return {field: name, invalid: isRequiredButEmpty || isNotLng, reason} | ||
| } else { | ||
| return false | ||
| } | ||
|
|
||

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.
Remove that.
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.
This is not an error-level eslint rule with our eslint config, however, I think there is a lot of value in having it warn the developer because having functions with tons of complexity will make code very difficult to read and maintain in the future. It's not your fault that this method is bloated in the first place, but it will significantly improve the future readability and maintainability of this code to refactor this method into smaller chunks right now.