diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fffa03..7147caf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -209,7 +209,7 @@ jobs: bin/rails test:system - name: Keep screenshots from failed system tests - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@v7 if: failure() with: name: screenshots diff --git a/.tokeignore b/.tokeignore index 49d20cc..64c1068 100644 --- a/.tokeignore +++ b/.tokeignore @@ -6,3 +6,4 @@ package-lock.json tsconfig.json tsconfig.*.json db/migrate/ +config/languages.yml diff --git a/Dockerfile.dev b/Dockerfile.dev index 9fca8b3..76f16a5 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -48,5 +48,9 @@ RUN git config --system http.timeout 30 && \ EXPOSE 3000 EXPOSE 3036 +# Disable dev warnings +RUN bundle exec skylight disable_dev_warning +RUN bundle config set default_cli_command install --global + # Start the main process -CMD ["rails", "server", "-b", "0.0.0.0"] +CMD ["rails", "server", "-b", "0.0.0.0"] diff --git a/Gemfile b/Gemfile index f0cd11f..41f008d 100644 --- a/Gemfile +++ b/Gemfile @@ -171,3 +171,5 @@ gem "vite_rails", "~> 3.0" gem "rubyzip", "~> 3.2" gem "aws-sdk-s3", require: false + +gem "mailkick" diff --git a/Gemfile.lock b/Gemfile.lock index f64ae2c..ca439be 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -304,6 +304,8 @@ GEM net-imap net-pop net-smtp + mailkick (2.0.0) + activesupport (>= 7.1) marcel (1.1.0) matrix (0.4.3) mcp (0.7.1) @@ -680,6 +682,7 @@ DEPENDENCIES kamal letter_opener letter_opener_web (~> 3.0) + mailkick memory_profiler norairrecord (~> 0.5.1) oj diff --git a/app/controllers/api/hackatime/v1/hackatime_controller.rb b/app/controllers/api/hackatime/v1/hackatime_controller.rb index bef9f77..6262abe 100644 --- a/app/controllers/api/hackatime/v1/hackatime_controller.rb +++ b/app/controllers/api/hackatime/v1/hackatime_controller.rb @@ -254,6 +254,7 @@ class Api::Hackatime::V1::HackatimeController < ApplicationController results << [ new_heartbeat.attributes, 201 ] should_enqueue_mirror_sync ||= source_type == :direct_entry rescue => e + Sentry.capture_exception(e) Rails.logger.error("Error creating heartbeat: #{e.class.name} #{e.message}") results << [ { error: e.message, type: e.class.name }, 422 ] end diff --git a/app/controllers/api/v1/external_slack_controller.rb b/app/controllers/api/v1/external_slack_controller.rb index cefaac5..21cb661 100644 --- a/app/controllers/api/v1/external_slack_controller.rb +++ b/app/controllers/api/v1/external_slack_controller.rb @@ -53,6 +53,7 @@ module Api render json: { error: user.errors.full_messages }, status: :unprocessable_entity end rescue => e + Sentry.capture_exception(e) Rails.logger.error "Error creating user from external Slack data: #{e.message}" render json: { error: "Internal server error" }, status: :internal_server_error end diff --git a/app/controllers/api/v1/stats_controller.rb b/app/controllers/api/v1/stats_controller.rb index b8d916d..83a9351 100644 --- a/app/controllers/api/v1/stats_controller.rb +++ b/app/controllers/api/v1/stats_controller.rb @@ -280,6 +280,7 @@ class Api::V1::StatsController < ApplicationController JSON.parse(response.body)["user"]["id"] rescue => e + Sentry.capture_exception(e) Rails.logger.error("Error finding user by email: #{e}") nil end diff --git a/app/controllers/my/heartbeat_imports_controller.rb b/app/controllers/my/heartbeat_imports_controller.rb index 424d01f..24ebbaa 100644 --- a/app/controllers/my/heartbeat_imports_controller.rb +++ b/app/controllers/my/heartbeat_imports_controller.rb @@ -22,6 +22,7 @@ class My::HeartbeatImportsController < ApplicationController status: status }, status: :accepted rescue => e + Sentry.capture_exception(e) Rails.logger.error("Error starting heartbeat import for user #{current_user&.id}: #{e.message}") render json: { error: "error reading file: #{e.message}" }, status: :internal_server_error end diff --git a/app/controllers/settings/access_controller.rb b/app/controllers/settings/access_controller.rb index 39faa4f..2640e24 100644 --- a/app/controllers/settings/access_controller.rb +++ b/app/controllers/settings/access_controller.rb @@ -23,6 +23,7 @@ class Settings::AccessController < Settings::BaseController render json: { token: new_api_key.token }, status: :ok end rescue => e + Sentry.capture_exception(e) Rails.logger.error("error rotate #{e.class.name} #{e.message}") render json: { error: "cant rotate" }, status: :unprocessable_entity end diff --git a/app/controllers/settings/base_controller.rb b/app/controllers/settings/base_controller.rb index a8c7718..2eddcbd 100644 --- a/app/controllers/settings/base_controller.rb +++ b/app/controllers/settings/base_controller.rb @@ -103,7 +103,7 @@ class Settings::BaseController < InertiaController username: @user.username, theme: @user.theme, uses_slack_status: @user.uses_slack_status, - weekly_summary_email_enabled: @user.weekly_summary_email_enabled, + weekly_summary_email_enabled: @user.subscribed?("weekly_summary"), hackatime_extension_text_type: @user.hackatime_extension_text_type, allow_public_stats_lookup: @user.allow_public_stats_lookup, trust_level: @user.trust_level, diff --git a/app/controllers/settings/notifications_controller.rb b/app/controllers/settings/notifications_controller.rb index dd490a6..0c0d8e8 100644 --- a/app/controllers/settings/notifications_controller.rb +++ b/app/controllers/settings/notifications_controller.rb @@ -4,11 +4,22 @@ class Settings::NotificationsController < Settings::BaseController end def update - if @user.update(notifications_params) - PosthogService.capture(@user, "settings_updated", { fields: notifications_params.keys }) + list = "weekly_summary" + enabled = params.dig(:user, :weekly_summary_email_enabled) + + begin + if enabled == "1" || enabled == true + @user.subscribe(list) unless @user.subscribed?(list) + else + @user.unsubscribe(list) if @user.subscribed?(list) + end + + PosthogService.capture(@user, "settings_updated", { fields: [ "weekly_summary_email_enabled" ] }) redirect_to my_settings_notifications_path, notice: "Settings updated successfully" - else - flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings" + rescue => e + Sentry.capture_exception(e) + Rails.logger.error("Failed to update notification settings: #{e.message}") + flash.now[:error] = "Failed to update settings, sorry :(" render_notifications(status: :unprocessable_entity) end end @@ -22,8 +33,4 @@ class Settings::NotificationsController < Settings::BaseController status: status ) end - - def notifications_params - params.require(:user).permit(:weekly_summary_email_enabled) - end end diff --git a/app/javascript/pages/Users/Settings/Notifications.svelte b/app/javascript/pages/Users/Settings/Notifications.svelte index f860539..606564d 100644 --- a/app/javascript/pages/Users/Settings/Notifications.svelte +++ b/app/javascript/pages/Users/Settings/Notifications.svelte @@ -18,11 +18,7 @@ }: NotificationsPageProps = $props(); let csrfToken = $state(""); - let weeklySummaryEmailEnabled = $state(true); - - $effect(() => { - weeklySummaryEmailEnabled = user.weekly_summary_email_enabled; - }); + let weeklySummaryEmailEnabled = $state(user.weekly_summary_email_enabled); onMount(() => { csrfToken = diff --git a/app/jobs/weekly_summary_email_job.rb b/app/jobs/weekly_summary_email_job.rb index 52737ff..d4f7dbd 100644 --- a/app/jobs/weekly_summary_email_job.rb +++ b/app/jobs/weekly_summary_email_job.rb @@ -2,37 +2,31 @@ class WeeklySummaryEmailJob < ApplicationJob queue_as :literally_whenever def perform(reference_time = Time.current) - # Weekly summary delivery is intentionally disabled for now. - # Context: https://hackclub.slack.com/archives/D083UR1DR7V/p1772321709715969 - # Keep this no-op until we explicitly decide to turn the campaign back on. - reference_time + return unless Flipper.enabled?(:weekly_summary_emails) - # now_utc = reference_time.utc - # return unless send_window?(now_utc) + now_utc = reference_time.utc + cutoff = now_utc - 3.weeks - # User.where(weekly_summary_email_enabled: true).find_each do |user| - # recipient_email = user.email_addresses.order(:id).pick(:email) - # next if recipient_email.blank? - - # user_timezone = ActiveSupport::TimeZone[user.timezone] || ActiveSupport::TimeZone["UTC"] - # user_now = now_utc.in_time_zone(user_timezone) - # ends_at_local = user_now.beginning_of_week(:monday) - # starts_at_local = ends_at_local - 1.week - - # WeeklySummaryMailer.weekly_summary( - # user, - # recipient_email: recipient_email, - # starts_at: starts_at_local.utc, - # ends_at: ends_at_local.utc - # ).deliver_now - # rescue StandardError => e - # Rails.logger.error("Weekly summary email failed for user #{user.id}: #{e.class} #{e.message}") - # end + eligible_users(cutoff).find_each do |user| + WeeklySummaryUserEmailJob.perform_later(user.id, now_utc.iso8601) + end end private - def send_window?(time) - time.friday? && time.hour == 17 && time.min == 30 + def eligible_users(cutoff) + users = User.arel_table + heartbeats = Heartbeat.arel_table + + recent_activity_exists = Heartbeat.unscoped + .where(heartbeats[:user_id].eq(users[:id])) + .where(heartbeats[:deleted_at].eq(nil)) + .where(heartbeats[:time].gteq(cutoff.to_f)) + .arel + .exists + + User.subscribed("weekly_summary").where( + users[:created_at].gteq(cutoff).or(recent_activity_exists) + ).where.not(id: DeletionRequest.active.select(:user_id)) end end diff --git a/app/jobs/weekly_summary_user_email_job.rb b/app/jobs/weekly_summary_user_email_job.rb new file mode 100644 index 0000000..c65a092 --- /dev/null +++ b/app/jobs/weekly_summary_user_email_job.rb @@ -0,0 +1,26 @@ +class WeeklySummaryUserEmailJob < ApplicationJob + queue_as :literally_whenever + + def perform(user_id, now_utc_iso8601) + user = User.find_by(id: user_id) + return if user.nil? + return unless user.subscribed?("weekly_summary") + return if user.pending_deletion? + + recipient_email = user.email_addresses.order(:id).pick(:email) + return if recipient_email.blank? + + now_utc = Time.zone.parse(now_utc_iso8601) + user_timezone = ActiveSupport::TimeZone[user.timezone] || ActiveSupport::TimeZone["UTC"] + user_now = now_utc.in_time_zone(user_timezone) + ends_at_local = user_now.beginning_of_week(:monday) + starts_at_local = ends_at_local - 1.week + + WeeklySummaryMailer.weekly_summary( + user, + recipient_email: recipient_email, + starts_at: starts_at_local.utc, + ends_at: ends_at_local.utc + ).deliver_now + end +end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 0d6e196..298fd80 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,4 +1,6 @@ class ApplicationMailer < ActionMailer::Base + include Mailkick::UrlHelper + default from: ENV.fetch("SMTP_FROM_EMAIL", "noreply@timedump.hackclub.com") layout "mailer" end diff --git a/app/mailers/weekly_summary_mailer.rb b/app/mailers/weekly_summary_mailer.rb index bac5b42..b773a97 100644 --- a/app/mailers/weekly_summary_mailer.rb +++ b/app/mailers/weekly_summary_mailer.rb @@ -3,6 +3,7 @@ class WeeklySummaryMailer < ApplicationMailer def weekly_summary(user, recipient_email:, starts_at:, ends_at:) @user = user + @unsubscribe_url = mailkick_unsubscribe_url(@user, "weekly_summary") user_timezone = ActiveSupport::TimeZone[@user.timezone] @timezone = user_timezone || ActiveSupport::TimeZone["UTC"] @timezone_label = user_timezone ? @user.timezone : @timezone.tzinfo.identifier @@ -13,9 +14,7 @@ class WeeklySummaryMailer < ApplicationMailer @subject_period_label = "#{@starts_at_local.strftime("%b %-d")} - #{@ends_at_local.strftime("%b %-d, %Y")}" @period_label = @subject_period_label - coding_heartbeats = @user.heartbeats - .coding_only - .where(time: @starts_at.to_f...@ends_at.to_f) + coding_heartbeats = @user.heartbeats.where(time: @starts_at.to_f...@ends_at.to_f) @total_seconds = coding_heartbeats.duration_seconds @daily_average_seconds = (@total_seconds / 7.0).round diff --git a/app/models/user.rb b/app/models/user.rb index c86b6c5..218076c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,11 +5,14 @@ class User < ApplicationRecord include ::SlackIntegration include ::GithubIntegration + has_subscriptions + USERNAME_MAX_LENGTH = 21 # going over 21 overflows the navbar has_paper_trail after_create :track_signup + after_create :subscribe_to_default_lists before_validation :normalize_username encrypts :slack_access_token, :github_access_token, :hca_access_token @@ -26,7 +29,6 @@ class User < ApplicationRecord attribute :allow_public_stats_lookup, :boolean, default: true attribute :default_timezone_leaderboard, :boolean, default: true - attribute :weekly_summary_email_enabled, :boolean, default: true def country_name ISO3166::Country.new(country_code).common_name @@ -326,6 +328,10 @@ class User < ApplicationRecord PosthogService.capture(self, "account_created", { source: "signup" }) end + def subscribe_to_default_lists + subscribe("weekly_summary") + end + def normalize_username original = username @username_cleared_for_invisible = false diff --git a/app/services/programming_goals_progress_service.rb b/app/services/programming_goals_progress_service.rb index f88f32b..0875609 100644 --- a/app/services/programming_goals_progress_service.rb +++ b/app/services/programming_goals_progress_service.rb @@ -37,7 +37,7 @@ class ProgrammingGoalsProgressService def tracked_seconds_for_goal(goal, now:) time_window = time_window_for(goal.period, now: now) - scope = user.heartbeats.coding_only.where(time: time_window.begin.to_i..time_window.end.to_i) + scope = user.heartbeats.where(time: time_window.begin.to_i..time_window.end.to_i) scope = scope.where(project: goal.projects) if goal.projects.any? if goal.languages.any? diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index ef70c3d..6cf367b 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -27,6 +27,9 @@
+ <% if @unsubscribe_url.present? %> +

Don't want to get these anymore? <%= link_to "Unsubscribe", @unsubscribe_url, style: "color: #9ca3af; text-decoration: underline;" %>

+ <% end %>

15 Falls Road, Shelburne, VT 05482, United States

diff --git a/config/initializers/mailkick.rb b/config/initializers/mailkick.rb new file mode 100644 index 0000000..c009b7e --- /dev/null +++ b/config/initializers/mailkick.rb @@ -0,0 +1 @@ +Mailkick.headers = true diff --git a/db/migrate/20260301200000_create_mailkick_subscriptions_v2.rb b/db/migrate/20260301200000_create_mailkick_subscriptions_v2.rb new file mode 100644 index 0000000..11afad0 --- /dev/null +++ b/db/migrate/20260301200000_create_mailkick_subscriptions_v2.rb @@ -0,0 +1,14 @@ +class CreateMailkickSubscriptionsV2 < ActiveRecord::Migration[8.1] + def change + create_table :mailkick_subscriptions, if_not_exists: true do |t| + t.references :subscriber, polymorphic: true, null: false + t.string :list + t.timestamps + end + + unless index_exists?(:mailkick_subscriptions, [ :subscriber_type, :subscriber_id, :list ], name: "index_mailkick_subscriptions_on_subscriber_and_list") + add_index :mailkick_subscriptions, [ :subscriber_type, :subscriber_id, :list ], + unique: true, name: "index_mailkick_subscriptions_on_subscriber_and_list" + end + end +end diff --git a/db/migrate/20260301200002_create_notable_tables.rb b/db/migrate/20260301200002_create_notable_tables.rb new file mode 100644 index 0000000..6ab4fd0 --- /dev/null +++ b/db/migrate/20260301200002_create_notable_tables.rb @@ -0,0 +1,30 @@ +class CreateNotableTables < ActiveRecord::Migration[8.1] + def change + create_table :notable_jobs, if_not_exists: true do |t| + t.datetime :created_at + t.text :job + t.string :job_id + t.text :note + t.string :note_type + t.string :queue + t.float :queued_time + t.float :runtime + end + + create_table :notable_requests, if_not_exists: true do |t| + t.text :action + t.datetime :created_at + t.string :ip + t.text :note + t.string :note_type + t.text :params + t.text :referrer + t.string :request_id + t.float :request_time + t.integer :status + t.text :url + t.text :user_agent + t.references :user, polymorphic: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 14c9baa..6daeaef 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_02_23_212054) do +ActiveRecord::Schema[8.1].define(version: 2026_03_01_200002) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "pg_stat_statements" @@ -410,6 +410,45 @@ ActiveRecord::Schema[8.1].define(version: 2026_02_23_212054) do t.index ["start_date"], name: "index_leaderboards_on_start_date", where: "(deleted_at IS NULL)" end + create_table "mailkick_subscriptions", force: :cascade do |t| + t.datetime "created_at", null: false + t.string "list" + t.bigint "subscriber_id" + t.string "subscriber_type" + t.datetime "updated_at", null: false + t.index ["subscriber_type", "subscriber_id", "list"], name: "index_mailkick_subscriptions_on_subscriber_and_list", unique: true + t.index ["subscriber_type", "subscriber_id"], name: "index_mailkick_subscriptions_on_subscriber" + end + + create_table "notable_jobs", force: :cascade do |t| + t.datetime "created_at" + t.text "job" + t.string "job_id" + t.text "note" + t.string "note_type" + t.string "queue" + t.float "queued_time" + t.float "runtime" + end + + create_table "notable_requests", force: :cascade do |t| + t.text "action" + t.datetime "created_at" + t.string "ip" + t.text "note" + t.string "note_type" + t.text "params" + t.text "referrer" + t.string "request_id" + t.float "request_time" + t.integer "status" + t.text "url" + t.text "user_agent" + t.bigint "user_id" + t.string "user_type" + t.index ["user_type", "user_id"], name: "index_notable_requests_on_user" + end + create_table "oauth_access_grants", force: :cascade do |t| t.bigint "application_id", null: false t.datetime "created_at", null: false @@ -647,7 +686,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_02_23_212054) do t.datetime "updated_at", null: false t.string "username" t.boolean "uses_slack_status", default: false, null: false - t.boolean "weekly_summary_email_enabled", default: true, null: false + t.boolean "weekly_summary_email_enabled", default: false, null: false t.index ["github_uid", "github_access_token"], name: "index_users_on_github_uid_and_access_token" t.index ["github_uid"], name: "index_users_on_github_uid" t.index ["slack_uid"], name: "index_users_on_slack_uid", unique: true diff --git a/test/controllers/settings_notifications_controller_test.rb b/test/controllers/settings_notifications_controller_test.rb index 90fe881..f947c5f 100644 --- a/test/controllers/settings_notifications_controller_test.rb +++ b/test/controllers/settings_notifications_controller_test.rb @@ -13,14 +13,27 @@ class SettingsNotificationsControllerTest < ActionDispatch::IntegrationTest assert_inertia_component "Users/Settings/Notifications" end - test "notifications update persists weekly summary email preference" do + test "notifications update subscribes user to weekly summary" do user = users(:one) + user.unsubscribe("weekly_summary") if user.subscribed?("weekly_summary") + sign_in_as(user) + + patch my_settings_notifications_path, params: { user: { weekly_summary_email_enabled: "1" } } + + assert_response :redirect + assert_redirected_to my_settings_notifications_path + assert user.reload.subscribed?("weekly_summary") + end + + test "notifications update unsubscribes user from weekly summary" do + user = users(:one) + user.subscribe("weekly_summary") unless user.subscribed?("weekly_summary") sign_in_as(user) patch my_settings_notifications_path, params: { user: { weekly_summary_email_enabled: "0" } } assert_response :redirect assert_redirected_to my_settings_notifications_path - assert_equal false, user.reload.weekly_summary_email_enabled + assert_not user.reload.subscribed?("weekly_summary") end end diff --git a/test/integration/email_login_test.rb b/test/integration/email_login_test.rb new file mode 100644 index 0000000..f8ddbf1 --- /dev/null +++ b/test/integration/email_login_test.rb @@ -0,0 +1,133 @@ +require "test_helper" + +class EmailLoginTest < ActionDispatch::IntegrationTest + test "full email sign-in flow creates token and signs user in" do + user = User.create!(timezone: "UTC") + email = "login-flow-#{SecureRandom.hex(4)}@example.com" + user.email_addresses.create!(email: email, source: :signing_in) + + assert_difference -> { SignInToken.count }, 1 do + post email_auth_path, params: { email: email } + end + + assert_response :redirect + + token = SignInToken.last + assert_equal user.id, token.user_id + + get auth_token_path(token: token.token) + + assert_response :redirect + assert_redirected_to root_path + assert_equal user.id, session[:user_id] + end + + test "email sign-in is case-insensitive" do + user = User.create!(timezone: "UTC") + email = "case-test-#{SecureRandom.hex(4)}@example.com" + user.email_addresses.create!(email: email, source: :signing_in) + + post email_auth_path, params: { email: email.upcase } + + assert_response :redirect + + token = SignInToken.last + assert_equal user.id, token.user_id + end + + test "email sign-in with continue param preserves redirect" do + user = User.create!(timezone: "UTC") + email = "continue-#{SecureRandom.hex(4)}@example.com" + user.email_addresses.create!(email: email, source: :signing_in) + continue_path = "/oauth/authorize?client_id=test&response_type=code" + + post email_auth_path, params: { email: email, continue: continue_path } + + token = SignInToken.last + assert_equal continue_path, token.continue_param + + get auth_token_path(token: token.token) + + assert_response :redirect + assert_redirected_to continue_path + assert_equal user.id, session[:user_id] + end + + test "email sign-in token can only be used once" do + user = User.create!(timezone: "UTC") + email = "once-#{SecureRandom.hex(4)}@example.com" + user.email_addresses.create!(email: email, source: :signing_in) + + post email_auth_path, params: { email: email } + token = SignInToken.last + + get auth_token_path(token: token.token) + assert_equal user.id, session[:user_id] + + delete signout_path + assert_nil session[:user_id] + + get auth_token_path(token: token.token) + assert_redirected_to root_path + assert_nil session[:user_id] + end + + test "expired email token does not sign user in" do + user = User.create!(timezone: "UTC") + token = user.sign_in_tokens.create!( + auth_type: :email, + expires_at: 1.hour.ago + ) + + get auth_token_path(token: token.token) + + assert_redirected_to root_path + assert_nil session[:user_id] + end + + test "invalid token shows error" do + get auth_token_path(token: "completely-bogus-token") + + assert_redirected_to root_path + assert_nil session[:user_id] + end + + test "email verification flow adds email to user" do + user = User.create!(timezone: "UTC") + sign_in_as(user) + + new_email = "verify-#{SecureRandom.hex(4)}@example.com" + + assert_difference -> { user.email_verification_requests.count }, 1 do + post add_email_auth_path, params: { email: new_email } + end + + assert_redirected_to my_settings_path + + verification_request = user.email_verification_requests.last + assert_equal new_email, verification_request.email + + assert_difference -> { user.email_addresses.count }, 1 do + get auth_token_path(token: verification_request.token) + end + + assert user.reload.email_addresses.exists?(email: new_email) + end + + test "sign out clears session" do + user = User.create!(timezone: "UTC") + sign_in_as(user) + + assert_equal user.id, session[:user_id] + + delete signout_path + + assert_redirected_to root_path + assert_nil session[:user_id] + end + + test "new user gets subscribed to weekly summary by default" do + user = User.create!(timezone: "UTC") + assert user.subscribed?("weekly_summary"), "New users should be subscribed to weekly_summary" + end +end diff --git a/test/jobs/weekly_summary_email_job_test.rb b/test/jobs/weekly_summary_email_job_test.rb index 0e39b65..d26c83c 100644 --- a/test/jobs/weekly_summary_email_job_test.rb +++ b/test/jobs/weekly_summary_email_job_test.rb @@ -1,74 +1,79 @@ require "test_helper" class WeeklySummaryEmailJobTest < ActiveJob::TestCase - DISABLED_REASON = "Weekly summary delivery is intentionally disabled. See WeeklySummaryEmailJob for context.".freeze - setup do ActionMailer::Base.deliveries.clear + Flipper.enable(:weekly_summary_emails) + GoodJob::Job.delete_all end - test "sends weekly summaries only for opted-in users with email at friday 17:30 utc" do - skip DISABLED_REASON - - # enabled_user = User.create!(timezone: "UTC", weekly_summary_email_enabled: true) - # enabled_user.email_addresses.create!(email: "enabled-#{SecureRandom.hex(4)}@example.com", source: :signing_in) - # create_coding_heartbeat(enabled_user, Time.utc(2026, 2, 21, 12, 0, 0), "project-enabled", "Ruby") - # create_coding_heartbeat(enabled_user, Time.utc(2026, 2, 21, 12, 20, 0), "project-enabled", "Ruby") - - # disabled_user = User.create!(timezone: "UTC", weekly_summary_email_enabled: false) - # disabled_user.email_addresses.create!(email: "disabled-#{SecureRandom.hex(4)}@example.com", source: :signing_in) - # create_coding_heartbeat(disabled_user, Time.utc(2026, 2, 21, 13, 0, 0), "project-disabled", "Ruby") - # create_coding_heartbeat(disabled_user, Time.utc(2026, 2, 21, 13, 20, 0), "project-disabled", "Ruby") - - # user_without_email = User.create!(timezone: "UTC", weekly_summary_email_enabled: true) - # create_coding_heartbeat(user_without_email, Time.utc(2026, 2, 20, 14, 0, 0), "project-no-email", "Ruby") - - # reference_time = Time.utc(2026, 2, 27, 17, 30, 0) # Friday, 5:30 PM GMT - - # assert_difference -> { ActionMailer::Base.deliveries.count }, 1 do - # WeeklySummaryEmailJob.perform_now(reference_time) - # end - - # mail = ActionMailer::Base.deliveries.last - # assert_equal [ enabled_user.email_addresses.first.email ], mail.to - # assert_equal "Your Hackatime weekly summary (Feb 16 - Feb 23, 2026)", mail.subject - # assert_includes mail.text_part.body.decoded, "Top projects:" + teardown do + Flipper.disable(:weekly_summary_emails) + GoodJob::Job.delete_all end - test "uses previous local calendar week for each user's timezone" do - skip DISABLED_REASON + test "enqueues for subscribed users who signed up recently or coded recently" do + reference_time = Time.utc(2026, 3, 1, 12, 0, 0) + cutoff = reference_time - 3.weeks - # user = User.create!(timezone: "America/Los_Angeles", weekly_summary_email_enabled: true) - # user.email_addresses.create!(email: "la-#{SecureRandom.hex(4)}@example.com", source: :signing_in) - # create_coding_heartbeat(user, Time.utc(2026, 2, 16, 8, 30, 0), "local-week-in-range", "Ruby") - # create_coding_heartbeat(user, Time.utc(2026, 2, 16, 8, 50, 0), "local-week-in-range", "Ruby") - # create_coding_heartbeat(user, Time.utc(2026, 2, 16, 7, 30, 0), "local-week-out-of-range", "Ruby") + recent_signup = User.create!(timezone: "UTC") + recent_signup.update_column(:created_at, cutoff + 1.hour) - # reference_time = Time.utc(2026, 2, 27, 17, 30, 0) + recent_coder = User.create!(timezone: "UTC") + recent_coder.update_column(:created_at, cutoff - 1.day) + create_coding_heartbeat(recent_coder, cutoff + 2.hours, "recent-coder", "Ruby") - # assert_difference -> { ActionMailer::Base.deliveries.count }, 1 do - # WeeklySummaryEmailJob.perform_now(reference_time) - # end + stale_user = User.create!(timezone: "UTC") + stale_user.update_column(:created_at, cutoff - 1.day) + create_coding_heartbeat(stale_user, cutoff - 2.hours, "stale-user", "Ruby") - # mail = ActionMailer::Base.deliveries.last - # assert_equal "Your Hackatime weekly summary (Feb 16 - Feb 23, 2026)", mail.subject - # assert_includes mail.text_part.body.decoded, "Feb 16 - Feb 23, 2026" - # assert_includes mail.text_part.body.decoded, "local-week-in-range" - # assert_not_includes mail.text_part.body.decoded, "local-week-out-of-range" + unsubscribed_recent_coder = User.create!(timezone: "UTC") + unsubscribed_recent_coder.unsubscribe("weekly_summary") + create_coding_heartbeat(unsubscribed_recent_coder, cutoff + 3.hours, "unsubscribed", "Ruby") + + pending_deletion_user = User.create!(timezone: "UTC") + DeletionRequest.create_for_user!(pending_deletion_user) + create_coding_heartbeat(pending_deletion_user, cutoff + 4.hours, "pending-deletion", "Ruby") + + assert_difference -> { GoodJob::Job.where(job_class: "WeeklySummaryUserEmailJob").count }, 2 do + WeeklySummaryEmailJob.perform_now(reference_time) + end + + jobs = GoodJob::Job.where(job_class: "WeeklySummaryUserEmailJob").order(:id) + enqueued_user_ids = jobs.map { |job| job.serialized_params.fetch("arguments").first.to_i }.sort + enqueued_reference_times = jobs.map { |job| job.serialized_params.fetch("arguments").second }.uniq + + assert_equal [ recent_signup.id, recent_coder.id ].sort, enqueued_user_ids + assert_equal [ reference_time.iso8601 ], enqueued_reference_times end - test "does not send weekly summaries outside friday 17:30 utc" do - skip DISABLED_REASON + test "does not enqueue summaries when feature flag is disabled" do + Flipper.disable(:weekly_summary_emails) + User.create!(timezone: "UTC") - # user = User.create!(timezone: "UTC", weekly_summary_email_enabled: true) - # user.email_addresses.create!(email: "outside-window-#{SecureRandom.hex(4)}@example.com", source: :signing_in) - # create_coding_heartbeat(user, Time.utc(2026, 2, 20, 12, 0, 0), "project-outside", "Ruby") + assert_no_difference -> { GoodJob::Job.where(job_class: "WeeklySummaryUserEmailJob").count } do + WeeklySummaryEmailJob.perform_now(Time.utc(2026, 3, 1, 12, 0, 0)) + end + end - # reference_time = Time.utc(2026, 2, 27, 17, 29, 0) + test "does not send user summary email when user is unsubscribed before perform" do + user = User.create!(timezone: "UTC") + user.email_addresses.create!(email: "unsubscribed-#{SecureRandom.hex(4)}@example.com", source: :signing_in) + user.unsubscribe("weekly_summary") - # assert_no_difference -> { ActionMailer::Base.deliveries.count } do - # WeeklySummaryEmailJob.perform_now(reference_time) - # end + assert_no_difference -> { ActionMailer::Base.deliveries.count } do + WeeklySummaryUserEmailJob.perform_now(user.id, Time.utc(2026, 3, 1, 12, 0, 0).iso8601) + end + end + + test "does not send user summary email when user has pending deletion request" do + user = User.create!(timezone: "UTC") + user.email_addresses.create!(email: "pending-deletion-#{SecureRandom.hex(4)}@example.com", source: :signing_in) + DeletionRequest.create_for_user!(user) + + assert_no_difference -> { ActionMailer::Base.deliveries.count } do + WeeklySummaryUserEmailJob.perform_now(user.id, Time.utc(2026, 3, 1, 12, 0, 0).iso8601) + end end private diff --git a/test/mailers/weekly_summary_mailer_test.rb b/test/mailers/weekly_summary_mailer_test.rb index 8f6f1a9..189f5ba 100644 --- a/test/mailers/weekly_summary_mailer_test.rb +++ b/test/mailers/weekly_summary_mailer_test.rb @@ -2,10 +2,7 @@ require "test_helper" class WeeklySummaryMailerTest < ActionMailer::TestCase setup do - @user = User.create!( - timezone: "UTC", - weekly_summary_email_enabled: true - ) + @user = User.create!(timezone: "UTC") @recipient_email = "weekly-mailer-#{SecureRandom.hex(4)}@example.com" @user.email_addresses.create!(email: @recipient_email, source: :signing_in) end @@ -33,6 +30,8 @@ class WeeklySummaryMailerTest < ActionMailer::TestCase assert_includes mail.text_part.body.decoded, "TOP LANGUAGES" assert_includes mail.text_part.body.decoded, "hackatime-web" assert_not_includes mail.html_part.body.decoded.downcase, "gradient" + assert_includes mail.html_part.body.decoded, "Unsubscribe" + assert_includes mail.header["List-Unsubscribe"].to_s, "/mailkick/subscriptions/" end private diff --git a/test/system/settings/notifications_settings_test.rb b/test/system/settings/notifications_settings_test.rb index b59ea1b..6e249bf 100644 --- a/test/system/settings/notifications_settings_test.rb +++ b/test/system/settings/notifications_settings_test.rb @@ -19,7 +19,7 @@ class NotificationsSettingsTest < ApplicationSystemTestCase end test "notifications settings updates weekly summary email preference" do - @user.update!(weekly_summary_email_enabled: true) + @user.subscribe("weekly_summary") unless @user.subscribed?("weekly_summary") visit my_settings_notifications_path @@ -30,6 +30,6 @@ class NotificationsSettingsTest < ApplicationSystemTestCase click_on "Save notification settings" assert_text "Settings updated successfully" - assert_equal false, @user.reload.weekly_summary_email_enabled + assert_not @user.reload.subscribed?("weekly_summary") end end