Skip to content

[#215] Resolve "admins can undo recycle"

aguestuser requested to merge 215-admins-can-undo-recycle into main

Closes #215 (closed)

Context

  • since twilio numbers cost money ($1/mo/number) we periodically "recycle" them by deleting the channel info associated with a number and throwing the number back in the pile of inactive numbers able to be used for new channels
  • we try to only recycle phone numbers for channels that people haven't used in a while. however, we might accidentally remove a channel that people still want to use. that would be suuuper sad!
  • this MR implements behavior to give people a warning and a 24 hour "grace period" before recycling the phone nubmer for their channel
  • it is particularly urgent to play now, since we want to recycle phone numbers for some fairly big channels before implementing channel sharding (so that we don't allocate entire containers to very large channels that nobody uses anymore!)

Behavior

WHEN a maintainer issues a recycle command for a channel

  • THEN the admins will receive a notification that says Your channel is about to be deactivated due to lack of use. To continue using the channel, respond with any message in the next 24 hours.
  • IF admins respond within 24 hours, THEN their channel will not be recycled
  • IF admins do not respond withing 24 hours, THEN their channel will be recycled (and they will receive notification that says Your channel is being deactivated due to lack of use. To create a new channel, visit https://signalboost.info

Changes

data model:

  • add a recycleRequest table and model (that stores a phone number and when it was enqueued for recycling)
  • when a sysadmin uses the cli to POST to phoneNumbers/recycle, now, isntead of hitting phoneNumberRegistrar.recycle (which would actually recycle the number), instead call recycleRequestRepository.requestToRecycle to create a new recycleRequest record

processing requests:

  • add a job that runs every 24 hours and calls phoneNumberRegistrar.processRecycleRequests, which in turn delegates to phoneNumberRepository.evaluateRecycleRequests
  • evalutaeRecycleRequests:
    • hits the DB to determine which phone numbers have been redeemed by their admins (by being used during the grace period) and which ones habe not been used and thus should be recycled.
    • leaves as pending any requests for which the grace period has not yet expired, regardless of whether the channels have been used or not
    • returns a redeemed / toRecycle tuple back to processRecycleRequests
  • processRecycleRequests takes this information and acts on it by:
    • recycling the unredeemed phone numbers (including notifying the channel members of the deactivation)
    • deleting all the matured recycle requests
    • notifying members of any redeemed phone numbers that their channels have not been deactivated
    • notifying maintainers (who issued the original recycle request a day ago) of the results of their requests (ie: how many phone numbers were redeemed and recycled and if eany errors happened)

tech debt / side effects:

  • add add util.repeateUntilCancelled, use it to make all jobs cancellable (and cancel them in jobs.stop())

    • motivation: this prevents non-deterministic unti test failures by stopping functinos from being called over and over again in the test for the jobs module and thereby interfering with the unit tests for those functions elsewhere
    • util.repeateUntilCancelled:
      • calls an async function repeatedly on an interval
      • returns a function that, if invoked, cancels the next call of the async function
  • fix localization bug in phoneNumberRegistrar.destroy

    • was sending destruction notification in default lang (EN) to all recipients
    • should send notification in reach recipient's lang
    • to fix, provide new impl of notifyMembersExcept
  • extract a notifier module with helpers for notifying members, admins, and channels

  • refactor shared db destroy logic in recycle/destroy

    • pull destroy logic into repository layer
    • leverage this to provide cleaner (less coupled) stubs in unit tests for the registrar layer
    • add unit tests to ensure that destroy deletes associated models of channels
  • extract all recurring job scheduling logic from the repository layer to the jobs module

Edited by aguestuser

Merge request reports

Loading