WSDOT / wsdot-ios-app

Source code for the WSDOT iOS application
GNU General Public License v3.0
10 stars 4 forks source link

App is not displaying alerts until actual start time has passed #200

Closed waynedyck closed 4 years ago

waynedyck commented 4 years ago

From what I can tell the Android app is simply displaying any alerts which are in the HighwayAlerts.js data file. I thought at one time there was a check to see if the alert was occurring on today's date and, if it was, to display it. That way we weren't showing alerts which had scheduled start dates two weeks in the future (e.g. future scheduled Mariners or Sounders games).

The iOS app is only checking if the current time has passed the scheduled alert start time,

https://github.com/WSDOT/wsdot-ios-app/blob/ce8d58a7af2740ba40edd82ae8230720b9629f91/wsdot/TrafficMapViewController.swift#L317

which means the alert is only showing in the app sometimes 30 minutes before the construction or closure event beings.

This should be adjusted to display any alerts occurring on the same day as the alert start date.

loganSims commented 4 years ago

For the sake of documentation, I want to compile some key things about our alert filtering logic into this comment. It can get a little confusing since we have both apps using their code to filter or ignore alerts, along with the highway alerts data script.

highway_alerts.py uses the following logic to determine which alerts to add to the data file:

  1. If we have an end time for the alert check the following:
    • If the difference between alert start time and now is less than 1 day and now is before end time add to the data file
    • If the difference between alert start time and now is less than 1 day and the start time is equal to the end time add to the data file (This accounts for a common user error where start and end times are set to the same time)
    • If the alert has an update time and if the update time is after the end time, assume still active and add to the data file
  2. Otherwise, if the difference between alert start time and now is less than 1 day add to the data file.

In any other case, the alert will not be added to the data file. This includes an alert with a start time further out than 1 day.

I confirmed that the Android app will display any alert present in the HighwayAlerts.js file. I will update the iOS app to do the same. This will make it so the HighwayAlerts.js file has the ultimate say in which alerts are considered active and removes that responsibility from the apps.

Once the iOS update is pushed out we will be able to safely say "If it's in the data file, it's in the apps."

loganSims commented 4 years ago

Just as an added note, start times are do not always line up with the start of the event mentioned in the alert. The following alert describes a closure on September 17th. The alerts start time is listed as September 3rd at 3:30 PM.

Screen Shot 2020-09-14 at 12 11 45 PM
waynedyck commented 4 years ago

I forgot the logic for displaying alerts was in the highway_alerts.py script. That's where I must have remembered seeing it. I think the, "If it's in the data file, it's in the apps" concept is perfect. Thanks!

loganSims commented 4 years ago

Couple more thing to add...

  1. The Android app has no end time checks while the iOS app does. In other words if an alerts end time has passed, regardless of an update from the data file, iOS will hide the alert. I will remove this check in iOS to better line up with the Android app and data file.

  2. The iOS app has another filter for alerts with a latitude and longitude of zero. The iOS app currently hides these alerts from the traffic map. Alerts with those coordinates are typically used as state wide notices so they really don't have a specific location and still need to be shown in the highest impact alerts ticker. For that reason, I plan on keeping that filter in iOS and also want to add it into Android. So while we can still technically say the data file is the single source of truth, there will be a small disclaimer that alerts with (0,0) and priority lower than "Highest Impact" will not be shown in the app.

waynedyck commented 4 years ago

Hiding the (0,0) alerts for sure, otherwise those alerts show up near Africa. We show all levels of alerts in the apps now don't we (e.g. Low, Moderate, High, etc.)?

loganSims commented 4 years ago

Correct. With the proposed changes the only alerts that could slip through the cracks are ones with (0,0) that have a priority lower than highest. The reason being alerts with (0,0) that have highest priority get shown in the alerts ticker, but not on the map.