Skip to content
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

fix(313) - Kamelets in Camel Routes are not handled properly #358

Merged
merged 5 commits into from Nov 22, 2023

Conversation

shivamG640
Copy link
Contributor

@shivamG640 shivamG640 commented Nov 17, 2023

Fixes #313

Now, when we replace the source node, the catalog page loads like this:
image

Similarly, the Catalog page looks like this when we replace the last node:
image

@shivamG640 shivamG640 requested a review from a team November 17, 2023 06:21
@@ -132,7 +133,8 @@ export class CamelRouteResource implements CamelResource, BeansAwareResource {
* as this mean that they can be used only as a consumer.
*/
return (item: ITile) => {
return !item.tags.includes('consumerOnly');
return (item.type !== CatalogKind.Kamelet && !item.tags.includes('consumerOnly')) ||
(item.type === CatalogKind.Kamelet && !item.tags.includes('source'));
Copy link
Member

Choose a reason for hiding this comment

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

The conditions look good 👌

Do you think it would be possible to add a test for this method in the camel-route.test.ts file?

Certainly, we would need to have a mock for the catalog but it's not available yet, so we could create 3 scenarios and assert that we get a function on each of them, after that, once we have the catalog mock, we can do better assertions against the real tiles.

Copy link
Member

Choose a reason for hiding this comment

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

The structure could be:

  describe('getCompatibleComponents')
    it(`scenario for from`)
    it(`scenario for InsertSpecialChil`)
    it(`remaining scenario`)

note the messages are simply placeholder, we would need a description that explains the test case appropriately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get it. I'll try to add those tests in some time!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lordrip Do you mean something like this ?
image

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's a good start, but remember to provide a descriptive name for the it.

  • it('should not provide isProducerOnly components')
  • it('should provide components')
  • it('should provide processors')
  • it('should provide source kamelets')

Copy link
Member

Choose a reason for hiding this comment

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

Then the assertion would be to receive a function out of the getCompatibleComponents method. Later on, when we provide a catalog mock, we'll change those assertions to something more specific, by testing which components we get in response.

@shivamG640
Copy link
Contributor Author

Hi @lordrip ,
I wrote a few tests as per my understanding of the code. Let me if this is something you were looking for or we can discuss changing this!

@shivamG640
Copy link
Contributor Author

Hi @lordrip ,
Thanks for your comments, I added those changes.
I'll merge it once you review it again!

Copy link
Member

@lordrip lordrip left a comment

Choose a reason for hiding this comment

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

Thanks @shivamG640 , looks good 👍

@lordrip lordrip merged commit 8e304aa into KaotoIO:main Nov 22, 2023
10 checks passed
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.

Kamelets in Camel Routes are not handled properly
3 participants