Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Birthday notifications #14

Open
7 tasks
Phanabani opened this issue Nov 11, 2020 · 8 comments
Open
7 tasks

Birthday notifications #14

Phanabani opened this issue Nov 11, 2020 · 8 comments
Assignees
Labels
major feature A big new feature, probably the focus of a major version bump

Comments

@Phanabani
Copy link
Owner

Phanabani commented Nov 11, 2020

  • Guild settings database table
    • Birthday announcements channel
  • Role to allow changing guild settings? Just check admin perms
  • User settings table
    • Enable/disable sending their birthday notification
    • Setting to display age in notif? Maybe this could be controlled by age privacy
  • Use user's timezone to decide when to send message (default to UTC)
@Phanabani Phanabani added the enhancement New feature or request label Nov 11, 2020
@Phanabani
Copy link
Owner Author

Phanabani commented May 26, 2021

Technical stuff:

  • On connect (?), schedule a task to announce a user's birthday
    • We could schedule the tasks all at once, but I'm not sure if there's a memory/cpu cost there
    • Probably better to schedule a daily task to query upcoming birthdays
  • Need a database table (or new column?) to track when birthday notif has been sent
    • This is necessary in case Sandpiper disconnects across a birthday boundary, or turns on in the middle of the day
    • Could either reset the next day or at new years
  • If she misses a birthday by 24 hours, she should just ignore it rather than announcing it late
  • How to handle users setting their birthday on the day of / <24 hours beforehand?
  • Multiple announcements in one day
    • Announce both together at the earliest one? No, I don't want this because it might be confusing. Consider a Korean dude's birthday on the same date as an American, the Korean dude will have his birthday start like 12 hours before the American. Just respect the birthdayer's timezone!

@Phanabani
Copy link
Owner Author

discordpy's tasks run at set intervals. I want them to run at the same time every day (midnight UTC), so upon startup, we can run our daily task once, then schedule the next task run in (midnight - now) hours.

Actually, does it really matter what time the task runs? As long as we ensure a 24 hour interval, we should be able to cover all birthdays.

@Phanabani
Copy link
Owner Author

Phanabani commented May 27, 2021

Our daily task should query the database for all birthdays within the next 24 hours. This should be a sorted list. We can then await discord.utils.sleep_until each of them in order. I don't trust how this function uses datetime.now, so I'll implement it myself using my utc_now function.

@Phanabani
Copy link
Owner Author

Phanabani commented May 27, 2021

I was considering storing the birthdays as a datetime converted to UTC in the database, but I think that's both inefficient in terms of space and added complexity because it would have to be recalculated whenever either the birthday date changes OR the timezone changes.

With this on-the-fly calculation of localized birthday datetimes, we just need to select dates which are either the current date or the next day (e.g. South Korea at 23:00 UTC is the next day), then we can convert their localized midnight to UTC and determine if they're within the next 24 hours. We don't need to check the previous day (e.g. Americas being several hours behind UTC) because we will have hit it already the previous day.

So, in summary:

  1. Select birthday, timezone where birthday is either today's date (UTC localized) or tomorrow
  2. Localize midnight to each timezone, then convert to UTC and check that it's <= 24 hours from now
  3. Return this list of midnight birthday datetimes (UTC localized), sorted in ascending order

Edit: I intended on doing this entire procedure in the database adapter, but I think it should only expose a get_birthdays interface which does no post-processing and returns the data for a given day. We can handle the processing elsewhere.

@Phanabani
Copy link
Owner Author

Don't announce birthdays of users not in the guild anymore. This ties in to #39. Under normal circumstances, you'd expect the user to be deleted from the table, but we're considering allowing them to leave their data in Sandpiper in case they left the server temporarily for some reason.

@Phanabani
Copy link
Owner Author

Enable/disable sending their birthday notification
Setting to display age in notif? Maybe this could be controlled by age privacy

I'm reconsidering these two issues. I think it's most logical to have the privacy settings control what data gets accessed by the birthday alerts module. For example:

  • Private birthdays won't be announced
  • Private ages won't be put in the birthday message
  • Private/missing preferred names will be replaced by regular usernames in the birthday message

However, is it intuitive that having your age as public means it will be announced? It doesn't seem so, and I think many people might not enjoy having their age displayed like this. The solution I was already thinking about is to add a NEW setting for displaying age, but that seems redundant/extraneous along with the privacy.

We could instead give more explicit hints to the user about what's going on when they set their data. For example, here's a diagram of possible a command flow:

  1. User sets birthday
    1. Birthday privacy == private
      1. "I can announce when it's your birthday if you set your birthday privacy to public"
    2. Birthday privacy == public
      1. "I will announce to your servers when it's your birthday"
      2. Age privacy == private
        1. "I will not show your new age in your birthday announcement"
      3. Age privacy == public
        1. "I will show your new age in your birthday announcement"
  2. User sets birthday privacy
    1. Birthday privacy == private
      1. "I will not announce your birthday. You can change this by setting birthday to public."
    2. Birthday privacy == public
      1. Same as 1.ii.
  3. User sets age privacy
    1. Birthday privacy == private
    2. Birthday privacy == public
      1. Age privacy == private
        1. Same as 1.ii.a.
      2. Age privacy == public
        1. Same as 1.ii.b.

I think this is a state machine. It probably doesn't need to be more complicated than a few nested conditionals but maybe look into that.

@Phanabani
Copy link
Owner Author

Define custom birthday messages in config!

@Phanabani
Copy link
Owner Author

Phanabani commented May 28, 2021

Handle user adding their birthday or editing birthday privacy within 24 hours of it starting.

@Phanabani Phanabani self-assigned this Jun 2, 2021
@Phanabani Phanabani added major feature A big new feature, probably the focus of a major version bump and removed enhancement New feature or request labels Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major feature A big new feature, probably the focus of a major version bump
Projects
Development

No branches or pull requests

1 participant