diff --git a/Gemfile b/Gemfile index c20323811..b09b258fd 100644 --- a/Gemfile +++ b/Gemfile @@ -100,3 +100,5 @@ group :development do gem 'spring', '~> 4.0' gem 'web-console', '~> 4.2' end + +gem 'maintenance_tasks', '~> 2.1.1' diff --git a/Gemfile.lock b/Gemfile.lock index 52d4d8c7f..47bccbcc9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -164,6 +164,8 @@ GEM actionview (>= 5.0.0) activesupport (>= 5.0.0) jmespath (1.6.1) + job-iteration (1.3.6) + activejob (>= 5.2) jquery-rails (4.5.0) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) @@ -189,6 +191,12 @@ GEM net-imap net-pop net-smtp + maintenance_tasks (2.1.1) + actionpack (>= 6.0) + activejob (>= 6.0) + activerecord (>= 6.0) + job-iteration (~> 1.3.6) + railties (>= 6.0) marcel (1.0.4) matrix (0.4.2) memory_profiler (1.0.0) @@ -415,6 +423,7 @@ DEPENDENCIES jquery-rails (~> 4.5.0) letter_opener_web (~> 2.0) listen (~> 3.7) + maintenance_tasks (~> 2.1.1) memory_profiler (~> 1.0) minitest (~> 5.16.0) minitest-ci (~> 3.4.0) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index bcd8aa87e..1b4aec6f0 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -355,25 +355,24 @@ def edit_profile render layout: 'without_sidebar' end - def validate_profile_website(profile_params) - uri = profile_params[:website] + def cleaned_profile_websites(profile_params) + sites = profile_params[:user_websites_attributes] - if URI.parse(uri).instance_of?(URI::Generic) - # URI::Generic indicates the user didn't include a protocol, so we'll add one now so that it can be - # parsed correctly in the view later on. - profile_params[:website] = "https://#{uri}" + sites.transform_values do |w| + w.merge({ label: w[:label].presence, url: w[:url].presence }) end - rescue URI::InvalidURIError - profile_params.delete(:website) - flash[:danger] = 'Invalid profile website link.' end def update_profile - profile_params = params.require(:user).permit(:username, :profile_markdown, :website, :twitter, :discord) - profile_params[:twitter] = profile_params[:twitter].delete('@') + profile_params = params.require(:user).permit(:username, + :profile_markdown, + :website, + :discord, + :twitter, + user_websites_attributes: [:id, :label, :url]) - if profile_params[:website].present? - validate_profile_website(profile_params) + if profile_params[:user_websites_attributes].present? + profile_params[:user_websites_attributes] = cleaned_profile_websites(profile_params) end @user = current_user @@ -389,8 +388,14 @@ def update_profile end end - profile_rendered = helpers.post_markdown(:user, :profile_markdown) - if @user.update(profile_params.merge(profile: profile_rendered)) + if params[:user][:profile_markdown].present? + profile_rendered = helpers.post_markdown(:user, :profile_markdown) + profile_params = profile_params.merge(profile: profile_rendered) + end + + status = @user.update(profile_params) + + if status flash[:success] = 'Your profile details were updated.' redirect_to user_path(current_user) else diff --git a/app/models/user.rb b/app/models/user.rb index 46a27df1a..d585a9818 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -27,6 +27,8 @@ class User < ApplicationRecord has_many :comment_threads_locked, class_name: 'CommentThread', foreign_key: :locked_by_id, dependent: :nullify has_many :category_filter_defaults, dependent: :destroy has_many :filters, dependent: :destroy + has_many :user_websites, dependent: :destroy + accepts_nested_attributes_for :user_websites belongs_to :deleted_by, required: false, class_name: 'User' validates :username, presence: true, length: { minimum: 3, maximum: 50 } @@ -43,7 +45,7 @@ class User < ApplicationRecord scope :active, -> { where(deleted: false) } scope :deleted, -> { where(deleted: true) } - after_create :send_welcome_tour_message + after_create :send_welcome_tour_message, :ensure_websites def self.list_includes includes(:posts, :avatar_attachment) @@ -61,6 +63,12 @@ def trust_level community_user.trust_level end + # Checks whether this user is the same as a given user + # @param [User] user user to compare with + def same_as?(user) + id == user.id + end + # This class makes heavy use of predicate names, and their use is prevalent throughout the codebase # because of the importance of these methods. # rubocop:disable Naming/PredicateName @@ -130,6 +138,18 @@ def website_domain website.nil? ? website : URI.parse(website).hostname end + def valid_websites_for + user_websites.where.not(url: [nil, '']).order(position: :asc) + end + + def ensure_websites + pos = user_websites.size + while pos < UserWebsite::MAX_ROWS + pos += 1 + UserWebsite.create(user_id: id, position: pos) + end + end + def is_moderator is_global_moderator || community_user&.is_moderator || is_admin || community_user&.privilege?('mod') || false end diff --git a/app/models/user_website.rb b/app/models/user_website.rb new file mode 100644 index 000000000..6b50d909f --- /dev/null +++ b/app/models/user_website.rb @@ -0,0 +1,6 @@ +class UserWebsite < ApplicationRecord + belongs_to :user + default_scope { order(:position) } + + MAX_ROWS = 3 +end diff --git a/app/tasks/maintenance/initialize_user_websites_task.rb b/app/tasks/maintenance/initialize_user_websites_task.rb new file mode 100644 index 000000000..80a91b948 --- /dev/null +++ b/app/tasks/maintenance/initialize_user_websites_task.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Maintenance + class InitializeUserWebsitesTask < MaintenanceTasks::Task + def collection + User.all + end + + def process(user) + unless user.user_websites.exists?(position: 1) + if user.website.present? + UserWebsite.create!(user_id: user.id, position: 1, label: 'website', url: user.website) + else + UserWebsite.create!(user_id: user.id, position: 1) + end + end + + unless user.user_websites.exists?(position: 2) + if user.twitter.present? + UserWebsite.create!(user_id: user.id, position: 2, label: 'Twitter', + url: "https://twitter.com/#{user.twitter}") + else + UserWebsite.create!(user_id: user.id, position: 2) + end + end + + # This check *should* be superfluous, but just in case... + unless user.user_websites.exists?(position: 3) + UserWebsite.create!(user_id: user.id, position: 3) + end + end + end +end diff --git a/app/views/users/edit_profile.html.erb b/app/views/users/edit_profile.html.erb index 4dbe1d2da..654a2ef29 100644 --- a/app/views/users/edit_profile.html.erb +++ b/app/views/users/edit_profile.html.erb @@ -46,26 +46,29 @@ <% end %>
-
-
- <%= f.label :website, class: "form-element" %> - A link to anywhere on the internet for your stuff. - <%= f.text_field :website, class: 'form-element', autocomplete: 'off', placeholder: 'https://...' %> -
- -
- <%= f.label :twitter, class: "form-element" %> - Your Twitter username, if you've got one you want to share. - <%= f.text_field :twitter, class: 'form-element', autocomplete: 'off', placeholder: '@username' %> -
- -
- <%= f.label :discord, class: 'form-element' %> - Your Discord user tag, username or username#1234. - <%= f.text_field :discord, class: 'form-element', autocomplete: 'off', placeholder: 'username#1234' %> +
+

Extra fields -- your web site, GitHub profile, social-media usernames, whatever you want. Only values that begin with "http" are rendered as links.

+
+ <%= f.fields_for :user_websites do |w| %> +
+
+
<%= w.text_field :label, class: 'form-element', autocomplete: 'off', placeholder: 'label' %>
+
+
+
<%= w.text_field :url, class: 'form-element', autocomplete: 'off', placeholder: 'https://...' %>
+
+
+ <% end %>
+
+ <%= f.label :discord, class: 'form-element' %> + Your Discord user tag, username or username#1234. + <%= f.text_field :discord, class: 'form-element', autocomplete: 'off', placeholder: 'username#1234' %> +
+ + <%= f.submit 'Save', class: 'button is-filled' %> <% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index bb10e0ec4..e6ff79907 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -24,39 +24,47 @@
-

- <% if @user.website.present? %> - <% unless !user_signed_in? && !@user.community_user.privilege?('unrestricted') %> - - - <%= link_to @user.website_domain, @user.website, rel: 'nofollow', - 'aria-label': "Visit website of #{rtl_safe_username(@user)} at #{@user.website_domain}" %> - - <% end %> - <% end %> - <% if @user.twitter.present? %> - - <%= link_to @user.twitter, "https://twitter.com/#{@user.twitter}", - 'aria-label': "Visit twitter account of #{rtl_safe_username(@user)}" %> - - <% end %> - <% if @user.discord.present? %> - - <%= @user.discord %> - - <% end %> -

<% effective_profile = raw(sanitize(@user.profile&.strip || '', scrubber: scrubber)) %> <% if effective_profile.blank? %>

A quiet enigma. We don't know anything about <%= rtl_safe_username(@user) %> yet.

- <% elsif !user_signed_in? && !@user.community_user.privilege?('unrestricted') %> - <%= sanitize(effective_profile, attributes: %w()) %> - <% else %> + <% elsif !user_signed_in? && !@user.community_user.privilege?('unrestricted') %> + <%= sanitize(effective_profile, attributes: %w()) %> + <% else %> <%= effective_profile %> <% end %>
+ <% unless !user_signed_in? && !@user.community_user.privilege?('unrestricted') %> + <% if @user.valid_websites_for.size.positive? %> +
+

Extra fields

+ + <% @user.valid_websites_for.each do |w| %> + + + + + <% end %> +
<%= w.label %> + <% if w.url[0,4] == 'http' %> + <%= link_to w.url, w.url, rel: 'nofollow' %> + <% else %> + <%= w.url %> + <% end %> +
+
+ <% end %> + <% end %> + +

+ <% if @user.discord.present? %> + + <%= @user.discord %> + + <% end %> +

+
<% if user_signed_in? %> <%= link_to new_subscription_path(type: 'user', qualifier: @user.id, return_to: request.path), class: "button is-outlined is-small" do %> @@ -79,7 +87,7 @@
<% end %> - <% if current_user&.id == @user.id %> + <% if current_user&.same_as?(@user) %> <%= link_to qr_login_code_path, class: 'button is-outlined is-small' do %> Mobile Sign In <% end %> @@ -156,7 +164,7 @@ <% if current_user&.id == @user.id || current_user&.is_moderator %> - +
User since <%= @user.created_at %>User since <%= @user.created_at %>
<% end %> @@ -166,7 +174,7 @@
<% @abilities.each do |a| %> - <% if @user.privilege?(a.internal_id) %> + <% if @user.privilege?(a.internal_id) %>
diff --git a/config/routes.rb b/config/routes.rb index 8b8b6b37b..bf646a447 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ Rails.application.routes.draw do + mount MaintenanceTasks::Engine, at: "/maintenance_tasks" # Add normal sign in/sign up, confirmations, registrations, unlocking and password editing routes only if no SSO or mixed sign in. devise_for :users, only: %i[sessions registrations confirmations unlock passwords], controllers: { sessions: 'users/sessions', registrations: 'users/registrations' }, diff --git a/db/migrate/20250123141400_create_user_websites.rb b/db/migrate/20250123141400_create_user_websites.rb new file mode 100644 index 000000000..ec77b2d17 --- /dev/null +++ b/db/migrate/20250123141400_create_user_websites.rb @@ -0,0 +1,11 @@ +class CreateUserWebsites < ActiveRecord::Migration[7.0] + def change + create_table :user_websites do |t| + t.column :label, :string, limit:80 + t.string :url + t.integer :position + end + add_reference :user_websites, :user, null: false, foreign_key: true + add_index(:user_websites, [:user_id, :url], unique: true) + end +end diff --git a/db/migrate/20250128030354_create_maintenance_tasks_runs.maintenance_tasks.rb b/db/migrate/20250128030354_create_maintenance_tasks_runs.maintenance_tasks.rb new file mode 100644 index 000000000..c9b215d68 --- /dev/null +++ b/db/migrate/20250128030354_create_maintenance_tasks_runs.maintenance_tasks.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20201211151756) +class CreateMaintenanceTasksRuns < ActiveRecord::Migration[6.0] + def change + create_table(:maintenance_tasks_runs) do |t| + t.string(:task_name, null: false) + t.datetime(:started_at) + t.datetime(:ended_at) + t.float(:time_running, default: 0.0, null: false) + t.integer(:tick_count, default: 0, null: false) + t.integer(:tick_total) + t.string(:job_id) + t.bigint(:cursor) + t.string(:status, default: :enqueued, null: false) + t.string(:error_class) + t.string(:error_message) + t.text(:backtrace) + t.timestamps + t.index(:task_name) + t.index([:task_name, :created_at], order: { created_at: :desc }) + end + end +end diff --git a/db/migrate/20250128030355_change_cursor_to_string.maintenance_tasks.rb b/db/migrate/20250128030355_change_cursor_to_string.maintenance_tasks.rb new file mode 100644 index 000000000..4e414cad3 --- /dev/null +++ b/db/migrate/20250128030355_change_cursor_to_string.maintenance_tasks.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20210219212931) +class ChangeCursorToString < ActiveRecord::Migration[6.0] + # This migration will clear all existing data in the cursor column with MySQL. + # Ensure no Tasks are paused when this migration is deployed, or they will be resumed from the start. + # Running tasks are able to gracefully handle this change, even if interrupted. + def up + change_table(:maintenance_tasks_runs) do |t| + t.change(:cursor, :string) + end + end + + def down + change_table(:maintenance_tasks_runs) do |t| + t.change(:cursor, :bigint) + end + end +end diff --git a/db/migrate/20250128030356_remove_index_on_task_name.maintenance_tasks.rb b/db/migrate/20250128030356_remove_index_on_task_name.maintenance_tasks.rb new file mode 100644 index 000000000..1b5cadf8d --- /dev/null +++ b/db/migrate/20250128030356_remove_index_on_task_name.maintenance_tasks.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20210225152418) +class RemoveIndexOnTaskName < ActiveRecord::Migration[6.0] + def up + change_table(:maintenance_tasks_runs) do |t| + t.remove_index(:task_name) + end + end + + def down + change_table(:maintenance_tasks_runs) do |t| + t.index(:task_name) + end + end +end diff --git a/db/migrate/20250128030357_add_arguments_to_maintenance_tasks_runs.maintenance_tasks.rb b/db/migrate/20250128030357_add_arguments_to_maintenance_tasks_runs.maintenance_tasks.rb new file mode 100644 index 000000000..8478268eb --- /dev/null +++ b/db/migrate/20250128030357_add_arguments_to_maintenance_tasks_runs.maintenance_tasks.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20210517131953) +class AddArgumentsToMaintenanceTasksRuns < ActiveRecord::Migration[6.0] + def change + add_column(:maintenance_tasks_runs, :arguments, :text) + end +end diff --git a/db/migrate/20250128030358_add_lock_version_to_maintenance_tasks_runs.maintenance_tasks.rb b/db/migrate/20250128030358_add_lock_version_to_maintenance_tasks_runs.maintenance_tasks.rb new file mode 100644 index 000000000..b302565af --- /dev/null +++ b/db/migrate/20250128030358_add_lock_version_to_maintenance_tasks_runs.maintenance_tasks.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20211210152329) +class AddLockVersionToMaintenanceTasksRuns < ActiveRecord::Migration[6.0] + def change + add_column( + :maintenance_tasks_runs, + :lock_version, + :integer, + default: 0, + null: false, + ) + end +end diff --git a/db/migrate/20250128030359_change_runs_tick_columns_to_bigints.maintenance_tasks.rb b/db/migrate/20250128030359_change_runs_tick_columns_to_bigints.maintenance_tasks.rb new file mode 100644 index 000000000..064b07705 --- /dev/null +++ b/db/migrate/20250128030359_change_runs_tick_columns_to_bigints.maintenance_tasks.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20220706101937) +class ChangeRunsTickColumnsToBigints < ActiveRecord::Migration[6.0] + def up + change_table(:maintenance_tasks_runs, bulk: true) do |t| + t.change(:tick_count, :bigint) + t.change(:tick_total, :bigint) + end + end + + def down + change_table(:maintenance_tasks_runs, bulk: true) do |t| + t.change(:tick_count, :integer) + t.change(:tick_total, :integer) + end + end +end diff --git a/db/migrate/20250128030360_add_index_on_task_name_and_status_to_runs.maintenance_tasks.rb b/db/migrate/20250128030360_add_index_on_task_name_and_status_to_runs.maintenance_tasks.rb new file mode 100644 index 000000000..74d77e6be --- /dev/null +++ b/db/migrate/20250128030360_add_index_on_task_name_and_status_to_runs.maintenance_tasks.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20220713131925) +class AddIndexOnTaskNameAndStatusToRuns < ActiveRecord::Migration[6.0] + def change + remove_index( + :maintenance_tasks_runs, + column: [:task_name, :created_at], + order: { created_at: :desc }, + name: :index_maintenance_tasks_runs_on_task_name_and_created_at, + ) + + add_index( + :maintenance_tasks_runs, + [:task_name, :status, :created_at], + name: :index_maintenance_tasks_runs, + order: { created_at: :desc }, + ) + end +end diff --git a/db/migrate/20250128030361_add_metadata_to_runs.maintenance_tasks.rb b/db/migrate/20250128030361_add_metadata_to_runs.maintenance_tasks.rb new file mode 100644 index 000000000..054ef0e2a --- /dev/null +++ b/db/migrate/20250128030361_add_metadata_to_runs.maintenance_tasks.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# This migration comes from maintenance_tasks (originally 20230622035229) +class AddMetadataToRuns < ActiveRecord::Migration[6.0] + def change + add_column(:maintenance_tasks_runs, :metadata, :text) + end +end diff --git a/db/schema.rb b/db/schema.rb index d71e83045..97e097dba 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[7.0].define(version: 2024_10_20_193053) do +ActiveRecord::Schema[7.0].define(version: 2025_01_28_030361) do create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "community_id" t.string "name" @@ -309,6 +309,27 @@ t.index ["name"], name: "index_licenses_on_name" end + create_table "maintenance_tasks_runs", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.string "task_name", null: false + t.datetime "started_at", precision: nil + t.datetime "ended_at", precision: nil + t.float "time_running", default: 0.0, null: false + t.bigint "tick_count", default: 0, null: false + t.bigint "tick_total" + t.string "job_id" + t.string "cursor" + t.string "status", default: "enqueued", null: false + t.string "error_class" + t.string "error_message" + t.text "backtrace" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.text "arguments" + t.integer "lock_version", default: 0, null: false + t.text "metadata" + t.index ["task_name", "status", "created_at"], name: "index_maintenance_tasks_runs", order: { created_at: :desc } + end + create_table "micro_auth_apps", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "name" t.string "app_id" @@ -684,6 +705,15 @@ t.index ["community_user_id"], name: "index_user_abilities_on_community_user_id" end + create_table "user_websites", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.string "label", limit: 80 + t.string "url" + t.integer "position" + t.bigint "user_id", null: false + t.index ["user_id", "url"], name: "index_user_websites_on_user_id_and_url", unique: true + t.index ["user_id"], name: "index_user_websites_on_user_id" + end + create_table "users", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "email" t.string "encrypted_password" @@ -826,6 +856,7 @@ add_foreign_key "thread_followers", "posts" add_foreign_key "user_abilities", "abilities" add_foreign_key "user_abilities", "community_users" + add_foreign_key "user_websites", "users" add_foreign_key "users", "users", column: "deleted_by_id" add_foreign_key "votes", "communities" add_foreign_key "warning_templates", "communities" diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 17a18c257..0aa5d0863 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -113,16 +113,41 @@ class UsersControllerTest < ActionController::TestCase assert_response 200 end - test 'should update profile text' do + test 'should redirect & show success notice on profile update' do sign_in users(:standard_user) - patch :update_profile, params: { user: { profile_markdown: 'ABCDEF GHIJKL', website: 'https://example.com/user', - twitter: '@standard_user' } } + patch :update_profile, params: { user: { username: 'std' } } assert_response 302 assert_not_nil flash[:success] assert_not_nil assigns(:user) assert_equal users(:standard_user).id, assigns(:user).id - assert_not_nil assigns(:user).profile - assert_equal 'standard_user', assigns(:user).twitter + end + + test 'should update profile text' do + sign_in users(:standard_user) + patch :update_profile, params: { + user: { profile_markdown: 'ABCDEF GHIJKL' } + } + assert_equal assigns(:user).profile.strip, '

ABCDEF GHIJKL

' + end + + test 'should update websites' do + sign_in users(:standard_user) + patch :update_profile, params: { + user: { user_websites_attributes: { + '0': { label: 'web', url: 'example.com' } + } } + } + assert_not_nil assigns(:user).user_websites + assert_equal 'web', assigns(:user).user_websites.first.label + assert_equal 'example.com', assigns(:user).user_websites.first.url + end + + test 'should update user discord link' do + sign_in users(:standard_user) + patch :update_profile, params: { + user: { discord: 'example_user#1234' } + } + assert_equal 'example_user#1234', assigns(:user).discord end test 'should get full posts list for a user' do