Emergency alerts not sorting properly

[danfox01]

In some cases when multiple alerts are active, they seem to be sorting by the date they were published instead of in order based on alert type.

[martypowell]

@danfox01 can you please set this up for @tmccoy529 and I in dev?

[martypowell]

@danfox01 please assign this to @tmccoy529 when it's recreated in dev.

[danfox01]

Dev is updated with 5 sample alerts -- one for each alert type. However, I think the dev site needs to be pointed to testservices.bcpl.info in order to see them.

Expected Behavior:

Alerts should always sort in order based on their alert type:

• Emergency/Weather: Systemwide
• Emergency/Weather: Branch
• Holiday Closing
• Website Downtime
• Long-Term Branch Closure

If two alerts of the same type are active, they should be sorted by their start date as a secondary dimension.

[danfox01]

@tmccoy529 and @martypowell -- let's hold off making changes to this for the moment.

We heard from BCPL today that they would prefer a simpler system of setting the sort order by number., rather than selecting by type. We intentionally avoided that for a few logistical reasons, but I want to reevaluate.

We can drop the ticket to sev. 3 and I'll post some final requirements here. I still want to do this soon, but hoping we can avoid double work if other changes are needed.

[danfox01]

OK, scratch that last comment about changing any functionality. Let's just focus on fixing the sort order for these. @tmccoy529 when do you think you'll have time to dig into this one? Let me know and I'll update the ticket.

[martypowell]

@tmccoy529 I think we want to do this in the service, which is in tfs. I'm guessing the docs on the project probably could be updated, to help someone get working on that, so lets try and do that as part of this too.

[tmccoy529]

@martypowell Yeah, I was debating how we should handle this one this morning on my mind numbing drive in. Sanjay and I will jump on this one and see what we can do. Once we are in the right location I cant imagine it will take long.

[tmccoy529]

@danfox01 I am just currently waiting to get access to the test box so I can move the code over for testing purposes. Once that is done I will let you know when its ready to test.

[danfox01]

@tmccoy529, awesome. Thanks, keep me posted.

[danfox01]

@tmccoy529 any luck on getting access to testservices? If not i can help push that along.

[tmccoy529]

@danfox01 this has been moved over to test services. Give it a try and see what you all find.

[martypowell]

@danfox01 just fyi, this is ready for testing.

[danfox01]

Oh snap, thanks. I missed @tmccoy529's mesasge earlier.

We did refresh dev on Tuesday. I can re-add the alerts for testing, but does any code need to be moved back over to SE.

[tmccoy529]

@danfox01 nothing was changed in SE for this particular fix. So it should be good to go still.

[danfox01]

@tmccoy529, I've got three active alerts in dev right now but not seeing them on the site. Can you take a look?

[tmccoy529]

@danfox01 yeah let me take a look. I wonder if the web config I moved over is pointing at staging still.

[tmccoy529]

@danfox01 In the process of investigating the banners not showing up I found a flaw in my code which would have worked most times but not all the time so I'm glad we stalled on this. Anyway that has been fixed. As far as the banners not showing, this is because its pointing at prod not dev for alert data. I've updated this to reflect dev. Happy testing.

[danfox01]

@tmccoy529 this looks great. We can talk about moving to prod in our 2:00 today.

[jdomasky]

@tmccoy529 I tested the alerts in dev this afternoon, per Marty, and the order is wrong. Link: https://dev.bcpl.info/alerts.html

[tmccoy529]

@jdomasky the test box is no longer pointing to that branch. Sanjay and I were working on adding pins to a blog entry. Ive reverted it back to the alerts branch. Can you try again?

Also this is the order of priority I have in the code.

emergency-systemwide - 1 emergency-branch - 2 holiday-closing - 3 website-downtime - 4 long-term-branch-closure - 5

Is this correct or should anything be moved?

[jdomasky]

@tmccoy529 The order you listed is correct. I tried again and the Alerts page is now sorted in the correct order. However, the alert bar is displaying the priority 2 alert at this time. I believe it should be displaying the priority 1 alert.

[tmccoy529]

@jdomasky great catch!!!!

@danfox01 @martypowell the alerts notification banner was not modified in this fix. Looking at the code it appears that we are grabbing all alerts of type emergency, sorting them by start date so most recent is first, then taking the first one in that list to display in the banner. In this case emergency system has a start date of 1-31-2019 and emergency branch is 2-1-2019 so that is why its appearing first.

var displayAlerts = activeAlerts .OrderByDescending(alert => alert.StartDate) .ToList(); if (!displayAlerts.Any()) return activeAlerts.First();

We can however make this display based on priority since we added this nifty little priority field to these alerts now.....just a thought.

[danfox01]

Thanks @tmmcoy529. I didn’t realize that the code was different for the banner vs the alert page. The banner is actually the more important of the two, because it only shows one event at a time and we always want the highest priority to show. That’s actually how we noted the bug in the first place.

The banner tested fine earlier this week but it must have just been coincidental that I didn’t have the right combo of alert types and dates to cause a failure.

I think we can go forward with tomorrow’s change but we’ll need to follow up quickly with a fix for the banner as well. What do you think?

Sorry for the confusion!

[tmccoy529]

@danfox01 I think I can implement a fix fairly quickly for the banner. Would you prefer to hold off and just do a push for both fixes say for Monday?

[danfox01]

Yes that’s totally fine. I want to get @martypowell’s opinion first thing in the morning as well, but that works for me.

[martypowell]

Hey guys for now we will hold off. Tim I'm sure you could make the changes, but postponing will give us some time to document everything and I'd like to talk about adding some unit tests for the service logic. We will shoot for Monday per @danfox01

@tmccoy529

[tmccoy529]

@danfox01 @jdomasky this issue is ready for retest at your earliest convenience.

[danfox01]

@tmccoy529 looks great. Thanks.

[tmccoy529]

@danfox01 This was in the release on Thursday 2/28