Skip to content

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Aug 19, 2021

There are various cases where generateStage returns {} so the pipeline array has valid stages with empty objects mixed in. Solution was to just filter those out where they get made in openCreateView.

I split the code out so I can easily unit test it as openCreateView suffers from that thunk dispatch problem where it isn't easy to spy on what it passes on.

Comment on lines +688 to +689
// generateStage can return {} under various conditions
.filter((stage) => !isEmpty(stage));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can change generateStage in a way that returns something that is easier to identify as an empty stage (like null or something) and then we don't need this dependency on isEmpty to do the check (lodash is used a lot across the codebase, but we are trying to use it less recently and remove it where appropriate), wdyt?

Copy link
Contributor Author

@lerouxb lerouxb Aug 20, 2021

Choose a reason for hiding this comment

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

I considered that and had a look at the other places (outside of tests) that call generateStage():

We'd have to change those to turn null back into {} because that seems to be what they expect. So I'm not sure - it seems debatable either way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense, thanks for the context!

@lerouxb lerouxb merged commit db22d48 into main Aug 20, 2021
@lerouxb lerouxb deleted the COMPASS-4977-hidden-stages branch August 20, 2021 10:20
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.

2 participants