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

tweak: Suppress Alewife platform closure alert. #2095

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

cmaddox5
Copy link
Contributor

Asana task: ad-hoc

The Alewife platform is being bypassed this weekend. The alert was created using a new Alerts UI feature that the Pre-Fare screens cannot handle yet. We will use a static image in its place.

Note: the scenario we are testing here is suppressing the above alert and letting another shuttle alert render normally. There is a flex zone alert bug keeping this branch from displaying the alert properly. That is being resolved in #2094.

  • Tests added?

@cmaddox5 cmaddox5 requested a review from a team as a code owner June 27, 2024 15:42
Copy link

Coverage of commit a74ae44

Summary coverage rate:
  lines......: 45.0% (2911 of 6476 lines)
  functions..: 41.0% (1038 of 2531 functions)
  branches...: no data found

Files changed coverage rate:
                                                                          |Lines       |Functions  |Branches    
  Filename                                                                |Rate     Num|Rate    Num|Rate     Num
  ==============================================================================================================
  lib/screens/v2/widget_instance/reconstructed_alert.ex                   |86.8%    334| 100%    44|    -      0

Download coverage report

t.location_context.home_stop === "place-gover"

not suppressed
t.alert.id not in ["197140", "580015"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did these IDs come from? When I try to look up either of them in the API, I get a 404 error. Looking at the alerts currently in the feed, it seems like these should be 571855 and 578253.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

197140 is the alert ID that Dave created in test. I can put the values in variables so that they are more obviously labeled. The other ID is in prod so I'm not sure why the API isn't returning it. I found it on the Future tab in AlertsUI. Based on the discussion in this thread, the alert that will be used for the closure will be a station closure instead of a service change. That may be why 571855 starts and ends at the same time. They are only using it in notifications but will switch to 580015 when the alert is in effect. I will triple check with Dave.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, okay, I think I see what they're doing there... it doesn't really make sense to me (why two alerts? how is the second one being hidden from the feed?), but the ID is at least correct to what they've told us.

Incidentally, it looks like 571855 is now active currently, until tonight. Maybe we should add that one here in addition to 580015?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because 571855 is a Service Change, it won't show on these screens anyway.

Copy link

Coverage of commit 2ca1d2e

Summary coverage rate:
  lines......: 44.9% (2914 of 6483 lines)
  functions..: 41.0% (1038 of 2532 functions)
  branches...: no data found

Files changed coverage rate:
                                                                          |Lines       |Functions  |Branches    
  Filename                                                                |Rate     Num|Rate    Num|Rate     Num
  ==============================================================================================================
  lib/screens/v2/widget_instance/reconstructed_alert.ex                   |86.9%    336| 100%    44|    -      0

Download coverage report

@cmaddox5 cmaddox5 merged commit dac069f into main Jun 28, 2024
11 checks passed
@cmaddox5 cmaddox5 deleted the cm/suppress-alewife-platform-closure branch June 28, 2024 15:34
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.

2 participants