User score to determine which follower email to send #5773
Conversation
| @@ -10,6 +10,8 @@ def perform(article_id) | |||
| comment_score: article.comments.sum(:score), | |||
| hotness_score: BlackBox.article_hotness_score(article), | |||
| spaminess_rating: BlackBox.calculate_spaminess(article)) | |||
|
|
|||
| article.user&.calculate_score | |||
There was a problem hiding this comment.
It feels a little off that Articles::ScoreCalcJob updating a User's score.
There was a problem hiding this comment.
Yeah, that makes sense.
Since this should logically be performed after the article's score is calculated, it does make sense that they should occur in a synchronous context.
It could be renamed ScoreCalcJob could be renamed to reflect that its job is to calculate all scores related to itself and anything affected by its score, or we create another job sequence when it finishes?
Definitely don't want to over-engineer this.
There was a problem hiding this comment.
I can live with this for now but I agree it does feel a little off.
There was a problem hiding this comment.
I think removing the namespace and just calling it ScoreCalcJob is a nice middle ground in making it feel less off without being overengineered.
benhalpern
changed the title
| @@ -223,7 +223,7 @@ def set_remember_fields | |||
| end | |||
|
|
|||
| def calculate_score | |||
| score = (articles.where(featured: true).size * 100) + comments.sum(:score) | |||
| score = articles.sum(:score) + comments.sum(:score) | |||
There was a problem hiding this comment.
What is the plan for backfilling these to ensure that the scores all get updated and are consistent?
| @@ -10,6 +10,8 @@ def perform(article_id) | |||
| comment_score: article.comments.sum(:score), | |||
| hotness_score: BlackBox.article_hotness_score(article), | |||
| spaminess_rating: BlackBox.calculate_spaminess(article)) | |||
|
|
|||
| article.user&.calculate_score | |||
There was a problem hiding this comment.
I can live with this for now but I agree it does feel a little off.
| @@ -10,6 +10,8 @@ def perform(article_id) | |||
| comment_score: article.comments.sum(:score), | |||
| hotness_score: BlackBox.article_hotness_score(article), | |||
| spaminess_rating: BlackBox.calculate_spaminess(article)) | |||
|
|
|||
| article.user&.calculate_score | |||
There was a problem hiding this comment.
I think removing the namespace and just calling it ScoreCalcJob is a nice middle ground in making it feel less off without being overengineered.
| @@ -7,6 +7,8 @@ def perform(follow_id, mailer = NotifyMailer.name) | |||
| follow = Follow.find_by(id: follow_id, followable_type: "User") | |||
| return unless follow&.followable.present? && follow.followable.receives_follower_email_notifications? | |||
|
|
|||
| return if follow.follower.score < 25 # Restrict new follower emails to more active/established accounts | |||
There was a problem hiding this comment.
Instead of having a hard-coded value, do we maybe want to add this to SiteConfig?
|
Since this has not been active for a while and has a few conflicts I am going to close. This is probably a feature that we want to propose to @thepracticaldev/delightful! Feel free to fix the conflicts and specs and reopen! |
What type of PR is this? (check all applicable)
Description
We currently have the logic to check about when someone last received a new follower email so we don't send these too often if someone receives a lot of followers through onboarding.
However, people will generally get a lot of followers from onboarding who don't have any activities, and therefore following back is not super likely and the delight of receiving a new follower is watered down.
This changes the logic so that it only sends the email for "established users"
Established in this context means
scoreof at least 25.We have the user
scorecolumn which actually wasn't currently being recalculated, and essentially was not used at all. This changes "score" to be the simple combination of article and comment scores and adds a re-calculation alongside those score calcs.Having
scoreavailable will help fight spam and stuff like that. It's a simple sum we can do when we need it, but this should make it more efficient. We already have the column but are not using it, so this seems fairly risk-free.After deployment instructions
Since we didn't actually call
calculate_scorefrom the app itself, I think we'd only run it manually once or twice in the past, there is little data in this column at the moment, so it wouldn't be too bad to just let this fill up based on activity.That being said, I think we may as well run the console command of
User.find_each(&:calculate_score)to get everything up to date with the new stuff.