Pirate-Weather / pirateweather

Code and documentation for the Pirate Weather API
Apache License 2.0
670 stars 29 forks source link

Alert expires time can come before the alert start time. #353

Closed mbomb007 closed 4 weeks ago

mbomb007 commented 1 month ago

Describe the bug

This is likely an upstream problem with the NWS, but I have an alert where the start time comes after the expiration time.

Expected behavior

An alert's start time comes before the time it expires.

Actual behavior

The API is using the <onset> property for the start time, rather than the <effective> property, which is probably what it should use.

This is the data from the NWS Alert .cap file:

        <effective>2024-10-16T02:46:00-05:00</effective>
        <onset>2024-10-17T11:00:00-05:00</onset>
        <expires>2024-10-16T13:00:00-05:00</expires>
        <headline>Fire Weather Watch issued October 16 at 2:46AM CDT until October 17 at 7:00PM CDT [...]</headline>

API Endpoint

Production

Other details

https://api.weather.gov/alerts/urn:oid:2.49.0.1.840.0.909ae5ab6113a0467ac3b38120588e2c04187ad6.001.3.cap

Troubleshooting steps

cloneofghosts commented 1 month ago

It might be a NWS issue but will ping @alexander0042 to look into this and get back to you.

I took a look at the API for the location you edited out and I can see the issue and is likely what you mentioned so should be an easy fix. The regions still have a leading space and needs to be fixed which should just be the case of adding .strip() to the line where the region list is being split.

cloneofghosts commented 1 month ago

Looks like this one got missed from last week. Will try to ping @alexander0042 again to see if he can look into this.

When testing this issue I noticed that the alert description has no line breaks. When the description is shown on a page everything is in one text block which can be hard to read through.

mbomb007 commented 1 month ago

Yes, I just added line breaks with

      // Put in line breaks for readability.
      $description = str_replace(
        ['* ', ' ...'],
        ['<br><br>* ', '<br><br> ...'],
        $description
      );
      // Remove leading breaks that occur if there is no leading text.
      $description = preg_replace('/^(?:<br>)+/', '', $description);

But line breaks would be helpful.

alexander0042 commented 1 month ago

Sorry that I missed this- you're spot on about the onset time, I just grabbed the wrong variable there, so that's been corrected.

The line breaks were actually an intentional decision, Dark Sky didn't return line breaks (See: Issue 38 from the HA repo, so I wanted to keep it consistent with that. However, I see why it's a pain to not have them, and agree it would be better if they were there! You should see them live shortly, and I'll keep an eye out to see if it breaks anything