[server] Retention emails #5550
Conversation
…thout families after 30 days of sub
…eturn storage exceeded/exceeding users on the param basis
… of users depending on storage consumed
…fication_history table
…on_history on Subscription Update
server/pkg/repo/user.go
Outdated
| rows, err := repo.DB.Query(` | ||
| SELECT users.user_id, users.encrypted_email, users.email_decryption_nonce, users.email_hash, usage.storage_consumed, subscriptions.storage | ||
| FROM users | ||
| INNER JOIN usage | ||
| ON users.user_id = usage.user_id | ||
| INNER JOIN subscriptions | ||
| ON users.user_id = subscriptions.user_id AND usage.storage_consumed > subscriptions.storage AND users.encrypted_email IS NOT NULL AND users.family_admin_id IS NULL; | ||
| `) | ||
| ON users.user_id = subscriptions.user_id AND usage.storage_consumed >= (subscriptions.storage * $1 / 100.0) AND users.encrypted_email is not null AND users.family_admin_id is NULL; |
There was a problem hiding this comment.
* $1 / 100.0 Is this intentional?
There was a problem hiding this comment.
Yes, this logic is intentional and it filters users based on the storage they have consumed. The queries were ran manually to test if they work while I was writing the code. Here's a proof with 90% and 100% consumptions.
then the code below this does further logic. Please let me know if there are any inconsistencies you think are still there.
Also, thank you for raising the question. There was a small bug (which could've had bigger impact) regarding the comparison usage.storage_consumed >= (subscription.storage * 90 /100). The >= operator was the bug here leading to get both 90% and 100% consumed list of users when percentageThreshold == 90
There was a problem hiding this comment.
Understood.
I don't think equal will work as user storage might never reach exact 90%. Even if it reaches, the cron runs at fixed interval, it would increase after that.
There was a problem hiding this comment.
I was re-iterating over this once again. There's a catch
if we use just usage.storage_consumed = (subscription.storage * 90 /100), your argument is valid.
but if we use >=, the users between 90% - 100% consumption range will be considered for receiving an email.
One way I can think of is, handling both the thresholds separately.
mngshm
force-pushed
the
retention-emails
branch
2 times, most recently
from
665160f to
e7b5815
Compare
previously all users without families were fetched and later 30 days check was being done. With this modification, in the future custom duration can be passed and depending on that one could get the list of users. The compiler decides if more time has passed dependeing on what is the underlying nanoseconds value of time.Time value. Older dates are less in size compared to Newer Dates. Hence, s.created_at >= is an appropriate calculation
mngshm
force-pushed
the
retention-emails
branch
from
ce37e9d to
f609cef
Compare
| @@ -128,6 +129,38 @@ func (repo *UserRepository) GetAll(sinceTime int64, tillTime int64) ([]ente.User | |||
| return users, nil | |||
| } | |||
|
|
|||
| // GetSubscribedUsers will return the list of all subscribed. | |||
| func (repo *UserRepository) GetSubscribedUsersWithoutFamily(duration string) ([]ente.User, error) { | |||
There was a problem hiding this comment.
So, the created_at does not get updated when user switches from free to paid plan.
func (repo *UserRepository) GetSubscribedUsersWithoutFamily(accountAgeInDays int) ([]int64, error) {
rows, err := repo.DB.Query(`SELECT u.user_id, u.encrypted_email, u.email_decryption_nonce, u.email_hash, s.created_at
FROM subscriptions s
INNER JOIN users u ON s.user_id = u.user_id
WHERE u.creation_time <= $1
AND u.family_admin_id IS NULL
AND u.encrypted_email IS NOT NULL
AND s.product_id != 'free'`, time.MicrosecondBeforeDays(accountAgeInDays))
...There was a problem hiding this comment.
In this query, the condition happens between u.creation_time <= $1. This is not what was intended. The user might chose to not subscribe and consume the free 10GB storage first. Hence, I went ahead with using s.created_at. Ideally, the mail has to be sent to those who have crossed 30 days after subscribing and still do not have a family onboarded.
| WHERE s.created_at >= $1 | ||
| AND u.family_admin_id IS NULL |
There was a problem hiding this comment.
s.created_at >= $1 should be s.created_at <= $1 (fixed it in the suggested query) and also added check for deleted users.
mngshm
force-pushed
the
retention-emails
branch
from
0e4db1c to
5d5e418
Compare
No description provided.