Improve user edit UX, incl add comments

.claude/settings.local.json

@@ -6,10 +6,8 @@
       "Bash(bundle exec rails runner:*)",
       "Bash(bundle exec rspec:*)",
       "Bash(bundle exec rubocop:*)",
-
       "Bash(RAILS_ENV=test bundle exec rails runner:*)",
       "Bash(RAILS_ENV=test bundle exec rspec:*)",
-
       "Bash(git add:*)",
       "Bash(git apply:*)",
       "Bash(git checkout:*)",
@@ -19,20 +17,18 @@
       "Bash(git push:*)",
       "Bash(git reset:*)",
       "Bash(git stash:*)",
-
       "Bash(bin/rails db:migrate:*)",
       "Bash(bin/rails runner:*)",
-
       "Bash(chmod:*)",
       "Bash(mysql -u root:*)",
-
       "Bash(git -C /Users/maebeale/programming/awbw branch --show-current)",
-
       "Bash(bundle exec rails routes:*)",
       "Bash(bundle exec rubocop:*)",
       "Bash(git -C /Users/maebeale/programming/awbw diff --name-only main...HEAD)",
-      "Bash(git log:*)"
-
+      "Bash(git log:*)",
+      "Bash(bin/rails generate:*)",
+      "Bash(bin/rails db:schema:dump:*)",
+      "Bash(RAILS_ENV=test bin/rails:*)"
     ]
   }
 }

app/controllers/community_news_controller.rb

@@ -108,7 +108,7 @@ def destroy
   # Optional hooks for setting variables for forms or index
   def set_form_variables
     @organizations = Organization.pluck(:name, :id).sort_by(&:first)
-    @authors = User.active.or(User.where(id: @community_news.author_id))
+    @authors = User.has_access.or(User.where(id: @community_news.author_id))
                    .includes(:person)
                    .map { |u| [ u.full_name, u.id ] }.sort_by(&:first)
     @categories_grouped =

app/controllers/event_registrations_controller.rb

@@ -99,7 +99,7 @@ def set_form_variables
            .or(Event.where(id: @event_registration.event_id))
            .distinct
            .order(start_date: :desc)
-    @registrants = User.active.includes(:person).order("people.first_name, people.last_name")
+    @registrants = User.has_access.includes(:person).order("people.first_name, people.last_name")
   end
 
   private

app/controllers/people_controller.rb

@@ -77,7 +77,7 @@ def edit
       :contact_methods,
       :addresses,
       { avatar_attachment: :blob },
-      { comments: :created_by },
+      { comments: [ :created_by, :updated_by ] },
       { sectorable_items: :sector },
       organization_people: { organization: :logo_attachment }
     ).find(params[:id]).decorate
@@ -88,7 +88,8 @@ def edit
   def create
     @person = Person.new(person_params.except(:user_attributes))
     authorize! @person
-    @person.user ||= (User.find(params[:person][:user_attributes][:id]) if params[:person][:user_attributes])
+    @person.user ||= User.find_by(id: params[:user_id]) if params[:user_id].present?
+    @person.user ||= User.find_by(id: params.dig(:person, :user_attributes, :id)) if params.dig(:person, :user_attributes, :id).present?
 
     unless params[:skip_duplicate_check].present?
       duplicates = find_duplicate_people(

app/controllers/resources_controller.rb

@@ -149,7 +149,7 @@ def set_form_variables
     @resource.build_downloadable_asset if @resource.downloadable_asset.blank?
     @resource.gallery_assets.build
     @windows_types = WindowsType.all
-    @authors = User.active.or(User.where(id: @resource.user_id))
+    @authors = User.has_access.or(User.where(id: @resource.user_id))
                    .includes(:person)
                    .order("people.first_name, people.last_name")
                    .map { |u| [ u.full_name, u.id ] }

app/controllers/stories_controller.rb

@@ -178,7 +178,7 @@ def set_form_variables
                             .order(:created_at)
     @windows_types = WindowsType.all
     @workshops = Workshop.all.order(:title)
-    @users = User.active.or(User.where(id: @story.created_by_id))
+    @users = User.has_access.or(User.where(id: @story.created_by_id))
                  .includes(:person)
                  .order("people.first_name, people.last_name")
     @categories_grouped =

app/controllers/story_ideas_controller.rb

@@ -87,7 +87,7 @@ def set_form_variables
     @windows_types = WindowsType.all
     @workshops = Workshop.order(:title)
 
-    @users = User.active.includes(:person)
+    @users = User.has_access.includes(:person)
     @users = @users.or(User.where(id: @story_idea.created_by_id)) if @story_idea&.created_by_id
     @users = @users.distinct.order("people.first_name, people.last_name")
     @story_idea.build_primary_asset if @story_idea.primary_asset.blank?

app/controllers/users_controller.rb

@@ -6,7 +6,8 @@ class UsersController < ApplicationController
   def index
     authorize!
     per_page = params[:number_of_items_per_page].presence || 25
-    base_scope = authorized_scope(User.includes(avatar_attachment: :blob,
+    base_scope = authorized_scope(User.includes(:created_by, :updated_by,
+                                                avatar_attachment: :blob,
                                                 person: { avatar_attachment: :blob }))
     filtered = base_scope.search_by_params(params).order(:first_name, :last_name)
     @users_count = filtered.count
@@ -16,6 +17,23 @@ def index
   def show
     authorize! @user
     @user = User.find(params[:id]).decorate
+    @comments = @user.comments.includes(:created_by).newest_first.paginate(page: params[:comments_page], per_page: 5)
+
+    user_auth_events = Ahoy::Event
+      .where("name LIKE 'auth.%' OR name LIKE 'update.user'")
+      .where(
+        "(CAST(JSON_EXTRACT(properties, '$.record_id') AS UNSIGNED) = :id AND JSON_UNQUOTE(JSON_EXTRACT(properties, '$.record_type')) = 'User') OR " \
+        "(CAST(JSON_EXTRACT(properties, '$.resource_id') AS UNSIGNED) = :id AND JSON_UNQUOTE(JSON_EXTRACT(properties, '$.resource_type')) = 'User')",
+        id: @user.id
+      )
+
+    @last_admin_event = user_auth_events.where(name: %w[auth.admin_granted auth.admin_revoked]).order(time: :desc).first
+    @last_lock_event = user_auth_events.where(name: %w[auth.account_locked auth.account_unlocked]).order(time: :desc).first
+
+    @account_events = user_auth_events
+      .includes(:user)
+      .order(time: :desc)
+      .paginate(page: params[:page], per_page: 10)
   end
 
   def new
@@ -25,6 +43,7 @@ def new
   end
 
   def edit
+    @user = User.includes(comments: [ :created_by, :updated_by ]).find(params[:id])
     authorize! @user
     set_form_variables
   end
@@ -37,9 +56,10 @@ def create
     unless params[:skip_duplicate_check].present?
       email = @user.email
       if email.present? && !email.downcase.end_with?("@example.com")
-        duplicates = find_duplicate_users(email)
+        person_id = params[:person_id].presence || params.dig(:user, :person_id).presence || @user.person_id
+        duplicates = find_duplicate_users(email, exclude_person_id: person_id)
         if duplicates.any?
-          redirect_to check_duplicates_users_path(email: email, person_id: params[:person_id].presence || params.dig(:user, :person_id).presence)
+          redirect_to check_duplicates_users_path(email: email, person_id: person_id)
           return
         end
       end
@@ -55,6 +75,8 @@ def create
     # assign person
     person_id = params[:person_id].presence || params.dig(:user, :person_id).presence
     @user.person = Person.find(person_id) if person_id
+    @user.created_by = current_user
+    @user.updated_by = current_user
 
     if @user.save
       # @user.notifications.create(notification_type: 0)
@@ -70,7 +92,7 @@ def check_duplicates
 
     @email = params[:email]
     @person_id = params[:person_id]
-    @duplicates = find_duplicate_users(@email)
+    @duplicates = find_duplicate_users(@email, exclude_person_id: @person_id)
     @blocked = @duplicates.any? { |d| d[:blocked] }
   end
 
@@ -83,9 +105,16 @@ def update
       bypass_sign_in(@user)
     end
 
-    if @user.update(user_params.except(:password, :password_confirmation))
-      # @user.notifications.create(notification_type: 1)
-      redirect_to users_path, notice: "User was successfully updated."
+    @user.assign_attributes(user_params.except(:password, :password_confirmation))
+    @user.updated_by = current_user
+    @user.comments.select(&:new_record?).each { |c| c.created_by = current_user }
+    @user.comments.select(&:changed?).each { |c| c.updated_by = current_user }
+
+    if @user.save
+      bypass_sign_in(@user) if @user == current_user
+      notice = "User was successfully updated."
+      notice += " A confirmation email has been sent to #{@user.unconfirmed_email}." if @user.unconfirmed_email.present? && @user.saved_change_to_unconfirmed_email?
+      redirect_to users_path, notice: notice
     else
       flash[:alert] = "Unable to update user."
       set_form_variables
@@ -251,33 +280,41 @@ def password_params
     params.require(:user).permit(:current_password, :password, :password_confirmation)
   end
 
-  def find_duplicate_users(email)
+  def find_duplicate_users(email, exclude_person_id: nil)
     return [] if email.blank?
 
     email_lower = email.downcase
     duplicates = []
 
     # Check existing users with same email
-    User.where("LOWER(email) = ?", email_lower).limit(10).each do |user|
+    users_scope = User.where("LOWER(email) = ?", email_lower).includes(:person)
+    users_scope = users_scope.where.not(person_id: exclude_person_id) if exclude_person_id
+    users_scope.limit(10).each do |user|
       duplicates << {
         id: user.id,
         name: user.person&.full_name || "#{user.first_name} #{user.last_name}".strip,
+        person_id: user.person_id,
         email: user.email,
         type: "user",
         blocked: true
       }
     end
 
-    # Check people with matching email (who may not have a user yet)
-    Person.includes(:user)
-          .left_joins(:user)
+    # Check people with matching email or secondary email
+    exclude_person_ids = duplicates.map { |d| d[:person_id] }.compact
+    exclude_person_ids << exclude_person_id.to_i if exclude_person_id
+    people_scope = Person.includes(:user)
           .where("LOWER(people.email) = :email OR LOWER(people.email_2) = :email", email: email_lower)
-          .where(users: { id: nil })
-          .limit(10).each do |person|
+    people_scope = people_scope.where.not(id: exclude_person_ids) if exclude_person_ids.any?
+    people_scope.limit(10).each do |person|
+      primary_match = person.email&.downcase == email_lower
       duplicates << {
         id: person.id,
         name: person.full_name,
-        email: person.email || person.email_2,
+        email: primary_match ? person.email : person.email_2,
+        email_field: primary_match ? "primary" : "secondary",
+        has_user: person.user.present?,
+        user_email: person.user&.email,
         type: "person",
         blocked: false
       }
@@ -288,7 +325,7 @@ def find_duplicate_users(email)
 
   def user_params
     params.require(:user).permit(
-      :email, :comment, :person_id, :inactive, :primary_address, :time_zone, :super_user,
+      :email, :comment, :person_id, :inactive, :locked, :primary_address, :time_zone, :super_user,
 
       ##### legacy to remove later
       :agency_id, :legacy, :legacy_id, :subscribecode, :avatar, :first_name, :last_name, # legacy to remove later
@@ -297,6 +334,7 @@ def user_params
       #####
 
       organization_people_attributes: [ :id, :organization_id, :position, :title, :inactive, :primary_contact, :start_date, :end_date, :_destroy ],
+      comments_attributes: [ :id, :body ],
     )
   end
 end

app/controllers/workshop_variation_ideas_controller.rb

@@ -88,7 +88,7 @@ def set_form_variables
     @workshops = Workshop.published.order(:title)
     @organizations = authorized_scope(Organization.all).order(:name)
     @windows_types = WindowsType.order(:name)
-    @users = User.active.or(User.where(id: @workshop_variation_idea.created_by_id))
+    @users = User.has_access.or(User.where(id: @workshop_variation_idea.created_by_id))
                  .order(:first_name, :last_name)
   end
 

app/frontend/javascript/controllers/comment_edit_toggle_controller.js

@@ -1,5 +1,10 @@
 import { Controller } from "@hotwired/stimulus";
 
+// Connects to data-controller="comment-edit-toggle"
+//
+// Toggles between view and edit modes for inline comment editing.
+// When exiting edit mode, syncs textarea values back to the truncated display.
+//
 export default class extends Controller {
   static targets = ["editLabel", "viewLabel"];
 
@@ -9,6 +14,21 @@ export default class extends Controller {
 
   toggle() {
     this.editing = !this.editing;
+
+    // When leaving edit mode, sync textarea values to view display
+    if (!this.editing) {
+      this.element.querySelectorAll(".nested-fields").forEach((item) => {
+        const textarea = item.querySelector(".comment-edit textarea");
+        const viewBody = item.querySelector(".comment-view .comment-body");
+        if (textarea && viewBody) {
+          const text = textarea.value;
+          viewBody.textContent =
+            text.length > 135 ? text.substring(0, 132) + "..." : text;
+          viewBody.title = text;
+        }
+      });
+    }
+
     this.element
       .querySelectorAll(".comment-view")
       .forEach((el) => (el.style.display = this.editing ? "none" : ""));
@@ -17,8 +37,8 @@ export default class extends Controller {
       .forEach((el) => (el.style.display = this.editing ? "" : "none"));
 
     if (this.hasEditLabelTarget && this.hasViewLabelTarget) {
-      this.editLabelTarget.classList.toggle("hidden", this.editing);
-      this.viewLabelTarget.classList.toggle("hidden", !this.editing);
+      this.editLabelTarget.style.display = this.editing ? "none" : "";
+      this.viewLabelTarget.style.display = this.editing ? "" : "none";
     }
   }
 }

app/frontend/javascript/controllers/dirty_form_controller.js

@@ -2,14 +2,13 @@ import { Controller } from "@hotwired/stimulus";
 
 // Connects to data-controller="dirty-form"
 //
-// Tracks whether a form has unsaved changes and prompts on cancel.
-// Listens for input changes and cocoon nested field additions/removals.
-//
-// data-dirty-form-target="cancel" on the cancel link
+// Tracks whether a form has unsaved changes and warns before navigating away.
+// Uses three layers of protection:
+//   1. turbo:before-visit   – catches Turbo Drive link clicks (custom message)
+//   2. beforeunload         – catches non-Turbo navigations / tab close (browser message)
+//   3. confirmCancel action – explicit cancel button binding (custom message)
 //
 export default class extends Controller {
-  static targets = ["cancel"];
-
   connect() {
     this.dirty = false;
     this.handleChange = () => (this.dirty = true);
@@ -18,12 +17,35 @@ export default class extends Controller {
     this.element.addEventListener("change", this.handleChange);
     this.element.addEventListener("cocoon:after-insert", this.handleChange);
     this.element.addEventListener("cocoon:after-remove", this.handleChange);
+
+    // Clear dirty flag on form submit so guards don't block saving
+    this.handleSubmit = () => (this.dirty = false);
+    this.element.addEventListener("submit", this.handleSubmit);
+
+    // Guard Turbo Drive navigations (links without data-turbo="false")
+    this.handleTurboVisit = (event) => {
+      if (this.dirty && !confirm("You have unsaved changes. Are you sure you want to leave?")) {
+        event.preventDefault();
+      }
+    };
+    document.addEventListener("turbo:before-visit", this.handleTurboVisit);
+
+    // Guard non-Turbo navigations (data-turbo="false" links, browser back, tab close)
+    this.handleBeforeUnload = (event) => {
+      if (this.dirty) {
+        event.preventDefault();
+        event.returnValue = "";
+      }
+    };
+    window.addEventListener("beforeunload", this.handleBeforeUnload);
   }
 
   confirmCancel(event) {
     if (this.dirty) {
       if (!confirm("You have unsaved changes. Are you sure you want to leave?")) {
         event.preventDefault();
+      } else {
+        this.dirty = false;
       }
     }
   }
@@ -33,5 +55,8 @@ export default class extends Controller {
     this.element.removeEventListener("change", this.handleChange);
     this.element.removeEventListener("cocoon:after-insert", this.handleChange);
     this.element.removeEventListener("cocoon:after-remove", this.handleChange);
+    this.element.removeEventListener("submit", this.handleSubmit);
+    document.removeEventListener("turbo:before-visit", this.handleTurboVisit);
+    window.removeEventListener("beforeunload", this.handleBeforeUnload);
   }
 }

app/frontend/javascript/controllers/index.js

@@ -74,3 +74,6 @@ application.register("timeframe", TimeframeController)
 
 import ToggleLockController from "./toggle_lock_controller"
 application.register("toggle-lock", ToggleLockController)
+
+import ToggleUserIconController from "./toggle_user_icon_controller"
+application.register("toggle-user-icon", ToggleUserIconController)