Dashboard

OLMIS-6005: Removed all tests except one

OLMIS-6029: Added CONSOLIDATE_NOTIFICATIONS feature flag

OLMIS-6005: Added next test

OLMIS-6005: Added next tests

OLMIS-6005: Added first two tests

OLMIS-6005: Temporarily removed tests

OLMIS-6005: Temporarily removed the last two tests

OLMIS-6005: Removed tests and re-added another one

OLMIS-6005: Removed the test to investigate the issue

OLMIS-6005: Removed some tests

OLMIS-6031: added performance tests for supply lines
OLMIS-6031: added performance tests for supply lines
OLMIS-6031: added performance tests for supply lines

    • -0
    • +82
    /performance/tests/supplyLines.yml
OLMIS-6005: Re-added one of the removed tests

OLMIS-6005: Merge

OLMIS-6005: Reverted removing first two tests and removed next two

OLMIS-6005: Removed first two tests

1. The review is quite big because for all new classes I had to add tests to avoid sonar issues. But the main changes are in the service package when a bunch of new classes has been created based o...

1. The review is quite big because for all new classes I had to add tests to avoid sonar issues. But the main changes are in the service package when a bunch of new classes has been created based on the diagram in the ticket. Also because of comments from the previous versions of the review I had to provide additional changes.
2. Each component of spring integration has two channels: input and output. Only start and end of flow has one channel: output and input, respectively. Everything starts in NotificationToSendRetriever and ends in EmailNotificationChannelHandler. As I said before it is easier to see what happens when you open the diagram from the ticket.

In the end, I am not sure if there is an easy way to remove unnecessary changes. If you still have an issue to understand changes you could try to do this together with the diagram from the ticket.

done

done

changed

changed

changed

changed

OLMIS-5971: fixed broken test

OLMIS-5971: Update test for NotificationTransformer (FEOLMIS-3638)

OLMIS-5971: Updated log level and message (FEOLMIS-3638)

Yes, currently this class does nothing but we change that in OLMIS-6029

Yes, currently this class does nothing but we change that in OLMIS-6029

Łukasz Lewczyński, thanks for sending this. I have two main comments: 1. The size of this review is too large and unfocused. It's difficult to grasp what is supposed to be evident and what's a WI...

Łukasz Lewczyński, thanks for sending this.

I have two main comments:

1. The size of this review is too large and unfocused. It's difficult to grasp what is supposed to be evident and what's a WIP. Next time try much smaller reviews chained together that build upon one another. Less than a dozen files, no more than 1k LOC.

2. It's not really clear what's happening in the service layer with regards to what should be a filter, what's a router, and where we'd see an aggregator. This is likely a continuation of the above.

Could you clean this up?

Is this class a placeholder and a WIP? I'm confused how this filter supports a digest operation? Wouldn't we want an aggregator over a filter anyhow?

Is this class a placeholder and a WIP? I'm confused how this filter supports a digest operation? Wouldn't we want an aggregator over a filter anyhow?

I am curious about the decision for some log statements to be errors? Some of them seem like warnings to me. Like in NotificationTransformer or NotificationChannelRouter.

I am curious about the decision for some log statements to be errors? Some of them seem like warnings to me. Like in NotificationTransformer or NotificationChannelRouter.