app/controllers/settings_controller.rb

@@ -2,35 +2,41 @@ class SettingsController < ApplicationController
   before_action :authenticate_user!
   before_action :set_main_nav_section
   before_action :set_settings_section, only: [:show, :update, :update_email]
+  before_action :set_user, only: [:show, :update, :update_email]
 
   def index
     redirect_to setting_path(:profile)
   end
 
   def show
-    @user = current_user
   end
 
   def update
-    @user = current_user
-    @user.preferences.merge! user_params[:preferences]
-    @user.save!
+    @user.preferences.merge!(user_params[:preferences] || {})
+    @user.display_name = user_params[:display_name]
 
-    redirect_to setting_path(@settings_section), flash: {
-      success: 'Settings saved.'
-    }
+    if @user.save
+      if @user.display_name && (@user.display_name != @user.ldap_entry[:display_name])
+        LdapManager::UpdateDisplayName.call(@user.dn, user_params[:display_name])
+      end
+
+      redirect_to setting_path(@settings_section), flash: {
+        success: 'Settings saved.'
+      }
+    else
+      @validation_errors = @user.errors
+      render :show, status: :unprocessable_entity
+    end
   end
 
   def update_email
-    if current_user.valid_ldap_authentication?(email_params[:current_password])
-      current_user.email = email_params[:email]
-
-      if current_user.update email: email_params[:email]
+    if @user.valid_ldap_authentication?(email_params[:current_password])
+      if @user.update email: email_params[:email]
         redirect_to setting_path(:account), flash: {
           notice: 'Please confirm your new address using the confirmation link we just sent you.'
         }
       else
-        @validation_errors = current_user.errors
+        @validation_errors = @user.errors
         render :show, status: :unprocessable_entity
       end
     else
@@ -49,27 +55,31 @@ class SettingsController < ApplicationController
 
   private
 
-  def set_main_nav_section
-    @current_section = :settings
-  end
-
-  def set_settings_section
-    @settings_section = params[:section]
-    allowed_sections = [:profile, :account, :lightning, :xmpp]
-
-    unless allowed_sections.include?(@settings_section.to_sym)
-      redirect_to setting_path(:profile)
+    def set_main_nav_section
+      @current_section = :settings
     end
-  end
 
-  def user_params
-    params.require(:user).permit(preferences: [
-      :lightning_notify_sats_received,
-      :xmpp_exchange_contacts_with_invitees
-    ])
-  end
+    def set_settings_section
+      @settings_section = params[:section]
+      allowed_sections = [:profile, :account, :lightning, :xmpp]
 
-  def email_params
-    params.require(:user).permit(:email, :current_password)
-  end
+      unless allowed_sections.include?(@settings_section.to_sym)
+        redirect_to setting_path(:profile)
+      end
+    end
+
+    def set_user
+      @user = current_user
+    end
+
+    def user_params
+      params.require(:user).permit(:display_name, preferences: [
+        :lightning_notify_sats_received,
+        :xmpp_exchange_contacts_with_invitees
+      ])
+    end
+
+    def email_params
+      params.require(:user).permit(:email, :current_password)
+    end
 end

app/models/user.rb

@@ -1,6 +1,8 @@
 class User < ApplicationRecord
   include EmailValidatable
 
+  attr_accessor :display_name
+
   serialize :preferences, UserPreferences
 
   # Relations
@@ -17,7 +19,7 @@ class User < ApplicationRecord
   has_many :accounts, through: :lndhub_user
 
   validates_uniqueness_of :cn
-  validates_length_of :cn, :minimum => 3
+  validates_length_of :cn, minimum: 3
   validates_format_of :cn, with: /\A([a-z0-9\-])*\z/,
                            if: Proc.new{ |u| u.cn.present? },
                            message: "is invalid. Please use only letters, numbers and -"
@@ -31,6 +33,9 @@ class User < ApplicationRecord
   validates_uniqueness_of :email
   validates :email, email: true
 
+  validates_length_of :display_name, minimum: 3, maximum: 35, allow_blank: true,
+                                     if: -> { defined?(@display_name) }
+
   scope :confirmed, -> { where.not(confirmed_at: nil) }
   scope :pending,   -> { where(confirmed_at: nil) }
 
@@ -115,8 +120,13 @@ class User < ApplicationRecord
     @dn = Devise::LDAP::Adapter.get_dn(self.cn)
   end
 
-  def ldap_entry
-    ldap.fetch_users(uid: self.cn, ou: self.ou).first
+  def ldap_entry(reload: false)
+    return @ldap_entry if defined?(@ldap_entry) && !reload
+    @ldap_entry = ldap.fetch_users(uid: self.cn, ou: self.ou).first
+  end
+
+  def display_name
+    @display_name ||= ldap_entry[:display_name]
   end
 
   def services_enabled

app/services/ldap_manager/update_display_name.rb

@@ -0,0 +1,12 @@
+module LdapManager
+  class UpdateDisplayName < LdapManagerService
+    def initialize(dn, display_name)
+      @dn = dn
+      @display_name = display_name
+    end
+
+    def call
+      replace_attribute @dn, :displayName, @display_name
+    end
+  end
+end

app/services/ldap_manager/update_email.rb

@@ -6,7 +6,7 @@ module LdapManager
     end
 
     def call
-      replace_attribute @dn, :mail, [ @address ]
+      replace_attribute @dn, :mail, @address
     end
   end
 end

app/services/ldap_service.rb

@@ -50,7 +50,7 @@ class LdapService < ApplicationService
       treebase = ldap_config["base"]
     end
 
-    attributes = %w{dn cn uid mail admin service}
+    attributes = %w{dn cn uid mail displayName admin service}
     filter     = Net::LDAP::Filter.eq("uid", args[:uid] || "*")
 
     entries = ldap_client.search(base: treebase, filter: filter, attributes: attributes)
@@ -59,6 +59,7 @@ class LdapService < ApplicationService
       {
         uid: e.uid.first,
         mail: e.try(:mail) ? e.mail.first : nil,
+        display_name: e.try(:displayName) ? e.displayName.first : nil,
         admin: e.try(:admin) ? 'admin' : nil,
         service: e.try(:service)
       }

app/views/settings/_account.html.erb

@@ -3,7 +3,7 @@
       "settings--account--email-validation-failed-value": @validation_errors.present?
     } do %>
   <h3>E-Mail</h3>
-  <%= form_for(current_user, url: update_email_settings_path, method: "post") do |f| %>
+  <%= form_for(@user, url: update_email_settings_path, method: "post") do |f| %>
     <%= hidden_field_tag :section, "account" %>
     <p class="mb-2">
       <%= f.label :email, 'Address', class: 'font-bold' %>
@@ -34,7 +34,7 @@
         <%= f.password_field :current_password, class: "w-full", required: true %>
       </p>
       <p class="mt-6">
-        <%= f.submit "Update", class: "btn-md btn-blue" %>
+        <%= f.submit "Update", class: "btn-md btn-blue w-full md:w-auto" %>
       </p>
     </div>
   <% end %>

app/views/settings/_profile.html.erb

@@ -21,10 +21,15 @@
   <p class="text-sm text-gray-500">
     Your user address for Chat and Lightning Network.
   </p>
-
-  <%# <%= form_for(@user, as: "profile", url: settings_profile_path) do |f| %>
-  <%#   <p class="mt-8">
-  <%#     <%= f.submit "Save changes", class: 'btn-md btn-blue w-full sm:w-auto' %>
-  <%#   </p>
-  <%# <% end %>
+  <%= form_for(@user, url: setting_path(:profile), html: { :method => :put }) do |f| %>
+    <%= render FormElements::FieldsetComponent.new(tag: "div", title: "Display name") do %>
+      <%= f.text_field :display_name, class: "w-full sm:w-3/5 mb-2" %>
+      <% if @validation_errors.present? && @validation_errors[:display_name].present? %>
+        <p class="error-msg"><%= @validation_errors[:display_name].first %></p>
+      <% end %>
+    <% end %>
+    <p class="mt-8 pt-6 border-t border-gray-200 text-right">
+      <%= f.submit 'Save', class: "btn-md btn-blue w-full md:w-auto" %>
+    </p>
+  <% end %>
 </section>

app/views/shared/_sidenav_settings.html.erb

@@ -1,20 +1,20 @@
 <%= render SidenavLinkComponent.new(
   name: "Profile", path: setting_path(:profile), icon: "user",
-  active: current_page?(setting_path(:profile))
+  active: @settings_section.to_s == "profile"
 ) %>
 <%= render SidenavLinkComponent.new(
   name: "Account", path: setting_path(:account), icon: "key",
-  active: current_page?(setting_path(:account))
+  active: @settings_section.to_s == "account"
 ) %>
 <% if Setting.ejabberd_enabled %>
 <%= render SidenavLinkComponent.new(
   name: "Chat", path: setting_path(:xmpp), icon: "message-circle",
-  active: current_page?(setting_path(:xmpp))
+  active: @settings_section.to_s == "xmpp"
 ) %>
 <% end %>
 <% if Setting.lndhub_enabled %>
 <%= render SidenavLinkComponent.new(
   name: "Lightning", path: setting_path(:lightning), icon: "zap",
-  active: current_page?(setting_path(:lightning))
+  active: @settings_section.to_s == "lightning"
 ) %>
 <% end %>

spec/features/settings/account_spec.rb

@@ -2,54 +2,57 @@ require 'rails_helper'
 
 RSpec.describe 'Account settings', type: :feature do
   let(:user) { create :user }
-  let(:geraint) { create :user, id: 2, cn: 'geraint', email: "lamagliarosa@example.com" }
 
-  before do
-    login_as user, :scope => :user
-    geraint.save!
+  feature "Update email address" do
+    let(:geraint) { create :user, id: 2, cn: 'geraint', email: "lamagliarosa@example.com" }
 
-    allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
-      .with("invalid password").and_return(false)
-    allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
-      .with("valid password").and_return(true)
-  end
+    before do
+      login_as user, :scope => :user
+      geraint.save!
 
-  scenario 'Update email address fails with invalid password' do
-    visit setting_path(:account)
-    fill_in 'Address', with: "lamagliarosa@example.com"
-    fill_in 'Current password', with: "invalid password"
-    click_button "Update"
-
-    expect(current_url).to eq(setting_url(:account))
-    expect(user.reload.unconfirmed_email).to be_nil
-    within ".flash-msg" do
-      expect(page).to have_content("did not match your current password")
+      allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
+        .with("invalid password").and_return(false)
+      allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
+        .with("valid password").and_return(true)
     end
-  end
 
-  scenario 'Update email address fails when new address already taken' do
-    visit setting_path(:account)
-    fill_in 'Address', with: "lamagliarosa@example.com"
-    fill_in 'Current password', with: "valid password"
-    click_button "Update"
+    scenario 'fails with invalid password' do
+      visit setting_path(:account)
+      fill_in 'Address', with: "lamagliarosa@example.com"
+      fill_in 'Current password', with: "invalid password"
+      click_button "Update"
 
-    expect(current_url).to eq(setting_url(:update_email))
-    expect(user.reload.unconfirmed_email).to be_nil
-    within ".error-msg" do
-      expect(page).to have_content("has already been taken")
+      expect(current_url).to eq(setting_url(:account))
+      expect(user.reload.unconfirmed_email).to be_nil
+      within ".flash-msg" do
+        expect(page).to have_content("did not match your current password")
+      end
     end
-  end
 
-  scenario 'Update email address works' do
-    visit setting_path(:account)
-    fill_in 'Address', with: "lamagliabianca@example.com"
-    fill_in 'Current password', with: "valid password"
-    click_button "Update"
+    scenario 'fails when new address already taken' do
+      visit setting_path(:account)
+      fill_in 'Address', with: "lamagliarosa@example.com"
+      fill_in 'Current password', with: "valid password"
+      click_button "Update"
 
-    expect(current_url).to eq(setting_url(:account))
-    expect(user.reload.unconfirmed_email).to eq("lamagliabianca@example.com")
-    within ".flash-msg" do
-      expect(page).to have_content("Please confirm your new address")
+      expect(current_url).to eq(setting_url(:update_email))
+      expect(user.reload.unconfirmed_email).to be_nil
+      within ".error-msg" do
+        expect(page).to have_content("has already been taken")
+      end
+    end
+
+    scenario 'works with valid password and address' do
+      visit setting_path(:account)
+      fill_in 'Address', with: "lamagliabianca@example.com"
+      fill_in 'Current password', with: "valid password"
+      click_button "Update"
+
+      expect(current_url).to eq(setting_url(:account))
+      expect(user.reload.unconfirmed_email).to eq("lamagliabianca@example.com")
+      within ".flash-msg" do
+        expect(page).to have_content("Please confirm your new address")
+      end
     end
   end
 end

spec/features/settings/profile_spec.rb

@@ -0,0 +1,45 @@
+require 'rails_helper'
+
+RSpec.describe 'Profile settings', type: :feature do
+  let(:user) { create :user, cn: "mwahlberg" }
+
+  before do
+    login_as user, :scope => :user
+  end
+
+  feature "Update display name" do
+    before do
+      allow(user).to receive(:display_name).and_return("Mark")
+      allow_any_instance_of(User).to receive(:dn).and_return("cn=mwahlberg,ou=kosmos.org,cn=users,dc=kosmos,dc=org")
+      allow_any_instance_of(User).to receive(:ldap_entry).and_return({
+        uid: user.cn, ou: user.ou, display_name: "Mark"
+      })
+    end
+
+    scenario 'fails with validation error' do
+      visit setting_path(:profile)
+      fill_in 'Display name', with: "M"
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:profile))
+      expect(page).to have_field('Display name', with: 'M')
+      within ".error-msg" do
+        expect(page).to have_content("is too short")
+      end
+    end
+
+    scenario 'works with valid input' do
+      expect(LdapManager::UpdateDisplayName).to receive(:call)
+        .with(user.dn, "Marky Mark").and_return(true)
+
+      visit setting_path(:profile)
+      fill_in 'Display name', with: "Marky Mark"
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:profile))
+      within ".flash-msg" do
+        expect(page).to have_content("Settings saved")
+      end
+    end
+  end
+end