Update trip#shape_id and frequency stop_times on pattern updates #164
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #164 +/- ##
============================================
+ Coverage 55.21% 58.17% +2.96%
- Complexity 734 827 +93
============================================
Files 143 144 +1
Lines 7178 7596 +418
Branches 830 939 +109
============================================
+ Hits 3963 4419 +456
+ Misses 2876 2823 -53
- Partials 339 354 +15
Continue to review full report at Codecov.
|
landonreed
changed the title
| tablePrefix, | ||
| tablePrefix, | ||
| "pattern_id", | ||
| "stop_sequence" |
There was a problem hiding this comment.
Since these are hardcoded strings, they should be included in the base string.
| statement.setInt(oneBasedIndex++, arrivalTime + dwellTime); | ||
| // Set "where clause" with value for pattern_id and stop_sequence | ||
| statement.setString(oneBasedIndex++, patternStop.get("pattern_id").asText()); | ||
| statement.setInt(oneBasedIndex++, patternStop.get("stop_sequence").asInt()); |
There was a problem hiding this comment.
Using a variable for the prepared statement variable index seems a little too fancy. Since the variables aren't dynamically created and the length of the parameters isn't varying, I think it may be best to simply hardcode the field position number.
There was a problem hiding this comment.
It seems more maintainable and less error-prone to me to use an incrementing variable. As long as the you're setting the parameters in the correct order, adding a new parameter is simply a matter of inserting it in the right spot and using the same oneBasedIndex. If we were to use hardcoded numbers, updating the statement could involve updating multiple numbers one by one if the new parameter is placed anywhere except the end.
There was a problem hiding this comment.
ok, I don't have a super strong opinion on this, so go ahead.
…r trips This refactor includes a check for Pattern#useFrequency when either a pattern is updated or trips are updated in order to appropriately trigger other behaviors (for example, keeping stop times in sync with patter stops travel times or disallowing frequency entries for timetable-based patterns). fixes #173
| * which is a prerequisite for creating a frequency trip with stop times. | ||
| */ | ||
| @Test | ||
| public void canCreateFrequencyForFrequencyPattern() throws IOException, SQLException, InvalidNamespaceException { |
There was a problem hiding this comment.
Great start! This should be renamed to indicate that it also does some checking of the updating and deleting of some trips.
|
🎉 This PR is included in version 4.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
In addition to some formatting tweaks and added comments, this seeks to address the underlying cause for ibi-group/datatools-ui#385. It is another example of updates for "exemplar" fields not making their way back to the referencing entities (#109).
It also fixes #165, where stop_time arrivals and departures were not kept in sync with updates to the underlying pattern travel times. This is only a concern with frequency-based patterns, but the same method could potentially be utilized for updating travel times in timetable-based trips if needed in the future.
Finally, it adds some tests to
JdbcTableWriterfor creating frequency trips and fixes #173, which was discovered in the process of writing these tests.