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

Use backend report status directly for "readiness" and "activation" #1430

Open
wants to merge 6 commits into
base: feature/add-new-AA-storm
Choose a base branch
from

Conversation

Doniaab
Copy link
Collaborator

@Doniaab Doniaab commented Feb 5, 2025

Description

This PR is a change to use the report status to display readiness in the left panel and activated districts.
What this means is that if the status is ready, even though some districts are listed as activated, we don't show them as active since the landfall lead time is above 72 hours.

In the future we might display activated regions in that case, when they will be available from the backend.

image

How to test the feature:

  • You can go to this url and select 12pm at this date on the timeline : https://prism-1430.surge.sh/?hazardLayerIds=anticipatory_action_storm&date=2025-02-03
  • The report at this date and time is in status "ready" and one watched district "Maganja Da Costa" is in the list exposed_area_48kt: We shouldn't see this district in the activated list on the left panel because the status is "ready".
  • We should display readiness on the left panel.

Checklist - did you ...

Test your changes with

  • REACT_APP_COUNTRY=rbd yarn start
  • REACT_APP_COUNTRY=cambodia yarn start
  • REACT_APP_COUNTRY=mozambique yarn start
  • Add / update necessary tests?
  • Add / update outdated documentation?

Screenshot/video of feature:

Copy link

github-actions bot commented Feb 5, 2025

Build succeeded and deployed at https://prism-1430.surge.sh
(hash 07ebfdb deployed at 2025-02-06T16:39:46)

const category = AACategoryKeyToCategoryMap[categoryKey];

// If the storm status is not active, all watched districts should be marked as inactive
if (!isActivated) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this check necessary? won't the sets be empty anyway further down?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this check is necessary because if isActivated is false, we need all watched districts to be explicitly added to naDistricts. Otherwise, they wouldn't be included anywhere.

Also we can be in ready status and have watched districts in the affected districts list. In this case the status already take in account the 'landfall leaditme > 72 hours' and we dont have to put those district in the activated list.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a doubt about whether we should display districts in the NA>XXX boxes in the leftPanel when the report status is monitoring or ready because on figma, there is no district displayed when the status is ready / monitoring
image

Since, the spec have changed a bit in the meantime (and the figma template did not follow on changes), it could well be that the current implementation is as expected (ie display NAxxx boxes when status is monitoring/ready)

@ericboucher
Copy link
Collaborator

Ok, is this ready for review by @wadhwamatic ?

@ericboucher ericboucher marked this pull request as ready for review February 6, 2025 17:21
@ericboucher ericboucher changed the title Use report status for readiness and activated Use backend report status directly for "readiness" and "activation" Feb 6, 2025
];
}
return [activeResult, naResult];
// Determine if watched districts are active based on storm status
Copy link
Collaborator

@Max-Z80 Max-Z80 Feb 7, 2025

Choose a reason for hiding this comment

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

It would be nice to add a test on parseAndTransformAA()

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.

3 participants