Repository: ibi-group/datatools-server
Issue: Bug report: MTC feed merge

From MTC: 
> Production TDM's Merge functionality is not following [rule number 5 defined in user docs](https://mtc-datatools.readthedocs.io/en/mtc-docs/user/merging-feeds/#merge-rules). Two feeds with same service_id and overlapping dates are merged but the start and end dates are not updated. Attached are sample feeds for reference.
>
> Please investigate the issue and help us provide a fix.  

Input datasets:
[Livermore_Amador_Valley_Transit_Authority-current.zip](https://github.com/ibi-group/datatools-server/files/4583453/Livermore_Amador_Valley_Transit_Authority-current.zip)
[Livermore_Amador_Valley_Transit_Authority-Future-2020May-01.zip](https://github.com/ibi-group/datatools-server/files/4583452/Livermore_Amador_Valley_Transit_Authority-Future-2020May-01.zip)

The merged result calendar.txt should have records with prefixed service_ids `Livermore_Amador_Valley_Transit_Authority30` that have end_dates of May 3rd (but they are showing June 14th).

Changed files:
src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java, src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java

diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java b/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java
index 1279464e..3770f435 100644
--- a/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java
+++ b/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java
@@ -508,7 +508,9 @@ public class MergeFeedsJob extends MonitorableJob {
                         // Get index of field from GTFS spec as it appears in feed
                         int index = fieldsFoundList.indexOf(field);
                         String val = csvReader.get(index);
-                        // Default value to write is unchanged from value found in csv.
+                        // Default value to write is unchanged from value found in csv (i.e. val). Note: if looking to
+                        // modify the value that is written in the merged file, you must update valueToWrite (e.g.,
+                        // updating the current feed's end_date or accounting for cases where IDs conflict).
                         String valueToWrite = val;
                         // Handle filling in agency_id if missing when merging regional feeds.
                         if (newAgencyId != null && field.name.equals("agency_id") && mergeType
@@ -615,10 +617,9 @@ public class MergeFeedsJob extends MonitorableJob {
                                             getFieldIndex(fieldsFoundInZip, "end_date");
                                         if (index == endDateIndex) {
                                             LocalDate endDate = LocalDate
-                                                .parse(csvReader.get(endDateIndex),
-                                                    GTFS_DATE_FORMATTER);
+                                                .parse(csvReader.get(endDateIndex), GTFS_DATE_FORMATTER);
                                             if (!endDate.isBefore(futureFeedFirstDate)) {
-                                                val = futureFeedFirstDate
+                                                val = valueToWrite = futureFeedFirstDate
                                                     .minus(1, ChronoUnit.DAYS)
                                                     .format(GTFS_DATE_FORMATTER);
                                             }
diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java
index dd4a8226..26d97f1d 100644
--- a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java
+++ b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java
@@ -23,6 +23,7 @@ import java.util.UUID;
 
 import static com.conveyal.datatools.TestUtils.assertThatFeedHasNoErrorsOfType;
 import static com.conveyal.datatools.TestUtils.assertThatSqlCountQueryYieldsExpectedCount;
+import static com.conveyal.datatools.TestUtils.assertThatSqlQueryYieldsRowCount;
 import static com.conveyal.datatools.TestUtils.createFeedVersion;
 import static com.conveyal.datatools.TestUtils.createFeedVersionFromGtfsZip;
 import static com.conveyal.datatools.TestUtils.zipFolderFiles;
@@ -523,7 +524,6 @@ public class MergeFeedsJobTest extends UnitTest {
      * Tests whether a MTC feed merge of two feed versions correctly removes calendar records that have overlapping
      * service but keeps calendar_dates records that share service_id with the removed calendar and trips that reference
      * that service_id.
-     *
      */
     @Test
     public void canMergeFeedsWithMTCForServiceIds3 () throws SQLException {
@@ -563,6 +563,15 @@ public class MergeFeedsJobTest extends UnitTest {
             ),
             1
         );
+        // Amended calendar record from earlier feed version should also have a modified end date (one day before the
+        // earliest start_date from the future feed).
+        assertThatSqlCountQueryYieldsExpectedCount(
+            String.format(
+                "SELECT count(*) FROM %s.calendar WHERE service_id='Fake_Agency4:common_id' AND end_date='20170914'",
+                mergedNamespace
+            ),
+            1
+        );
         // Modified cal_to_remove should still exist in calendar_dates. It is modified even though it does not exist in
         // the future feed due to the MTC requirement to update all service_ids in the past feed.
         // See https://github.com/ibi-group/datatools-server/issues/244