System-End/hackatime
1
0
Fork 0
mirror of https://github.com/System-End/hackatime.git synced 2026-04-20 00:35:22 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Revert all commits after d5d987a8f4 (#1020)

Browse source 
Reverts the following commits:
- 2e3cfd60 Revert Mailkick
- de320443 Merge branch 'main'
- efd189a8 Add safety-hatch for summary emails
- d5c62922 Remove exploding queries (#1018)
- 2b37201d Bump actions/upload-artifact from 6 to 7 (#1007)
- a4c10b47 Expand scopes to all time for summaries/goals (#1017)
- 4dec2f44 Unsubscribe links + slow job monitoring (#1016)
This commit is contained in:
Mahad Kalam 2026-03-01 13:09:10 +00:00 • committed by GitHub
parent 0c7fb13c82
commit c846217ce6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
31 changed files with 53 additions and 255 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.github/workflows/ci.yml vendored
View file

@ -209,7 +209,7 @@ jobs:
bin/rails test:system
- name: Keep screenshots from failed system tests
uses: actions/upload-artifact@v7
uses: actions/upload-artifact@v6
if: failure()
with:
name: screenshots

1
.tokeignore
View file

@ -6,4 +6,3 @@ package-lock.json
tsconfig.json
tsconfig.*.json
db/migrate/
config/languages.yml

6
Dockerfile.dev
View file

@ -48,9 +48,5 @@ 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"]

4
Gemfile
View file

@ -171,7 +171,3 @@ gem "vite_rails", "~> 3.0"
gem "rubyzip", "~> 3.2"
gem "aws-sdk-s3", require: false
gem "notable"
gem "mailkick"

8
Gemfile.lock
View file

@ -304,8 +304,6 @@ 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)
@ -357,9 +355,6 @@ GEM
faraday (>= 1.0, < 3.0)
faraday-net_http_persistent
net-http-persistent
notable (0.6.1)
activesupport (>= 7.1)
safely_block (>= 0.4)
oj (3.16.15)
bigdecimal (>= 3.0)
ostruct (>= 0.2)
@ -534,7 +529,6 @@ GEM
ruby_identicon (0.0.6)
chunky_png (~> 1.4.0)
rubyzip (3.2.2)
safely_block (0.5.0)
sanitize (7.0.0)
crass (~> 1.0.2)
nokogiri (>= 1.16.8)
@ -686,10 +680,8 @@ DEPENDENCIES
kamal
letter_opener
letter_opener_web (~> 3.0)
mailkick
memory_profiler
norairrecord (~> 0.5.1)
notable
oj
paper_trail
pg

3
app/controllers/api/hackatime/v1/hackatime_controller.rb
View file

@ -254,7 +254,6 @@ 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
@ -271,7 +270,6 @@ class Api::Hackatime::V1::HackatimeController < ApplicationController
end
rescue => e
# never raise an error here because it will break the heartbeat flow
Sentry.capture_exception(e)
Rails.logger.error("Error queuing project mapping: #{e.class.name} #{e.message}")
end
@ -280,7 +278,6 @@ class Api::Hackatime::V1::HackatimeController < ApplicationController
MirrorFanoutEnqueueJob.perform_later(@user.id)
rescue => e
Sentry.capture_exception(e)
Rails.logger.error("Error enqueuing mirror sync fanout: #{e.class.name} #{e.message}")
end

1
app/controllers/api/v1/external_slack_controller.rb
View file

@ -53,7 +53,6 @@ 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

1
app/controllers/api/v1/stats_controller.rb
View file

@ -280,7 +280,6 @@ 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

1
app/controllers/my/heartbeat_imports_controller.rb
View file

@ -22,7 +22,6 @@ 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

1
app/controllers/settings/access_controller.rb
View file

@ -23,7 +23,6 @@ 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

13
app/controllers/settings/notifications_controller.rb
View file

@ -4,14 +4,11 @@ class Settings::NotificationsController < Settings::BaseController
end
def update
enabled = params.dig(:user, :weekly_summary_email_enabled)
@user.weekly_summary_email_enabled = enabled == "1" || enabled == true
if @user.save
PosthogService.capture(@user, "settings_updated", { fields: [ "weekly_summary_email_enabled" ] })
if @user.update(notifications_params)
PosthogService.capture(@user, "settings_updated", { fields: notifications_params.keys })
redirect_to my_settings_notifications_path, notice: "Settings updated successfully"
else
flash.now[:error] = "Failed to update settings, sorry :("
flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings"
render_notifications(status: :unprocessable_entity)
end
end
@ -25,4 +22,8 @@ class Settings::NotificationsController < Settings::BaseController
status: status
)
end
def notifications_params
params.require(:user).permit(:weekly_summary_email_enabled)
end
end

6
app/javascript/pages/Users/Settings/Notifications.svelte
View file

@ -18,7 +18,11 @@
}: NotificationsPageProps = $props();
let csrfToken = $state("");
let weeklySummaryEmailEnabled = $state(user.weekly_summary_email_enabled);
let weeklySummaryEmailEnabled = $state(true);
$effect(() => {
weeklySummaryEmailEnabled = user.weekly_summary_email_enabled;
});
onMount(() => {
csrfToken =

46
app/jobs/weekly_summary_email_job.rb
View file

@ -2,31 +2,37 @@ class WeeklySummaryEmailJob < ApplicationJob
queue_as :literally_whenever
def perform(reference_time = Time.current)
return unless Flipper.enabled?(:weekly_summary_emails)
# 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
now_utc = reference_time.utc
cutoff = now_utc - 3.weeks
# now_utc = reference_time.utc
# return unless send_window?(now_utc)
eligible_users(cutoff).find_each do |user|
WeeklySummaryUserEmailJob.perform_later(user.id, now_utc.iso8601)
end
# 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
end
private
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.where(weekly_summary_email_enabled: true).where(
users[:created_at].gteq(cutoff).or(recent_activity_exists)
)
def send_window?(time)
time.friday? && time.hour == 17 && time.min == 30
end
end

24
app/jobs/weekly_summary_user_email_job.rb
View file

@ -1,24 +0,0 @@
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?
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

2
app/mailers/application_mailer.rb
View file

@ -1,6 +1,4 @@
class ApplicationMailer < ActionMailer::Base
include Mailkick::UrlHelper
default from: ENV.fetch("SMTP_FROM_EMAIL", "noreply@timedump.hackclub.com")
layout "mailer"
end

5
app/mailers/weekly_summary_mailer.rb
View file

@ -3,7 +3,6 @@ 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
@ -14,7 +13,9 @@ 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.where(time: @starts_at.to_f...@ends_at.to_f)
coding_heartbeats = @user.heartbeats
.coding_only
.where(time: @starts_at.to_f...@ends_at.to_f)
@total_seconds = coding_heartbeats.duration_seconds
@daily_average_seconds = (@total_seconds / 7.0).round

8
app/models/user.rb
View file

@ -5,14 +5,11 @@ 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
@ -29,6 +26,7 @@ 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
@ -328,10 +326,6 @@ class User < ApplicationRecord
PosthogService.capture(self, "account_created", { source: "signup" })
end
def subscribe_to_default_lists
# no-op: weekly_summary_email_enabled defaults to false
end
def normalize_username
original = username
@username_cleared_for_invisible = false

2
app/services/programming_goals_progress_service.rb
View file

@ -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.where(time: time_window.begin.to_i..time_window.end.to_i)
scope = user.heartbeats.coding_only.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?

3
app/views/layouts/mailer.html.erb
View file

@ -27,9 +27,6 @@
<tr>
<td>
<div class="px-4 py-5 text-center text-xs text-gray-400">
<% if @unsubscribe_url.present? %>
<p class="my-1">Don't want to get these anymore? <%= link_to "Unsubscribe", @unsubscribe_url, style: "color: #9ca3af; text-decoration: underline;" %></p>
<% end %>
<p class="my-1">15 Falls Road, Shelburne, VT 05482, United States</p>
</div>
</td>

1
config/initializers/mailkick.rb
View file

@ -1 +0,0 @@
Mailkick.headers = true

19
db/migrate/20260301111352_create_notable_requests.rb
View file

@ -1,19 +0,0 @@
class CreateNotableRequests < ActiveRecord::Migration[8.1]
def change
create_table :notable_requests do |t|
t.string :note_type
t.text :note
t.references :user, polymorphic: true
t.text :action
t.integer :status
t.text :url
t.string :request_id
t.string :ip
t.text :user_agent
t.text :referrer
t.text :params
t.float :request_time
t.datetime :created_at
end
end
end

14
db/migrate/20260301111406_create_notable_jobs.rb
View file

@ -1,14 +0,0 @@
class CreateNotableJobs < ActiveRecord::Migration[8.1]
def change
create_table :notable_jobs do |t|
t.string :note_type
t.text :note
t.text :job
t.string :job_id
t.string :queue
t.float :runtime
t.float :queued_time
t.datetime :created_at
end
end
end

11
db/migrate/20260301112503_create_mailkick_subscriptions.rb
View file

@ -1,11 +0,0 @@
class CreateMailkickSubscriptions < ActiveRecord::Migration[8.1]
def change
create_table :mailkick_subscriptions do |t|
t.references :subscriber, polymorphic: true, index: false
t.string :list
t.timestamps
end
add_index :mailkick_subscriptions, [ :subscriber_type, :subscriber_id, :list ], unique: true, name: "index_mailkick_subscriptions_on_subscriber_and_list"
end
end

17
db/migrate/20260301112514_seed_mailkick_subscriptions_from_weekly_summary.rb
View file

@ -1,17 +0,0 @@
class SeedMailkickSubscriptionsFromWeeklySummary < ActiveRecord::Migration[8.1]
def up
execute <<~SQL
INSERT INTO mailkick_subscriptions (subscriber_type, subscriber_id, list, created_at, updated_at)
SELECT 'User', id, 'weekly_summary', NOW(), NOW()
FROM users
WHERE weekly_summary_email_enabled = TRUE
ON CONFLICT (subscriber_type, subscriber_id, list) DO NOTHING
SQL
end
def down
execute <<~SQL
DELETE FROM mailkick_subscriptions WHERE list = 'weekly_summary'
SQL
end
end

5
db/migrate/20260301112933_remove_weekly_summary_email_enabled_from_users.rb
View file

@ -1,5 +0,0 @@
class RemoveWeeklySummaryEmailEnabledFromUsers < ActiveRecord::Migration[8.1]
def change
remove_column :users, :weekly_summary_email_enabled, :boolean, default: true, null: false
end
end

2
db/migrate/20260301125356_add_weekly_summary_email_enabled_back_to_users.rb
View file

@ -1,5 +1,5 @@
class AddWeeklySummaryEmailEnabledBackToUsers < ActiveRecord::Migration[8.1]
def change
add_column :users, :weekly_summary_email_enabled, :boolean, default: false, null: false
add_column :users, :weekly_summary_email_enabled, :boolean, default: false, null: false, if_not_exists: true
end
end

42
db/schema.rb generated
View file

@ -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_03_01_125356) do
ActiveRecord::Schema[8.1].define(version: 2026_02_23_212054) 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,44 +410,6 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_01_125356) 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
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
@ -685,7 +647,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_01_125356) 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: false, null: false
t.boolean "weekly_summary_email_enabled", default: true, 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

5
test/controllers/settings_notifications_controller_test.rb
View file

@ -17,13 +17,10 @@ class SettingsNotificationsControllerTest < ActionDispatch::IntegrationTest
user = users(:one)
sign_in_as(user)
user.subscribe("weekly_summary")
assert user.subscribed?("weekly_summary")
patch my_settings_notifications_path, params: { user: { weekly_summary_email_enabled: "0" } }
assert_response :redirect
assert_redirected_to my_settings_notifications_path
assert_not user.reload.subscribed?("weekly_summary")
assert_equal false, user.reload.weekly_summary_email_enabled
end
end

47
test/jobs/weekly_summary_email_job_test.rb
View file

@ -5,53 +5,6 @@ class WeeklySummaryEmailJobTest < ActiveJob::TestCase
setup do
ActionMailer::Base.deliveries.clear
Flipper.enable(:weekly_summary_emails)
GoodJob::Job.delete_all
end
teardown do
Flipper.disable(:weekly_summary_emails)
GoodJob::Job.delete_all
end
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
recent_signup = User.create!(timezone: "UTC")
recent_signup.update_column(:created_at, cutoff + 1.hour)
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")
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")
unsubscribed_recent_coder = User.create!(timezone: "UTC")
unsubscribed_recent_coder.unsubscribe("weekly_summary")
create_coding_heartbeat(unsubscribed_recent_coder, cutoff + 3.hours, "unsubscribed", "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 enqueue summaries when feature flag is disabled" do
Flipper.disable(:weekly_summary_emails)
User.create!(timezone: "UTC")
assert_no_difference -> { GoodJob::Job.where(job_class: "WeeklySummaryUserEmailJob").count } do
WeeklySummaryEmailJob.perform_now(Time.utc(2026, 3, 1, 12, 0, 0))
end
end
test "sends weekly summaries only for opted-in users with email at friday 17:30 utc" do

4
test/mailers/weekly_summary_mailer_test.rb
View file

@ -3,9 +3,9 @@ require "test_helper"
class WeeklySummaryMailerTest < ActionMailer::TestCase
setup do
@user = User.create!(
timezone: "UTC"
timezone: "UTC",
weekly_summary_email_enabled: true
)
@user.subscribe("weekly_summary")
@recipient_email = "weekly-mailer-#{SecureRandom.hex(4)}@example.com"
@user.email_addresses.create!(email: @recipient_email, source: :signing_in)
end

4
test/system/settings/notifications_settings_test.rb
View file

@ -19,7 +19,7 @@ class NotificationsSettingsTest < ApplicationSystemTestCase
end
test "notifications settings updates weekly summary email preference" do
@user.subscribe("weekly_summary")
@user.update!(weekly_summary_email_enabled: true)
visit my_settings_notifications_path
@ -30,6 +30,6 @@ class NotificationsSettingsTest < ApplicationSystemTestCase
click_on "Save notification settings"
assert_text "Settings updated successfully"
assert_not @user.reload.subscribed?("weekly_summary")
assert_equal false, @user.reload.weekly_summary_email_enabled
end
end