add proactive edit locking for issues

CHANGELOG

@@ -1,6 +1,5 @@
 [v#.#.#] ([month] [YYYY])
-  - [entity]:
-    - [future tense verb] [feature]
+  - Issues: prevent editing a record while another user is already editing it
   - Upgraded gems:
     - rails-html-sanitizer, sqlite3, websocket-driver
   - Bugs fixes:

app/assets/stylesheets/hera/modules.scss

@@ -8,6 +8,7 @@
 @import 'hera/modules/divider';
 @import 'hera/modules/dropdown';
 @import 'hera/modules/dots_menu';
+@import 'hera/modules/edit_locked';
 @import 'hera/modules/editor_toolbar';
 @import 'hera/modules/icons';
 @import 'hera/modules/inline_editable';

app/assets/stylesheets/hera/modules/_edit_locked.scss

@@ -0,0 +1,85 @@
+.edit-locked {
+  padding: 3rem 2rem;
+  text-align: center;
+
+  .actions {
+    display: flex;
+    gap: 0.75rem;
+    justify-content: center;
+  }
+
+  .description {
+    color: var(--text-muted);
+    font-size: 1rem;
+    line-height: 1.6;
+    margin: 0 auto 2rem;
+    max-width: 28rem;
+  }
+
+  .editor {
+    align-items: center;
+    background: var(--secondary-bg);
+    border-radius: 0.5rem;
+    display: flex;
+    gap: 0.75rem;
+    justify-content: center;
+    margin: 0 auto 2rem;
+    max-width: 22rem;
+    padding: 1rem 1.5rem;
+  }
+
+  .editor-info {
+    text-align: left;
+  }
+
+  .editor-name {
+    display: block;
+    font-size: 1rem;
+    font-weight: 600;
+  }
+
+  .editor-status {
+    align-items: center;
+    color: var(--text-muted);
+    display: flex;
+    font-size: 0.85rem;
+    gap: 0.35rem;
+  }
+
+  .heading {
+    color: var(--text-default);
+    font-size: 1.4rem;
+    font-weight: 700;
+    margin-bottom: 0.75rem;
+  }
+
+  .icon {
+    align-items: center;
+    background: $orange-100;
+    border-radius: 50%;
+    display: flex;
+    height: 4.5rem;
+    justify-content: center;
+    margin: 0 auto 1.5rem;
+    width: 4.5rem;
+
+    i {
+      color: $orange-500;
+      font-size: 1.8rem;
+    }
+  }
+
+  .pulse {
+    animation: edit-locked-pulse 2s infinite;
+    background: var(--brand-bg);
+    border-radius: 50%;
+    display: inline-block;
+    height: 0.5rem;
+    width: 0.5rem;
+  }
+}
+
+@keyframes edit-locked-pulse {
+  0%, 100% { opacity: 1; }
+  50% { opacity: 0.4; }
+}

app/controllers/concerns/edit_lockable.rb

@@ -0,0 +1,34 @@
+module EditLockable
+  extend ActiveSupport::Concern
+
+  protected
+
+  def acquire_edit_session(record)
+    EditingSession.acquire(record_type: record.class.name, record_id: record.id, user: current_user)
+  end
+
+  def check_edit_lock
+    record = lockable_record
+    competing_sessions = EditingSession.for_record(record).active.by_others(current_user)
+
+    if competing_sessions.any? && params[:force] != 'true'
+      @locked_by = competing_sessions.includes(:user).map(&:user)
+      @locked_record = record
+      @back_path = url_from(request.referer) || root_path
+      render 'shared/edit_locked'
+      return
+    end
+
+    acquire_edit_session(record)
+  end
+
+  def lockable_record
+    @lockable_record ||=
+      instance_variable_get("@#{controller_name.singularize}") ||
+        send("set_or_initialize_#{controller_name.singularize}")
+  end
+
+  def release_edit_session(record)
+    EditingSession.for_record(record).where(user: current_user).destroy_all
+  end
+end

app/controllers/issues_controller.rb

@@ -2,6 +2,7 @@ class IssuesController < AuthenticatedController
   include ConflictResolver
   include ContentFromTemplate
   include DynamicFieldNamesCacher
+  include EditLockable
   include EventPublisher
   include IssuesHelper
   include LiquidEnabledResource
@@ -16,6 +17,7 @@ class IssuesController < AuthenticatedController
   before_action :set_columns, only: :index
 
   before_action :set_or_initialize_issue, except: [:import, :index]
+  before_action :check_edit_lock, only: :edit
   before_action :set_auto_save_key, only: [:new, :create, :edit, :update]
   before_action :set_affected_nodes, only: [:show]
   before_action :set_form_cancel_path, only: [:new, :edit]
@@ -31,8 +33,8 @@ def show
                         .group('nodes.id')
                         .sort_by { |node, _| node.label }
 
-    @first_node      = @affected_nodes.first
-    @first_evidence  = Evidence.where(node: @first_node, issue: @issue)
+    @first_node = @affected_nodes.first
+    @first_evidence = Evidence.where(node: @first_node, issue: @issue)
 
     load_conflicting_revisions(@issue)
   end
@@ -81,6 +83,7 @@ def update
       updated_at_before_save = @issue.updated_at.to_i
 
       if @issue.update(issue_params)
+        release_edit_session(@issue)
         @modified = true
         check_for_edit_conflicts(@issue, updated_at_before_save)
         format.html { redirect_to_main_or_qa }

app/models/configuration.rb

@@ -15,6 +15,11 @@ class Configuration < ApplicationRecord
   # -- Class Methods --------------------------------------------------------
   # --------------------------------------------------------------- Misc admin:
 
+  def self.editing_session_stale_after
+    create_with(value: 1440)
+      .find_or_create_by(name: 'admin:editing_session_stale_after').value.to_i
+  end
+
   def self.max_deleted_inline
     create_with(value: 15)
       .find_or_create_by(name: 'admin:max_deleted_inline').value.to_i

app/models/editing_session.rb

@@ -0,0 +1,28 @@
+class EditingSession < ApplicationRecord
+  ALLOWED_RECORD_TYPES = %w[Issue].freeze
+
+  belongs_to :user
+  belongs_to :record, polymorphic: true
+
+  validates :record_type, presence: true, inclusion: { in: ALLOWED_RECORD_TYPES }
+
+  scope :active, -> { where(started_at: stale_after.ago..) }
+  scope :by_others, ->(user) { where.not(user: user) }
+  scope :for_record, ->(record) {
+    where(record_type: record.class.name, record_id: record.id)
+  }
+  scope :stale, -> { where(started_at: ...stale_after.ago) }
+
+  def self.acquire(record_type:, record_id:, user:)
+    purge_stale_for(record_type: record_type, record_id: record_id)
+    create_or_find_by!(record_type: record_type, record_id: record_id, user: user)
+  end
+
+  def self.purge_stale_for(record_type:, record_id:)
+    where(record_type: record_type, record_id: record_id).stale.destroy_all
+  end
+
+  def self.stale_after
+    Configuration.editing_session_stale_after.minutes
+  end
+end

app/views/qa/_state_button.html.erb

@@ -18,3 +18,4 @@
       <% end %>
     <% end %>
   </div>
+</div>

app/views/shared/edit_locked.html.erb

@@ -0,0 +1,40 @@
+<% content_for :title, 'Record locked' %>
+
+<div class="content-container mt-4">
+  <div class="edit-locked">
+    <div class="icon">
+      <i class="fa-solid fa-lock"></i>
+    </div>
+
+    <h3 class="heading">
+      This <%= @locked_record.model_name.human.downcase %> is currently being edited
+    </h3>
+
+    <p class="description">
+      Another team member is currently editing this record. Go back, or edit
+      anyway and risk overwriting their changes.
+    </p>
+
+    <% @locked_by.each do |user| %>
+      <div class="editor">
+        <%= avatar_image(user, size: 48) %>
+        <div class="editor-info">
+          <span class="editor-name"><%= user.name %></span>
+          <span class="editor-status">
+            <span class="pulse"></span> Currently editing
+          </span>
+        </div>
+      </div>
+    <% end %>
+
+    <div class="actions">
+      <%= link_to @back_path, class: 'btn btn-primary' do %>
+        <i class="fa-solid fa-arrow-left"></i> Go back
+      <% end %>
+
+      <%= link_to url_for(request.parameters.merge(force: 'true')), class: 'btn btn-outline-danger' do %>
+        <i class="fa-solid fa-lock-open"></i> Edit anyway
+      <% end %>
+    </div>
+  </div>
+</div>

db/migrate/20260722100000_create_editing_sessions.rb

@@ -0,0 +1,11 @@
+class CreateEditingSessions < ActiveRecord::Migration[8.0]
+  def change
+    create_table :editing_sessions do |t|
+      t.references :user, null: false, foreign_key: true
+      t.references :record, polymorphic: true, null: false
+      t.datetime :started_at, null: false, precision: nil, default: -> { 'CURRENT_TIMESTAMP' }
+
+      t.index [:user_id, :record_type, :record_id], unique: true, name: 'index_editing_sessions_uniqueness'
+    end
+  end
+end

spec/factories/editing_sessions.rb

@@ -0,0 +1,7 @@
+FactoryBot.define do
+  factory :editing_session do
+    association :user
+    record_type { 'Issue' }
+    record_id { create(:issue).id }
+  end
+end

spec/features/edit_locking_spec.rb

@@ -0,0 +1,52 @@
+require 'rails_helper'
+
+describe 'Edit locking multi-actor flow' do
+  let(:project) { Project.new }
+  let(:issue) { create(:issue, node: project.issue_library) }
+  let(:password) { 'spec-password' }
+
+  before do
+    Configuration.find_or_create_by(name: 'admin:password')
+      .update!(value: BCrypt::Password.create(password))
+  end
+
+  def sign_in_as(username)
+    visit login_path
+    fill_in 'Username', with: username
+    fill_in 'Password', with: password
+    click_button 'Log in'
+  end
+
+  it 'locks the record for a second editor, lets them bypass it, and releases the lock on save' do
+    Capybara.using_session(:user_a) { sign_in_as('user-a@example.com') }
+    Capybara.using_session(:user_b) { sign_in_as('user-b@example.com') }
+
+    Capybara.using_session(:user_a) do
+      visit edit_project_issue_path(project, issue)
+      expect(page).to have_content('Edit issue')
+    end
+
+    Capybara.using_session(:user_b) do
+      visit edit_project_issue_path(project, issue)
+      expect(page).to have_content('currently being edited')
+      expect(page).to have_content('user-a@example.com')
+
+      click_link 'Go back'
+      expect(page).not_to have_content('currently being edited')
+    end
+
+    Capybara.using_session(:user_b) do
+      visit edit_project_issue_path(project, issue)
+      click_link 'Edit anyway'
+      expect(page).to have_content('Edit issue')
+    end
+
+    Capybara.using_session(:user_a) do
+      find('.btn-states button[type="submit"]').click
+      expect(page).to have_content('Issue updated.')
+    end
+
+    user_a = User.find_by(email: 'user-a@example.com')
+    expect(EditingSession.for_record(issue).where(user: user_a)).not_to exist
+  end
+end