-
Notifications
You must be signed in to change notification settings - Fork 53
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 option to export patterns #567
Conversation
…om/ibi-group/datatools-server into add-option-for-publishing-patterns
…ary files with comments
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.
LGTM. Will be good to get this merged so dev is in-sync with GTFS-lib dev which already contains this work.
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.
Very clean, just one major q
@@ -63,6 +63,12 @@ private static Snapshot getSnapshotFromRequest(Request req) { | |||
return Persistence.snapshots.getById(id); | |||
} | |||
|
|||
private static boolean getPublishProprietaryFiles(Request req) { | |||
return Boolean.parseBoolean( | |||
req.queryParamOrDefault("publishProprietaryFiles",Boolean.FALSE.toString()) |
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.
req.queryParamOrDefault("publishProprietaryFiles",Boolean.FALSE.toString()) | |
req.queryParamOrDefault("publishProprietaryFiles", Boolean.FALSE.toString()) |
Missing space
@@ -130,7 +130,8 @@ public void jobLogic() { | |||
snapshot.feedTransformResult = dbTarget.feedTransformResult; | |||
// If the user has selected to create a new version from the resulting snapshot, do so here. | |||
if (rules.createNewVersion) { | |||
addNextJob(new CreateFeedVersionFromSnapshotJob(feedSource, snapshot, owner)); | |||
// Publishing the proprietary files will preserve the pattern names in the newly created feed version. | |||
addNextJob(new CreateFeedVersionFromSnapshotJob(feedSource, snapshot, owner, true)); |
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 feels like a scary new default. How are we sure this won't have weird side effects?
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 a safe default bc the method is only used to process incoming GTFS files. All the export methods allow for user selection if they want to export the pattern names and this just allows re-import
Checklist
dev
before they can be merged tomaster
)Description
This PR facilitates the export of the proprietary
datatools_patterns.txt
file, introduced in https://github.com/conveyal/gtfs-lib/pulls, which allows the preservation of custom pattern names in the editor.Front end PR: https://github.com/ibi-group/datatools-ui/compare/add-option-for-publishing-patterns?expand=1
GTFS-lib PR: conveyal/gtfs-lib#394