Merge pull request #1524 from codidact/cellio/1248-generalize-profile…

…-links

generalize profile links

Gemfile

@@ -100,3 +100,5 @@ group :development do
   gem 'spring', '~> 4.0'
   gem 'web-console', '~> 4.2'
 end
+
+gem 'maintenance_tasks', '~> 2.1.1'

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)

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

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

app/models/user_website.rb

@@ -0,0 +1,6 @@
+class UserWebsite < ApplicationRecord
+  belongs_to :user
+  default_scope { order(:position) }
+
+  MAX_ROWS = 3
+end

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

app/views/users/edit_profile.html.erb

@@ -46,26 +46,29 @@
   <% end %>
   <div class="post-preview"></div>
 
-  <div class="grid">
-    <div class="grid--cell is-4 is-12-sm form-group">
-      <%= f.label :website, class: "form-element" %>
-      <span class="form-caption">A link to anywhere on the internet for your stuff.</span>
-      <%= f.text_field :website, class: 'form-element', autocomplete: 'off', placeholder: 'https://...' %>
-    </div>
-
-    <div class="grid--cell is-4 is-12-sm form-group">
-      <%= f.label :twitter, class: "form-element" %>
-      <span class="form-caption">Your Twitter username, if you've got one you want to share.</span>
-      <%= f.text_field :twitter, class: 'form-element', autocomplete: 'off', placeholder: '@username' %>
-    </div>
-
-    <div class="grid--cell is-4 is-12-sm form-group">
-      <%= f.label :discord, class: 'form-element' %>
-      <span class="form-caption">Your Discord user tag, <code>username</code> or <code>username#1234</code>.</span>
-      <%= f.text_field :discord, class: 'form-element', autocomplete: 'off', placeholder: 'username#1234' %>
+  <div>
+    <p>Extra fields -- your web site, GitHub profile, social-media usernames, whatever you want. Only values that begin with "http" are rendered as links.</p>
+    <div class="grid">
+      <%= f.fields_for :user_websites do |w| %>
+      <div class="grid grid--cell is-12 is-12-sm">
+        <div class="grid grid--cell is-3 is-3-sm">
+          <div class="grid--cell is-12"><%= w.text_field :label, class: 'form-element', autocomplete: 'off', placeholder: 'label' %></div>
+        </div>
+        <div class="grid grid--cell is-6 is-9-sm">
+          <div class="grid--cell is-12"><%= w.text_field :url, class: 'form-element', autocomplete: 'off', placeholder: 'https://...' %></div>
+        </div>
+      </div>
+      <% end %>
     </div>
   </div>
 
+  <div class="form-group has-padding-2">
+    <%= f.label :discord, class: 'form-element' %>
+    <span class="form-caption">Your Discord user tag, <code>username</code> or <code>username#1234</code>.</span>
+    <%= f.text_field :discord, class: 'form-element', autocomplete: 'off', placeholder: 'username#1234' %>
+  </div>
+
+
   <%= f.submit 'Save', class: 'button is-filled' %>
 <% end %>
 

app/views/users/show.html.erb

@@ -24,39 +24,47 @@
   <div class="grid--cell is-9-lg is-12">
     <div class="h-p-0 h-p-t-0">
       <div class="profile-text">
-      <p>
-        <% if @user.website.present? %>
-  	  <% unless !user_signed_in? && !@user.community_user.privilege?('unrestricted') %>
-          <span class="h-m-r-4">
-            <i class="fas fa-link"></i>
-	    <%= link_to @user.website_domain, @user.website, rel: 'nofollow',
-                'aria-label': "Visit website of #{rtl_safe_username(@user)} at #{@user.website_domain}" %>
-          </span>
-          <% end %>
-        <% end %>
-        <% if @user.twitter.present? %>
-          <span class="h-m-r-4">
-            <i class="fab fa-twitter"></i> <%= link_to @user.twitter, "https://twitter.com/#{@user.twitter}",
-                                                       'aria-label': "Visit twitter account of #{rtl_safe_username(@user)}" %>
-          </span>
-        <% end %>
-        <% if @user.discord.present? %>
-          <span class="h-m-r-4">
-            <i class="fab fa-discord h-m-r-1"></i> <%= @user.discord %>
-          </span>
-        <% end %>
-      </p>
 
       <% effective_profile = raw(sanitize(@user.profile&.strip || '', scrubber: scrubber)) %>
       <% if effective_profile.blank? %>
         <p class="is-lead">A quiet enigma. We don't know anything about <span dir="ltr"><%= rtl_safe_username(@user) %></span> yet.</p>
-	<% 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 %>
       </div>
 
+      <% unless !user_signed_in? && !@user.community_user.privilege?('unrestricted') %>
+        <% if @user.valid_websites_for.size.positive? %>
+        <div>
+          <p><strong>Extra fields</strong></p>
+          <table class="table is-with-hover">
+            <% @user.valid_websites_for.each do |w| %>
+            <tr>
+              <td><%= w.label %></td>
+              <td>
+              <% if w.url[0,4] == 'http' %>
+                <%= link_to w.url, w.url, rel: 'nofollow' %>
+              <% else %>
+              <%= w.url %>
+              <% end %>
+              </td>
+            </tr>
+            <% end %>
+          </table>
+        </div>
+        <% end %>
+      <% end %>
+
+      <p>
+        <% if @user.discord.present? %>
+          <span class="h-m-r-4">
+            <i class="fab fa-discord h-m-r-1"></i> <%= @user.discord %>
+          </span>
+        <% end %>
+      </p>
+
       <div class="button-list h-p-2">
         <% 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 %>
@@ -83,7 +91,7 @@
             </div>
           </div>
         <% 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 %>
             <i class="fas fa-mobile-alt"></i> Mobile Sign In
           <% end %>
@@ -160,7 +168,7 @@
     <% if current_user&.id == @user.id || current_user&.is_moderator %>
     <table class="table is-full-width">
       <tr>
-	<td>User since <%= @user.created_at %></td>
+        <td>User since <%= @user.created_at %></td>
       </tr>
     </table>
     <% end %>
@@ -170,7 +178,7 @@
 
       <div class="widget">
         <% @abilities.each do |a| %>
-	<% if @user.privilege?(a.internal_id) %>
+        <% if @user.privilege?(a.internal_id) %>
           <div class="widget--body" title="<%= a.summary %>">
             <i class="fa-fw fas fa-<%= a.icon %>"></i>
             <a href="<%= ability_url(a.internal_id, for: @user.id) %>">

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

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

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