Skip to content

Conversation

@pmckeown
Copy link

The data structure for the all journey import from seperate files feature is broken in the new version.

The lib expects a map like:

{
  "TreeID1": { //tree content },
  "TreeID2": { //tree content }
}

But this version is providing it in a different shape.

This breaks importing journeys exported using the versions frodo-cli v3.0.0 and lib 3.0.1.

Command:
frodo journey import --directory alpha/journey https://<LOCAL_DEPLOYMENT>/am alpha --all-separate

@vscheuber vscheuber requested a review from phalestrivir March 31, 2025 17:25
@vscheuber
Copy link
Contributor

vscheuber commented Mar 31, 2025

@phalestrivir @hfranklin could you take a look at this PR and if the issue it addresses was caused by some of the great work you did on export/import?

Copy link
Contributor

@phalestrivir phalestrivir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this change reverts back to how it was pre 3.0.0. However, I don't think we want to do that.

The reason for the change is that I noticed the exports were NOT in the same format as they are when you have them exported by Cloud in the admin UI. The cloud format is:

{
  "trees": {
    "myTreeName": {
       "tree": { ... },
       "nodes": { ... },
       "innerNodes": { ... },
        ....
    }
  }
}

This code was updated so it would work with that format. I also meant to update the exports to be in that format. This is the case when you do frodo config export -AD, it will export the journeys separately in this corrected format. The tests use an export made from frodo config export -AD, so me adding in this change allowed the tests to pass. However, I must have missed that the frodo journey export -AD command wasn't doing the same thing.

Looking at JourneyOps further in the "exportJourneysToFiles" method, I see that we are doing saveJsonToFile(treeValue, file, includeMeta);. Normally, we would do something like saveToFile('trees', treeValue, '_id', file, includeMeta); instead to export it in the right format, but due to the format that the trees are in this won't work since the _id field is on treeValue.tree and not on treeValue, so we would either need to modify saveToFile in frodo-lib to allow for cases like this, or take the code from saveToFile and customize it to fit our need here. I think the second option would be simplest to avoid potentially having to refactor anything that uses saveToFile, as well as to avoid another frodo-lib PR. To do that, we would just need to modify the saveJsonToFile method call to be saveJsonToFile({ trees: { [treeValue.tree._id]: treeValue }}, file, includeMeta);, and that should work

To update the PR, just add this fix to the export method, and revert the change in the import method, and that should be everything to fix the issue. The journey export test snapshots will likely need to be updated as well for the tests to pass

@vscheuber
Copy link
Contributor

@phalestrivir @pmckeown is this PR ready to merge?

@pmckeown
Copy link
Author

Sounds like @phalestrivir has a better approach to fix this issue but I don't have enough indepth knowledge of the code base to apply his recommendations.

@phalestrivir
Copy link
Contributor

@pmckeown No worries, I can go ahead and make those changes for you. I probably won't be able to get to it until later this week though

@phalestrivir
Copy link
Contributor

This PR can be closed, I created a new PR with the fix to the exports here

@pmckeown
Copy link
Author

Thank you @phalestrivir

@pmckeown pmckeown closed this Apr 30, 2025
phalestrivir referenced this pull request Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants