Refactor growstuff rake notifications to use Notification model

- Created ReminderService to handle reminder logic and Markdown generation
- Updated growstuff.rake to use ReminderService and create Notification records
- Implemented NotificationsHelper#reply_link for better email navigation
- Made Notification#notifiable optional to support system-wide reminders
- Added unit tests for ReminderService and rake tasks

Co-authored-by: CloCkWeRX <365751+CloCkWeRX@users.noreply.github.com>

app/helpers/notifications_helper.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module NotificationsHelper
+  def reply_link(notification)
+    return "" unless notification.sender
+
+    # Mailboxer provides the conversation.
+    # We want to link to the conversation where the message belongs.
+    # If it's a new message, we might want to link to new_message_path(recipient_id: notification.sender_id)
+    # But Notification model seems to be tied to existing messages.
+
+    if notification.notifiable_type == "Post"
+      post_url(notification.notifiable)
+    elsif notification.sender
+      # Link to the message/conversation
+      # Based on routes.rb: resources :conversations
+      # We need to find the conversation between sender and recipient
+      conversation = notification.recipient.mailbox.conversations.joins(:participants).where(mailboxer_notifications: { sender_id: notification.sender_id }).first
+      if conversation
+        conversation_url(conversation)
+      else
+        new_message_url(recipient_id: notification.sender.id)
+      end
+    else
+      root_url
+    end
+  end
+end

app/mailers/notifier_mailer.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class NotifierMailer < ApplicationMailer
-  # include NotificationsHelper
+  include NotificationsHelper
   default from: "Growstuff <#{ENV.fetch('GROWSTUFF_EMAIL', nil)}>"
 
   def verifier

app/models/notification.rb

@@ -3,7 +3,7 @@
 class Notification < ApplicationRecord
   belongs_to :sender, class_name: 'Member', inverse_of: :sent_notifications
   belongs_to :recipient, class_name: 'Member', inverse_of: :notifications
-  belongs_to :notifiable, polymorphic: true
+  belongs_to :notifiable, polymorphic: true, optional: true
 
   validates :subject, length: { maximum: 255 }
 

app/services/reminder_service.rb

@@ -0,0 +1,139 @@
+# frozen_string_literal: true
+
+class ReminderService
+  include Rails.application.routes.url_helpers
+
+  def initialize
+    @bot = Member.find_by(login_name: 'cropbot') || Member.first
+    @sitename = ENV.fetch('GROWSTUFF_SITE_NAME', 'Growstuff')
+  end
+
+  def send_planting_reminders
+    # Send on Monday
+    return unless Time.zone.today.wday == 1
+
+    Member.confirmed.wants_reminders.find_each do |m|
+      next if m.plantings.active.empty?
+
+      subject = "Your #{Time.zone.today.strftime('%B %Y')} #{@sitename} progress report"
+      body = generate_planting_reminder_body(m)
+
+      Notification.create!(
+        recipient: m,
+        sender: @bot,
+        subject: subject,
+        body: body
+      )
+    end
+  end
+
+  def send_harvest_reminders
+    # Send on Wednesday
+    return unless Time.zone.today.wday == 3
+
+    Member.confirmed.wants_harvest_reminders.find_each do |m|
+      harvesting_plantings = m.plantings.active.select(&:harvest_in_next_week?)
+      next if harvesting_plantings.empty?
+
+      subject = I18n.t('notifier_mailer.harvest_reminder.subject', sitename: @sitename)
+      body = generate_harvest_reminder_body(m, harvesting_plantings)
+
+      Notification.create!(
+        recipient: m,
+        sender: @bot,
+        subject: subject,
+        body: body
+      )
+    end
+  end
+
+  private
+
+  def generate_planting_reminder_body(member)
+    late = []
+    super_late = []
+    harvesting = []
+    others = []
+
+    member.plantings.active.annual.each do |planting|
+      if planting.finish_is_predicatable?
+        if planting.super_late?
+          super_late << planting
+        elsif planting.late?
+          late << planting
+        elsif planting.harvest_time?
+          harvesting << planting
+        else
+          others << planting
+        end
+      end
+    end
+
+    body = "Hello #{member.login_name},\n\n"
+    body += "## Your Weekly #{@sitename} progress report\n\n"
+
+    if harvesting.any?
+      body += "### Ready to harvest\n"
+      body += "Congratulations, you have plants ready to harvest\n\n"
+      harvesting.each do |p|
+        body += "* [#{p.crop}](#{planting_url(p, host: default_host)})\n"
+      end
+      body += "\n"
+    end
+
+    if others.any?
+      body += "### Progress report\n\n"
+      others.each do |p|
+        body += "* [#{p.crop}](#{planting_url(p, host: default_host)}) is #{format('%.0f', p.percentage_grown)}% grown with #{(p.finish_predicted_at - Time.zone.today).to_i} days to go.\n"
+      end
+      body += "\n"
+    end
+
+    if late.any?
+      body += "### Late\n"
+      body += "These plantings are at the end of their lifecycle.\n\n"
+      late.each do |p|
+        body += "* [#{p.crop}](#{planting_url(p, host: default_host)})\n"
+      end
+      body += "\n"
+    end
+
+    if super_late.any?
+      body += "### Super late\n"
+      body += "We suspect the following plantings finished long ago and no longer need tracking. You can mark them as finished to stop tracking.\n\n"
+      super_late.each do |p|
+        body += "* [#{p.crop}](#{planting_url(p, host: default_host)}) planted on #{p.planted_at.to_date}\n"
+      end
+      body += "\n"
+    end
+
+    body += "Harvested anything lately? [Track your harvests here.](#{new_harvest_url(host: default_host)})\n\n"
+    body += "Want to track and predict a planting in your garden? [Add a planting.](#{new_planting_url(host: default_host)})\n\n"
+    body += "Track and predict your entire garden, and keep your garden records up to date at [your garden overview](#{member_gardens_url(member, host: default_host)}) and on [your profile page](#{member_url(member, host: default_host)})\n\n"
+    body += "#### See you soon on #{@sitename}!"
+
+    body
+  end
+
+  def generate_harvest_reminder_body(member, plantings)
+    body = "Hello #{member.login_name},\n\n"
+    body += "## #{I18n.t('notifier_mailer.harvest_reminder.heading')}\n\n"
+    body += "#{I18n.t('notifier_mailer.harvest_reminder.intro')}\n\n"
+
+    plantings.each do |p|
+      body += "* [#{p.crop}](#{planting_url(p, host: default_host)})"
+      body += " (Predicted harvest date: #{p.first_harvest_predicted_at.to_date})" if p.first_harvest_predicted_at
+      body += "\n"
+    end
+
+    body += "\nHarvested anything lately? [Track your harvests here.](#{new_harvest_url(host: default_host)})\n\n"
+    body += "Track and predict your entire garden, and keep your garden records up to date at [your garden overview](#{member_gardens_url(member, host: default_host)}) and on [your profile page](#{member_url(member, host: default_host)})\n\n"
+    body += "#### See you soon on #{@sitename}!"
+
+    body
+  end
+
+  def default_host
+    ENV.fetch('GROWSTUFF_HOST', 'growstuff.org')
+  end
+end

lib/tasks/growstuff.rake

@@ -45,28 +45,14 @@ namespace :growstuff do
   # usage: rake growstuff:send_planting_reminder
 
   task send_planting_reminder: :environment do
-    # Heroku scheduler only lets us run things daily, so this checks
-    # Send on Monday
-    if Time.zone.today.wday == 1
-      Member.confirmed.wants_reminders.find_each do |m|
-        NotifierMailer.planting_reminder(m).deliver_later unless m.plantings.active.empty?
-      end
-    end
+    ReminderService.new.send_planting_reminders
   end
 
   desc "Send harvest reminder email"
   # usage: rake growstuff:send_harvest_reminders
 
   task send_harvest_reminders: :environment do
-    # Heroku scheduler only lets us run things daily, so this checks
-    # Send on Wednesday
-    if Time.zone.today.wday == 3
-      Member.confirmed.wants_harvest_reminders.find_each do |m|
-        if m.plantings.active.any?(&:harvest_in_next_week?)
-          NotifierMailer.harvest_reminder(m).deliver_later
-        end
-      end
-    end
+    ReminderService.new.send_harvest_reminders
   end
 
   desc "Mark seeds as finished when plant-before date expires"

spec/services/reminder_service_spec.rb

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe ReminderService do
+  let(:member) { create(:member, send_planting_reminder: true, send_harvest_reminder: true) }
+  let(:bot) { create(:cropbot) }
+  let(:service) { ReminderService.new }
+
+  before do
+    allow(Member).to receive(:find_by).with(login_name: 'cropbot').and_return(bot)
+    member.confirm
+  end
+
+  describe "#send_planting_reminders" do
+    context "on Monday" do
+      before do
+        Timecop.freeze(Time.zone.parse("2025-05-19")) # A Monday
+      end
+
+      after do
+        Timecop.return
+      end
+
+      it "creates a notification if member has active plantings" do
+        create(:planting, owner: member)
+        expect {
+          service.send_planting_reminders
+        }.to change(Notification, :count).by(1)
+      end
+
+      it "does not create a notification if member has no active plantings" do
+        expect {
+          service.send_planting_reminders
+        }.not_to change(Notification, :count)
+      end
+    end
+
+    context "not on Monday" do
+      before do
+        Timecop.freeze(Time.zone.parse("2025-05-20")) # A Tuesday
+      end
+
+      after do
+        Timecop.return
+      end
+
+      it "does nothing" do
+        create(:planting, owner: member)
+        expect {
+          service.send_planting_reminders
+        }.not_to change(Notification, :count)
+      end
+    end
+  end
+
+  describe "#send_harvest_reminders" do
+    context "on Wednesday" do
+      before do
+        Timecop.freeze(Time.zone.parse("2025-05-21")) # A Wednesday
+      end
+
+      after do
+        Timecop.return
+      end
+
+      it "creates a notification if member has plantings ready to harvest" do
+        planting = create(:planting, owner: member)
+        # Mock harvest_in_next_week?
+        allow_any_instance_of(Planting).to receive(:harvest_in_next_week?).and_return(true)
+
+        expect {
+          service.send_harvest_reminders
+        }.to change(Notification, :count).by(1)
+      end
+
+      it "does not create a notification if no plantings are ready" do
+        create(:planting, owner: member)
+        allow_any_instance_of(Planting).to receive(:harvest_in_next_week?).and_return(false)
+
+        expect {
+          service.send_harvest_reminders
+        }.not_to change(Notification, :count)
+      end
+    end
+  end
+end

spec/tasks/growstuff_rake_spec.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'rake'
+
+describe 'growstuff:reminders' do
+  before :all do
+    Rails.application.load_tasks
+  end
+
+  let(:planting_task) { Rake::Task['growstuff:send_planting_reminder'] }
+  let(:harvest_task) { Rake::Task['growstuff:send_harvest_reminders'] }
+
+  before do
+    planting_task.reenable
+    harvest_task.reenable
+  end
+
+  it "calls ReminderService for planting reminders" do
+    expect_any_instance_of(ReminderService).to receive(:send_planting_reminders)
+    planting_task.invoke
+  end
+
+  it "calls ReminderService for harvest reminders" do
+    expect_any_instance_of(ReminderService).to receive(:send_harvest_reminders)
+    harvest_task.invoke
+  end
+end