Merge branch '285-user-name-edits' into develop

app/controllers/users/names_controller.rb

@@ -0,0 +1,41 @@
+##
+# Controller to allow current user to change their name
+#
+class Users::NamesController < ApplicationController
+  before_action :check_user_logged_in, :check_user_suspension, :load_user
+
+  def update
+    if @edit_user.update(user_params)
+      flash[:notice] = _('Name successfully updated.')
+      redirect_to user_url(@edit_user)
+    else
+      render action: 'edit'
+    end
+  end
+
+  private
+
+  def user_params
+    params.require(:user).permit(:name)
+  end
+
+  def check_user_logged_in
+    return if authenticated?
+
+    flash[:error] = _('You need to be logged in to change your name')
+    redirect_to frontpage_url
+  end
+
+  def check_user_suspension
+    return unless current_user.suspended?
+
+    flash[:error] = _('Suspended users cannot edit their profile')
+    redirect_to edit_profile_about_me_path
+  end
+
+  def load_user
+    # Don't make changes to the current_user, this could brake the layout as we
+    # use name/url_name in the login bar URLs
+    @edit_user = User.find_by(url_name: current_user.url_name)
+  end
+end

app/models/outgoing_message.rb

@@ -57,6 +57,10 @@ class OutgoingMessage < ApplicationRecord
              foreign_key: 'incoming_message_followup_id',
              class_name: 'IncomingMessage'
 
+  has_one :user,
+          inverse_of: :outgoing_messages,
+          through: :info_request
+
   # can have many events, for items which were resent by site admin e.g. if
   # contact address changed
   has_many :info_request_events,

app/models/user.rb

@@ -76,6 +76,9 @@ class User < ApplicationRecord
   has_many :embargoes,
            inverse_of: :user,
            through: :info_requests
+  has_many :outgoing_messages,
+           inverse_of: :user,
+           through: :info_requests
   has_many :draft_info_requests,
            -> { order(created_at: :desc) },
            inverse_of: :user,
@@ -355,6 +358,17 @@ def name=(name)
     write_attribute(:name, name.try(:strip))
   end
 
+  def previous_names
+    outgoing_messages.unscope(:order).
+      distinct(:from_name).
+      where.not(from_name: read_attribute(:name)).
+      pluck(:from_name)
+  end
+
+  def safe_previous_names
+    outgoing_messages.map(&:safe_from_name).uniq - [read_attribute(:name)]
+  end
+
   # For use in to/from in email messages
   def name_and_email
     MailHandler.address_from_name_and_email(name, email)
@@ -420,6 +434,7 @@ def erase!
     sha = Digest::SHA1.hexdigest(rand.to_s)
 
     transaction do
+      slugs.destroy_all
       sign_ins.destroy_all
       profile_photo&.destroy!
 
@@ -436,10 +451,13 @@ def erase!
   def anonymise!
     return if info_requests.none? && comments.none?
 
-    censor_rules.create!(text: read_attribute(:name),
-                         replacement: _('[Name Removed]'),
-                         last_edit_editor: 'User#anonymise!',
-                         last_edit_comment: 'User#anonymise!')
+    current_name = read_attribute(:name)
+    [current_name, *previous_names].each do |name|
+      censor_rules.create!(text: name,
+                           replacement: _('[Name Removed]'),
+                           last_edit_editor: 'User#anonymise!',
+                           last_edit_comment: 'User#anonymise!')
+    end
   end
 
   def close_and_anonymise

app/views/followups/_followup.html.erb

@@ -107,6 +107,14 @@
       <% end %>
     <% end %>
 
+    <% if @info_request.from_name != @info_request.user_name %>
+      <div class="warning">
+        <%= _('Your name has been changed since your last message in this ' \
+              'request. Please consider mentioning this to the authority ' \
+              'and explain that you are the original requester.') %>
+      </div>
+    <% end %>
+
     <% form_url =
          if incoming_message.nil?
            preview_request_followups_url(request_id: @info_request.id)

app/views/user/show/_show_profile.html.erb

@@ -39,6 +39,17 @@
 
     <h1><%= h(@display_user.name) + (@is_you ? _(" (you)") : "") %></h1>
 
+    <% if @display_user.active? && @is_you %>
+      <p><%= link_to _('Change your name'), edit_users_name_path %></p>
+    <% end %>
+
+    <% if @display_user.active? && @display_user.safe_previous_names.any? %>
+      <p>
+        <%= _('Previously known as:') %>
+        <%= @display_user.safe_previous_names.to_sentence %>
+      </p>
+    <% end %>
+
     <p class="subtitle">
       <%= _('Joined {{site_name}} in {{year}}',
             :site_name => site_name,

app/views/users/names/edit.html.erb

@@ -0,0 +1,39 @@
+<% @title = _('Change your name used on {{site_name}}', site_name: site_name) %>
+
+<h1><%= @title %></h1>
+
+<div id="notice">
+  <p>
+    <%= _('When you send a request, we use the name you gave us. This makes ' \
+          'it hard to stop your old and new names from being connected.') %>
+  </p>
+  <p>
+    <%= _('If you change your name, your old requests won\'t change. Your ' \
+          'old name will still show up on requests that you\'ve already sent ' \
+          'and on your public profile page.') %>
+  </p>
+  <p>
+    <%= _('If you don\'t want your new name to be linked to your old one, ' \
+          'it\'s a good idea to make a new account instead.') %>
+  </p>
+  <p>
+    <%= _('If you are trying to remove your name, please
+          <a href="{{contact_us_url}}">contact us</a>.',
+          contact_us_url: help_contact_path) %>
+  </p>
+</div>
+
+<div id="change_name" class="change_name">
+  <%= form_for @edit_user, url: users_name_path do |f| %>
+    <%= foi_error_messages_for :edit_user %>
+
+    <p>
+      <label class="form_label" for="name"><%= _('Name:') %></label>
+      <%= f.text_field 'name', { size: 20 } %>
+    </p>
+
+    <div class="form_button">
+      <%= submit_tag _('Change your name') %>
+    </div>
+  <% end %>
+</div>

config/routes.rb

@@ -304,6 +304,10 @@ def matches?(request)
         as: :disable_email_alerts
   end
 
+  namespace :users, path: 'profile' do
+    resource :name, only: [:edit, :update]
+  end
+
   namespace :profile, :module => 'user_profile' do
     resource :about_me, :only => [:edit, :update], :controller => 'about_me'
   end

lib/tasks/temp.rake

@@ -15,6 +15,23 @@ namespace :temp do
     puts "Migrating to User#slugs completed."
   end
 
+  desc 'Populate OutgoingMessage#from_name'
+  task populate_outgoing_message_from_name: :environment do
+    scope = OutgoingMessage.where(from_name: nil).includes(:user)
+    count = scope.count
+
+    scope.find_each.with_index do |outgoing_message, index|
+      user = outgoing_message.user
+      outgoing_message.update_columns(from_name: user.name)
+
+      erase_line
+      print "Populating OutgoingMessage#from_name #{index + 1}/#{count}"
+    end
+
+    erase_line
+    puts "Populating OutgoingMessage#from_name completed."
+  end
+
   def erase_line
     # https://en.wikipedia.org/wiki/ANSI_escape_code#Escape_sequences
     print "\e[1G\e[K"

spec/controllers/users/names_controller_spec.rb

@@ -0,0 +1,90 @@
+require 'spec_helper'
+
+RSpec.describe Users::NamesController do
+  describe 'GET edit' do
+    context 'without a logged in user' do
+      it 'redirects to the home page' do
+        sign_in nil
+        get :edit
+        expect(response).to redirect_to(frontpage_path)
+      end
+    end
+
+    context 'with a logged in user' do
+      let(:user) { FactoryBot.create(:user) }
+
+      it 'assigns the currently logged in user' do
+        sign_in user
+        get :edit
+        expect(assigns[:user]).to eq(user)
+      end
+
+      it 'is successful' do
+        sign_in user
+        get :edit
+        expect(response).to be_successful
+      end
+
+      it 'renders the edit form' do
+        sign_in user
+        get :edit
+        expect(response).to render_template(:edit)
+      end
+    end
+  end
+
+  describe 'PUT update' do
+    context 'without a logged in user' do
+      it 'redirects to the sign in page' do
+        sign_in nil
+        put :update, params: { user: { about_me: 'Bobby' } }
+        expect(response).to redirect_to(frontpage_path)
+      end
+    end
+
+    context 'with a banned user' do
+      before { sign_in FactoryBot.create(:user, :banned) }
+
+      it 'displays an error' do
+        put :update, params: { user: { name: 'Bobby' } }
+        expect(flash[:error]).to eq('Suspended users cannot edit their profile')
+      end
+
+      it 'redirects to edit' do
+        put :update, params: { user: { name: 'Bobby' } }
+        expect(response).to redirect_to(edit_profile_about_me_path)
+      end
+    end
+
+    context 'with valid attributes' do
+      let(:user) { FactoryBot.create(:user) }
+      before { sign_in user }
+
+      it 'assigns the currently logged in user' do
+        put :update, params: { user: { name: 'Bobby' } }
+        expect(assigns[:user]).to eq(user)
+      end
+
+      it 'updates the user name' do
+        put :update, params: { user: { name: 'Bobby' } }
+        expect(user.reload.name).to eq('Bobby')
+      end
+    end
+
+    context 'with bad parameters' do
+      before { sign_in FactoryBot.create(:user) }
+
+      it 'can raise missing parameter exeception' do
+        expect {
+          put :update, params: { name: 'Bobby' }
+        }.to raise_error(ActionController::ParameterMissing)
+      end
+
+      it 'can raise unpermitted parameter exeception' do
+        expect {
+          put :update, params: { user: { name: 'Updated text', role_ids: [1] } }
+        }.to raise_error(ActionController::UnpermittedParameters)
+      end
+    end
+  end
+end

spec/fixtures/outgoing_messages.yml

@@ -34,6 +34,7 @@ useless_outgoing_message:
   last_sent_at: 2007-10-25 10:41:12.686264
   created_at: 2007-10-12 01:56:58.586598
   what_doing: normal_sort
+  from_name: Bob Smith
 silly_outgoing_message:
   id: 2
   info_request_id: 103
@@ -53,6 +54,7 @@ silly_outgoing_message:
   last_sent_at: 2007-10-14 10:41:12.686264
   created_at: 2007-10-14 01:56:58.586598
   what_doing: normal_sort
+  from_name: Bob Smith
 badger_outgoing_message:
   id: 3
   info_request_id: 104
@@ -63,6 +65,7 @@ badger_outgoing_message:
   last_sent_at: 2011-10-14 10:41:12.686264
   created_at: 2011-10-14 01:56:58.586598
   what_doing: normal_sort
+  from_name: Bob Smith
 boring_outgoing_message:
   id: 4
   info_request_id: 105
@@ -73,7 +76,7 @@ boring_outgoing_message:
   last_sent_at: 2012-01-14 10:41:12.686264
   created_at: 2012-01-14 01:56:58.586598
   what_doing: normal_sort
-
+  from_name: Bob Smith
 another_boring_outgoing_message:
   id: 5
   info_request_id: 106
@@ -84,7 +87,7 @@ another_boring_outgoing_message:
   created_at: 2006-01-12 01:56:58.586598
   updated_at: 2006-01-12 01:56:58.586598
   what_doing: normal_sort
-
+  from_name: Bob Smith
 spam_1_outgoing_message:
   id: 6
   info_request_id: 107
@@ -95,6 +98,7 @@ spam_1_outgoing_message:
   created_at: 2007-01-12 01:56:58.586598
   updated_at: 2007-01-12 01:56:58.586598
   what_doing: normal_sort
+  from_name: Robin Houston
 spam_2_outgoing_message:
   id: 7
   info_request_id: 108
@@ -105,7 +109,7 @@ spam_2_outgoing_message:
   created_at: 2007-01-12 02:56:58.586598
   updated_at: 2007-01-12 02:56:58.586598
   what_doing: normal_sort
-
+  from_name: Robin Houston
 external_outgoing_message:
   id: 8
   info_request_id: 109
@@ -116,7 +120,6 @@ external_outgoing_message:
   created_at: 2009-01-12 01:56:58.586598
   updated_at: 2009-01-12 01:56:58.586598
   what_doing: normal_sort
-
 anonymous_external_outgoing_message:
   id: 9
   info_request_id: 110
@@ -127,7 +130,6 @@ anonymous_external_outgoing_message:
   created_at: 2009-01-12 01:56:58.586598
   updated_at: 2009-01-12 01:56:58.586598
   what_doing: normal_sort
-
 other_outgoing_message:
   id: 10
   info_request_id: 111
@@ -138,4 +140,4 @@ other_outgoing_message:
   created_at: 2009-01-12 01:56:58.586598
   updated_at: 2009-01-12 01:56:58.586598
   what_doing: normal_sort
-
+  from_name: Another User