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

Add a weekly Newsletter for the latest open Bounties #19 #36

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

clair13
Copy link

@clair13 clair13 commented Mar 16, 2023

Hello, my name is George, I worked on issue #19 and added a weekly newsletter for the latest 15 open bounties.
I used the letter-opener gem and whenever gem for the scheduled emails to all registered users.

Now is scheduled the newsletter every Sunday at 17.00. You can change that on the following file

config/schedule.rb .Also by changing the number on the rake bounty task on the 3rd line Bounty.open.last(15), you can change the number of open Bounties.

I added a mailer test and the test is passing all the assertions.

I would like to thank Chris @excid3 and everyone involved in this project.
Last but not least I would like to thank Cody @cnorm35 who opened this issue and helped me throughout the whole process answering all my questions and helping with issues I had along the way. It was a pleasure the whole process!!!

@excid3
Copy link
Owner

excid3 commented Mar 16, 2023

Excellent work @clair13! Do you happen to have a screenshot of the email?

@clair13
Copy link
Author

clair13 commented Mar 16, 2023

yes one moment

Στιγμιότυπο 2023-03-16, 8 28 36 μμ

From 3 bounties gets the 2 with status "open" and the one with the status assigned is not sent.

If you don't like the styling I can add some styles but would be better to explain me exactly what you would like.

@clair13
Copy link
Author

clair13 commented Mar 16, 2023

I am pretty sure that my styling ideas will not be so good ,that's why I am saying to tell me exactly what you need.Also feel free to tell me any other things that need change.

task send_bounty_email: :environment do
@bounties = Bounty.open.last(15)
if @bounties.any?
@users = User.all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @clair13,
I feel this is performance intensive task (although it is rake task but still) as it will load the entire table into memory, instantiating all the rows; then iterate through the instances.

We can use find_each, as it does this in batches, which is more efficient in terms of memory usage.

something like below

User.find_each do |user|
  BountyMailer.with(user: user, bounties: @bounties).open_bounty.deliver_later
end

References
Retrieving Multiple Objects in Batches
ActiveRecord::Batches

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion,I fixed it based on your suggestion.

@clair13 clair13 changed the title WIP Add a weekly Newsletter for latest open Bounties #19 Add a weekly Newsletter for the latest open Bounties #19 Mar 24, 2023
@clair13
Copy link
Author

clair13 commented Mar 24, 2023

I just wanted to follow up and see if there's anything else you need from me on this pull request. Please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants