Fix.pipeline testing changes

[jannisvisser]

Describe your changes

Changes related to testing of refactored floods pipeline.

Checklist before requesting a review

• I have performed a self-review of my code
• I have added tests wherever relevant
• I have made sure that all automated checks pass before requesting a review

Notes for the reviewer

1. Any helpful instructions or clarifications...

[gulfaraz]

Looks good overall. I strongly prefer to remove the flood-specific logic in body-event.ts. I've added details in the review comment. Let me know if it's unclear or needs discussion.

[gulfaraz]

Let's change this logic to apply across hazards instead of a flood-specific scenario.

if (firstTriggerLeadTimeFromNow) {
  if (firstLeadTimeString !== firstTriggerLeadTimeString) {
    contentContent.push(
      `<strong>${disasterTypeLabel}:</strong> Expected to start on ${firstLeadTimeString}, ${firstLeadTimeFromNow}.`,
    );
  }
  contentContent.push(
    `<strong>${disasterIssuedLabel}:</strong> Expected to trigger on ${firstTriggerLeadTimeString}, ${firstTriggerLeadTimeFromNow}.`,
  );

I interpret the above code is for warning-to-trigger and trigger scenarios.

• Expected to start on implies it will flood.

• Expected to start on implies the flood will reach the trigger threshold.

  contentContent.push(
    `<strong>${disasterIssuedLabel}:</strong> Expected on ${firstLeadTimeString}, ${firstLeadTimeFromNow}.`,
  );
  

The above is for warning scenario.

• Expected on is not clear.
• This should be changed to Expected to start on to be consistent.

With this change the above scenario can be combined with,

contentContent.push(
  `<strong>${disasterTypeLabel}:</strong> Expected to start on ${firstLeadTimeString}, ${firstLeadTimeFromNow}.`,
);

So we remove the need for,

if (disasterType === DisasterType.Floods) {

Does this make sense? Am I missing anything to consider?

[jannisvisser]

@gulfaraz I agree with the preference. What I did in this commit is simply align the (incorrect) logic with the correct logic in the chat section of the portal, which unfortunately also uses this exception on floods.

That said, I'm happy to look at what we can do, but your above suggestion is missing one thing. Namely, the bottom 2 cases do not only differ on the phrase 'to start' but also on 'disasterIssuedLabel' vs ' disasterTypeLabel'.

I will take another look to see what's possible.

[gulfaraz]

You're right, I missed the disasterIssuedLabel vs disasterTypeLabel difference.

It would be great if the check is not specific to a hazard or country.

[jannisvisser]

@gulfaraz see new commit. I made use of the fact that disasterIssuedLabel is based on eapAlertLabel, which is only filled for floods. If not filled I default to disasterTypeLabel, which allows for the combining of above scenarios. The previous fallback option was never used in practice anyway. I also added some code comments, as I think that doesn't hurt for this relatively complex logic. Ok?