Updates to gtfs spec changes - front end UI fields only #668

robertgregg3 · 2021-05-06T16:08:40Z

Relating to #663

Added the new location types to gtfs.yml
Added validation if one or more agencies exists to add an Agency ID
Added conditional validation to stop_lat and stop_lng to be required if the location type has a value of 2 or more

Also fix #716, fix #717.

… agency.txt #663

robertgregg3 · 2021-05-06T16:20:01Z

This is a first draft at the changes, so there are a few console logs and other bits I will remove when completed. Just wanted to see if I was on the right track, and I have a few questions:

I am getting a linting error on line 198 "Expected a 'break' statement before 'case'". I'm not sure why
On line 259 it looks like an attempt to validate the agency ID, so is it worth removing that one? Or is that for a different case?

datatools-ui/lib/editor/util/validation.js

Lines 259 to 271 in b299745

    
           case 'GTFS_AGENCY': 
        
             if ( 
        
               name === 'agency_id' && 
        
               agencies.length > 1 
        
             ) { 
        
               if (valueDoesNotExist) { 
        
                 return { 
        
                   field: name, 
        
                   invalid: true, 
        
                   reason: 'Field must be populated for feeds with more than one agency.' 
        
                 } 
        
               } 
        
             }

binh-dam-ibigroup · 2021-05-06T16:36:54Z

This is a first draft at the changes and I have a few questions:

I am getting a linting error on line 198 "Expected a 'break' statement before 'case'". I'm not sure why

On line 259 it looks like an attempt to validate the agency ID, so is it worth removing that one? Or is that for a different case?

datatools-ui/lib/editor/util/validation.js

Lines 259 to 271 in b299745

case 'GTFS_AGENCY':

if (

name === 'agency_id' &&

agencies.length > 1

) {

if (valueDoesNotExist) {

return {

field: name,

invalid: true,

reason: 'Field must be populated for feeds with more than one agency.'

}

}

}

@robertgregg3 Not all paths in that case statement lead to a return if you take a closer look!

binh-dam-ibigroup

There are some bits of logic I am not sure to follow. If I got it wrong, then a code comment would help people follow the reasoning. Also see my refactoring suggestions so far and my comment for making the lint check pass.

lib/editor/util/validation.js

binh-dam-ibigroup · 2021-05-06T21:38:46Z

lib/editor/util/validation.js

+      console.log(idList.length)
+      console.log({entity})


Remove console statements.

binh-dam-ibigroup · 2021-05-06T21:47:10Z

lib/editor/util/validation.js

      )
+      console.log(idList.length)
+      console.log({entity})
+      if (agencies.length > 1 && entity.agency_id === null) {


Try using valueDoesNotExist, isOptionalAndEmpty defined for the function that already checked for null/undefined/empty string.

Good point!

lib/editor/util/validation.js

binh-dam-ibigroup · 2021-05-06T21:52:47Z

lib/editor/util/validation.js

+      if (isNotLat) {
+        reason = 'Field must be valid latitude.'
+        return {field: name, invalid: isOptionalAndEmpty || isNotLat, reason}
+      }


Should the range check from isNotLat be enforced here regardless of stop type?

@binh-dam-ibigroup it is being enforced regardless. Line 189 is the conditional for the location_type:

binh-dam-ibigroup · 2021-05-06T21:55:11Z

lib/editor/util/validation.js

+        reason = 'Field must be valid latitude.'
+        return {field: name, invalid: isOptionalAndEmpty || isNotLat, reason}
+      }
+      if (entity && entity.location_type >= 2) {


<= 2 rather?

Yes! 🤦‍♂️

lib/editor/util/validation.js

binh-dam-ibigroup · 2021-05-06T21:57:17Z

lib/editor/util/validation.js

+        reason = 'Field must be valid longitude.'
+        return {field: name, invalid: isOptionalAndEmpty || isNotLng, reason}
+      }
+      if (entity && entity.location_type >= 2) {


<= 2 instead?

Yes! 🤦‍♂️

binh-dam-ibigroup · 2021-05-06T21:58:14Z

lib/editor/util/validation.js

+      }
+      if (entity && entity.location_type >= 2) {
+        if (isOptionalAndEmpty) {
+          reason = '1 Latitude and Longitude are required for your current location type'


Remove leading 1

binh-dam-ibigroup · 2021-05-06T21:58:26Z

lib/editor/util/validation.js

+      }
+      if (entity && entity.location_type >= 2) {
+        if (isOptionalAndEmpty) {
+          reason = '1 Latitude and Longitude are required for your current location type'


Remove leading 1

#668

codecov-commenter · 2021-05-10T10:48:40Z

Codecov Report

Merging #668 (ef69da3) into dev (4e7b8ea) will increase coverage by 0.00%.
The diff coverage is 18.91%.

@@           Coverage Diff           @@
##              dev     #668   +/-   ##
=======================================
  Coverage   15.68%   15.68%           
=======================================
  Files         329      329           
  Lines       16797    16823   +26     
  Branches     5062     5071    +9     
=======================================
+ Hits         2634     2639    +5     
- Misses      12113    12133   +20     
- Partials     2050     2051    +1

Flag	Coverage Δ
unit_tests	`15.68% <18.91%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/editor/components/pattern/PatternStopCard.js	`0.00% <0.00%> (ø)`
lib/editor/util/map.js	`38.51% <ø> (ø)`
lib/gtfs/util/index.js	`4.34% <ø> (ø)`
lib/types/index.js	`0.00% <ø> (ø)`
lib/editor/util/validation.js	`45.00% <41.17%> (+2.44%)`	⬆️

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 4e7b8ea...ef69da3. Read the comment docs.

binh-dam-ibigroup · 2021-05-10T13:48:23Z

lib/editor/util/validation.js

+        return {field: name, invalid: isNotLat, reason}
      }
-      if (entity && entity.location_type >= 2) {
+      if (!locationType || locationType < 2) {


Check the GTFS reference for the location types, I think the condition should be <= 2: https://developers.google.com/transit/gtfs/reference#stopstxt

…rious GTFS changes and updates gtfs.yml

… routes.txt

Added defult_lang, feed_contact_email, feed_contact_url #663

binh-dam-ibigroup · 2021-05-11T20:03:27Z

Reassign and request reviews when done, thanks!

….js): Update to the GTFS Spec U Added the changes where applicable to the gtfs.yml file. Added the new types for continuous drop off/pick up and added fields as well as basic validation in the patternStopCard file. BREAKING CHANGE: There have been added fields that need to coincide with changes on the backend as the api will be looking for the new fields and those fields need to be present on the Front end #663

Removed spaces. Improved boolean ternary #663

binh-dam-ibigroup · 2021-06-01T19:27:27Z

@robertgregg3 there are still build errors, could you fix them, please?

binh-dam-ibigroup

It looks like my previous comment regarding enforcing lat/lon values even for stop types that don't require it was not resolved? Also, the build needs to be fixed, and some long lines need to be broken out.

lib/editor/components/pattern/PatternStopCard.js

binh-dam-ibigroup · 2021-06-01T20:48:35Z

lib/editor/util/validation.js

 * Returns false if the value is ok.
 * Returns an EditorValidationIssue object if the value is not ok.
 */
+// eslint-disable-next-line complexity


Remove that.

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.

lib/editor/util/validation.js

Changed wheelchairBoarding to a string. Added continuous fields to the map.js. #663

binh-dam-ibigroup

See formatting and refactoring suggestions.

binh-dam-ibigroup · 2021-06-07T21:05:52Z

lib/editor/components/pattern/PatternStopCard.js

+                componentClass='select'
+                placeholder='select'
+                disabled={hasShapeId}
+                defaultValue={1}


defaultValue is no longer needed (we set the value prop instead). (Also, sort props alphabetically)

Gotcha! Thanks.

lib/editor/components/pattern/PatternStopCard.js

binh-dam-ibigroup · 2021-06-07T21:14:02Z

lib/editor/components/pattern/PatternStopCard.js

+            </FormGroup>
+          </Col>
+          <Col xs={6}>
+            <FormGroup


I would recommend extracting this <FormGroup> and its contents into a component and reuse in all 4 instances.

Yes good idea!

Any progress on this one, by curiosity?

lib/editor/components/pattern/PatternStopCard.js

binh-dam-ibigroup

Validation of stop_name is interfering with editing of stop data on existing feeds.

lib/editor/util/validation.js

binh-dam-ibigroup

LGTM, sorry it fell off the radar due to lack of assignment.

evansiroky

Good to go after addressing this comment: #668 (comment)

…attribute.

binh-dam-ibigroup · 2021-09-21T14:18:42Z

Blocking this PR pending resolution of #716, #717 that it introduced.

Requested changes were added from #690.

…GTFS spec change fix #716, #717

…fields, address comments

fix(saveEntity, saveTripsForCalendar): Resolve missing defaults with GTFS spec changes

Rob Gregg added 2 commits May 6, 2021 16:50

feat(gtfs.yml and validation.js): Updated GTFS Spec for stops.txt and…

b299745

… agency.txt #663

style(validation.js): removed space

976b5c7

#663

robertgregg3 assigned landonreed, evansiroky and binh-dam-ibigroup May 6, 2021

binh-dam-ibigroup requested changes May 6, 2021

View reviewed changes

binh-dam-ibigroup assigned robertgregg3 and unassigned binh-dam-ibigroup May 6, 2021

refactor(validation.js): SOrted typing issues and removed some code

b1341cf

#668

robertgregg3 assigned binh-dam-ibigroup May 10, 2021

binh-dam-ibigroup reviewed May 10, 2021

View reviewed changes

robertgregg3 unassigned landonreed and evansiroky May 10, 2021

Rob Gregg added 3 commits May 11, 2021 15:10

improvement(gtfs.yml, end-to-end.js, index.js,validation.js): WIP: Va…

8a9c383

…rious GTFS changes and updates gtfs.yml

improvement(gtfs.yml): Added continuous_pickup and drop off fields to…

baaba8b

… routes.txt

feat(gtfs.yml): Added fields to feed_info.txt

acd651a

Added defult_lang, feed_contact_email, feed_contact_url #663

binh-dam-ibigroup removed their assignment May 11, 2021

Rob Gregg added 2 commits June 1, 2021 16:47

refactor(patternStopCard.js): Refactored code from github feedback

5397f4a

Removed spaces. Improved boolean ternary #663

binh-dam-ibigroup requested changes Jun 1, 2021

View reviewed changes

refactor(end-to-end.js map.js index.js): Fix errors

ad6bfe6

Changed wheelchairBoarding to a string. Added continuous fields to the map.js. #663

robertgregg3 changed the title ~~Updates to agency.txt and stops.txt spec changes~~ Updates to gtfs spec changes - front end UI fields only Jun 2, 2021

robertgregg3 requested a review from binh-dam-ibigroup June 2, 2021 15:34

refactor(PatternStopCard.js): Refactored based on PR Feedback.

6a9e2ed

binh-dam-ibigroup reviewed Jun 7, 2021

View reviewed changes

binh-dam-ibigroup requested changes Jul 19, 2021

View reviewed changes

lib/editor/util/validation.js Outdated Show resolved Hide resolved

binh-dam-ibigroup added 2 commits July 19, 2021 17:03

fix(editor/util/validation): Fix stop name validation.

ef69da3

style(editor/util/validation): Reformat stop_name validation condition

00ea4d4

robertgregg3 removed their assignment Aug 2, 2021

Merge branch 'dev' into gtfs-spec-changes

e180c43

binh-dam-ibigroup self-assigned this Sep 9, 2021

binh-dam-ibigroup approved these changes Sep 9, 2021

View reviewed changes

binh-dam-ibigroup assigned evansiroky and unassigned binh-dam-ibigroup Sep 9, 2021

Merge branch 'dev' into gtfs-spec-changes

1c5fd78

evansiroky approved these changes Sep 16, 2021

View reviewed changes

evansiroky assigned robertgregg3 and unassigned evansiroky Sep 16, 2021

binh-dam-ibigroup assigned binh-dam-ibigroup and unassigned robertgregg3 Sep 20, 2021

binh-dam-ibigroup mentioned this pull request Sep 20, 2021

stop_times.continuous_drop_off/continous_pickup incorrectly loaded? conveyal/gtfs-lib#325

Closed

refactor(PatternStopCard): Clone before updating pickup/dropoff stop …

59d429a

…attribute.

binh-dam-ibigroup added the BLOCKED Blocked (waiting on another PR to be merged) label Sep 21, 2021

fix(saveEntity, saveTripsForCalendar): Resolve missing defaults with …

548fb24

…GTFS spec change fix #716, #717

philip-cline mentioned this pull request Sep 22, 2021

fix(saveEntity, saveTripsForCalendar): Resolve missing defaults with GTFS spec changes #720

Merged

5 tasks

philip-cline and others added 4 commits September 22, 2021 12:26

refactor(fetchBaseGtfs, saveTripsForCalendar): Fix fetch of new spec …

c48bb18

…fields, address comments

refactor(saveTripsForCalendar): Fix missing tripCopy ref

cda4f65

Merge pull request #720 from ibi-group/resolve-missing-defaults

be6b088

fix(saveEntity, saveTripsForCalendar): Resolve missing defaults with GTFS spec changes

Merge branch 'dev' into gtfs-spec-changes

a5f9c88

binh-dam-ibigroup removed the BLOCKED Blocked (waiting on another PR to be merged) label Sep 23, 2021

binh-dam-ibigroup merged commit c8a7c82 into dev Sep 23, 2021

binh-dam-ibigroup mentioned this pull request Sep 23, 2021

Feature Release: June 22, 2022 #709

Merged

7 tasks

miles-grant-ibigroup deleted the gtfs-spec-changes branch November 10, 2021 14:06

Updates to gtfs spec changes - front end UI fields only #668

Updates to gtfs spec changes - front end UI fields only #668

Uh oh!

Conversation

robertgregg3 commented May 6, 2021 • edited by binh-dam-ibigroup Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertgregg3 commented May 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

binh-dam-ibigroup commented May 6, 2021

Uh oh!

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented May 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

binh-dam-ibigroup commented May 11, 2021

Uh oh!

binh-dam-ibigroup commented Jun 1, 2021

Uh oh!

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

robertgregg3 commented May 6, 2021 •

edited by binh-dam-ibigroup

Loading

robertgregg3 commented May 6, 2021 •

edited

Loading

codecov-commenter commented May 10, 2021 •

edited

Loading