Skip to content

Include public incidents in the status page last updated timestamp - #488

Open
soymgomez wants to merge 2 commits into
cachethq:mainfrom
soymgomez:status-last-updated-incidents
Open

soymgomez wants to merge 2 commits into
cachethq:mainfrom
soymgomez:status-last-updated-incidents

Conversation

@soymgomez

Copy link
Copy Markdown
Contributor

Summary

  • the status bar shows "Last updated 1 year ago" even after incidents have been opened, updated and resolved in the last few days
  • Status::lastUpdated() only takes the latest updated_at of enabled components. Linking a component to an incident is stored on incident_components and does not touch the component (the effective status is resolved at read time), so the timestamp only moves when a component itself is edited
  • take the most recent of:
    • enabled components (as before)
    • public incidents (viewableBy(false), so hidden, authenticated-only and unpublished incidents are left out)
    • updates on those public incidents. Posting an update only touches the incident when it changes its status
  • add tests for public incidents, their updates, and for excluding non-public and unpublished incidents

Validation

  • the two new "includes" tests fail on main and pass with the fix
  • vendor/bin/pest: 1154 passed (4295 assertions)
  • vendor/bin/pint --test: passed
  • vendor/bin/phpstan analyse src/Status.php: no errors

Status::lastUpdated() only looked at enabled components. Linking a component to an incident is stored on incident_components and does not touch the component, so the status bar kept showing the last time a component was edited ("Last updated 1 year ago") after incidents had been opened, updated and resolved. Take the most recent of enabled components, public incidents and the updates of public incidents.
Larastan with prefer-lowest dependencies types the whereHasMorph closure builder as Builder<Model>, so viewableBy() was reported as undefined. Filter on updateable_type and a subquery of public incident ids instead, as SlackRssController already does.
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.

1 participant