Fix manual lock grace period bug

[RobertJoonas]

Changes

The bug is caused by Plausible.GracePeriod.active?/1 unconditionally returning true for any manual_lock grace_period. It should return false when the manual lock grace period has been terminated manually via CS.

The bug is pretty tricky, so I'll do my best explaining what happens under the hood:

• An enterprise customer has not upgraded in time and our customer support decides to hit the "Lock" button on their team
• This action: 1) locks the team instantly and 2) sets their (manual lock) grace_period.is_over = true
• At the next UTC midnight, site_locker.ex runs -- checks every team and ends up calling Plausible.Teams.Billing.check_needs_to_upgrade/2
• ❗️check_needs_to_upgrade ends up returning :no_upgrade_needed because the Teams.GracePeriod.expired?(team) cond branch wrongly returns false
• site_locker.ex unlocks the site but keeps the grace period in place

At this point, we're already in a messed up state because on one hand, the team is unlocked, but on the other, it has a manually terminated grace period. Now we get to rendering usage_notification for the team in the team settings billing section:

• usage_notification_type tries to find the type of the notification to render
• From these three cond branches (see screenshot below), we incorrectly end up in the third one because:
  • the team is not locked (as mentioned above)
  • Teams.GracePeriod.expired?(team) returns false (also mentioned above)
  • manual_lock_active? returns false as well -- this is correct because is_over being true means it's not active
  • Finally, GracePeriod.active? returns true (the root bug)

• the usage_notification component then tries to render "days left" but no case clause matches nil -- a manual_lock grace period has end_date = nil

Expected behaviour:

• The manually locked team stays locked after site_locker runs (unless the usage has decreased enough to unlock)
• usage_notification renders :dashboard_locked

Tests

• Automated tests have been added

Changelog

• This PR does not make a user-facing change

Documentation

• This change does not need a documentation update

Dark mode

• This PR does not change the UI

[zoldar]

Nice work explaining all the context of this tricky bug 👏

[zoldar]

Would it make sense to move that clause to the top, so it always takes precedence over anything else? 🤔

[RobertJoonas]

Yes, I think it would. Although it wouldn't make a functional difference (because end date being a %Date{} and manual_lock being true are be mutually exclusive), a manual lock grace period getting an end date by mistake sounds more probable than a grace period randomly becoming manual_lock: true. 84b9943