Prevent multiple calls to concept set list when loading cohort editor

[anthonysena]

Aims to fix #2887 by extending the concept set browser to only load the repository concept set list when the modal is activated.

[chrisknoll]

Suggestion (sorry I didn't try this myself): I noticed you have set up subscriptions (make sure you dispose them) to check if the showModal observable changes, and it is true, to do the concept set fetch....

I think the problem is that inside the modal component, we place the concept set component that does the fetch and it's just the lifecycle of the component that wants to do it: we instantiate 5 components so we will fetch the 5 concept sets.

To prevent the instantiation of the component could we put an if: block around the contents of the modal such that everything inside the module won't be instantiated until the showModal goes true, and then the component gets instantiated and the service call is made? (This is theoretically)

The end effect shoudl be the same as what you have (ie: the if: binding observes the showModal and will control the load) but without creating addiitional manual subscriptions that you need to clean up.

[anthonysena]

Suggestion (sorry I didn't try this myself): I noticed you have set up subscriptions (make sure you dispose them) to check if the showModal observable changes, and it is true, to do the concept set fetch....

That's correct - I need to clean up the subscription for sure.

I think the problem is that inside the modal component, we place the concept set component that does the fetch and it's just the lifecycle of the component that wants to do it: we instantiate 5 components so we will fetch the 5 concept sets.

To prevent the instantiation of the component could we put an if: block around the contents of the modal such that everything inside the module won't be instantiated until the showModal goes true, and then the component gets instantiated and the service call is made? (This is theoretically)

In theory that makes sense and would work well. What I recall from the code base is that the component for displaying the concept set is used in a number of places. So, my thinking was that by extending the component to be "modal aware" through its exposed parameter list, it would make modifying other things in Atlas a bit easier so that we only need to worry about where it is used modally vs. having to think about decorating those code blocks where it is used modally. They are functionally equivalent in my mind so I don't have a strong opinion either way.

The end effect shoudl be the same as what you have (ie: the if: binding observes the showModal and will control the load) but without creating addiitional manual subscriptions that you need to clean up.

Agreed - in the meantime, I will update this PR to clean up the subscription but if we want to go with the if: block approach we can close this out in favor of that set of changes.

[anthonysena]

@chrisknoll I've updated this PR to use the if: approach you described and removed the subscription. It should be ready for review now.