Prevent new profile record being created every time a user is updated.

Fix solr indexing weirdness with trainers

Author: fbacall

app/controllers/users_controller.rb

@@ -6,6 +6,7 @@ class UsersController < ApplicationController
   prepend_before_action :set_user, only: %i[show edit update destroy change_token]
   prepend_before_action :init_user, only: [:create]
   before_action :set_breadcrumbs
+  before_action :check_profile_id, only: [:update]
 
   include ActionView::Helpers::TextHelper
 
@@ -127,7 +128,7 @@ def init_user
 
   def user_params
     allowed_parameters = [:email, :username, :password, :image, :image_url, {
-      profile_attributes: [:firstname, :surname, :email, :website, :public,
+      profile_attributes: [:id, :firstname, :surname, :email, :website, :public,
                            :description, :location, :orcid, :experience,
                            { expertise_academic: [] }, { expertise_technical: [] },
                            { interest: [] }, { activity: [] }, { language: [] },
@@ -141,4 +142,13 @@ def user_params
   def invitees_enabled?
     feature_enabled?('invitation')
   end
+
+  # Prevent assigning other profiles
+  def check_profile_id
+    profile_id = @user.profile.id
+    incoming_id = user_params.dig(:profile_attributes, :id)
+    if profile_id && incoming_id && profile_id.to_i != incoming_id.to_i
+      handle_error(:forbidden, 'Invalid profile ID.')
+    end
+  end
 end

app/models/concerns/searchable.rb

@@ -86,7 +86,12 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s
           end
         end
 
-        if attribute_method?(:public) && !user&.is_admin? # Find a better way of checking this
+        if name == 'Trainer' || name == 'Profile'
+          # `public` means "Show in trainer registry" for Trainers/Profiles
+          any_of do
+            with(:public, true)
+          end
+        elsif attribute_method?(:public) && !user&.is_admin? # Find a better way of checking this
           any_of do
             with(:public, true)
             with(:user_id, user.id) if user

app/models/profile.rb

@@ -9,48 +9,14 @@ class Profile < ApplicationRecord
   validates :website, url: true, http_url: true, allow_blank: true
   validates :orcid, orcid: true, allow_blank: true
   after_validation :check_public
+  after_commit :reindex_trainer
   clean_array_fields(:expertise_academic, :expertise_technical, :fields,
                      :interest, :activity, :language, :social_media)
   update_suggestions(:expertise_technical, :interest)
 
   extend FriendlyId
   friendly_id :full_name, use: :slugged
 
-  after_update_commit :reindex
-  after_destroy_commit :reindex
-
-  if TeSS::Config.solr_enabled
-    # :nocov:
-    searchable do
-      # full text search fields
-      text :firstname
-      text :surname
-      text :description
-      # sort title
-      string :sort_title do
-        full_name.downcase
-      end
-      # other fields
-      integer :user_id
-      string :full_name
-      string :location
-      string :orcid
-      string :experience do
-        TrainerExperienceDictionary.instance.lookup_value(self.experience, 'title')
-      end
-      string :expertise_academic, multiple: true
-      string :expertise_technical, multiple: true
-      string :fields, :multiple => true
-      string :interest, multiple: true
-      string :activity, multiple: true
-      string :language, multiple: true
-      string :social_media, multiple: true
-      time :updated_at
-      boolean :public
-    end
-    # :nocov:
-  end
-
   def self.facet_fields
     field_list = %w( full_name )
   end
@@ -93,14 +59,14 @@ def check_public
     public ? self.type = 'Trainer' : self.type = 'Profile'
   end
 
-  def reindex
-    if TeSS::Config.solr_enabled
-      Trainer.reindex
-    end
-  end
-
   def should_generate_new_friendly_id?
     firstname_changed? or surname_changed?
   end
 
+  # This is needed if the `public` status of the profile changes when it is already instantiated - it won't be cast
+  # to a Trainer object and indexed by solr (only Trainer class is `searchable`).
+  def reindex_trainer
+    return unless TeSS::Config.solr_enabled
+    becomes(Trainer).solr_index
+  end
 end

app/models/trainer.rb

@@ -2,10 +2,6 @@
 
 # define model for Trainer as subset of Profile
 class Trainer < Profile
-
-  after_update_commit :reindex
-  after_destroy_commit :reindex
-
   extend FriendlyId
   friendly_id :full_name, use: :slugged
 

test/controllers/users_controller_test.rb

@@ -118,9 +118,19 @@ class UsersControllerTest < ActionController::TestCase
   end
 
   test "should update profile" do
-    sign_in users(:regular_user)
-    patch :update, params: { id: @user, user: { profile_attributes: { email: 'hot@mail.com' } } }
+    sign_in @user
+    profile_id = @user.profile.id
+    patch :update, params: { id: @user, user: { profile_attributes: { id: profile_id, email: 'hot@mail.com' } } }
     assert_redirected_to user_path(assigns(:user))
+    assert_equal profile_id, assigns(:user).profile.id
+  end
+
+  test "should not update profile ID" do
+    sign_in @user
+    another_profile_id = users(:another_regular_user).profile.id
+    patch :update, params: { id: @user, user: { profile_attributes: { id: another_profile_id, email: 'hot@mail.com' } } }
+    assert_response :forbidden
+    assert_not_equal another_profile_id, @user.reload.profile.id
   end
 
   test "should reset token" do

test/test_helper.rb

@@ -220,6 +220,8 @@ def mock_images
     WebMock.stub_request(:get, 'https://bio.tools/api/tool?q=Material%20with%20suggestions')
            .with(headers: { 'Accept' => '*/*', 'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'User-Agent' => 'Ruby' })
            .to_return(status: 200, body: '', headers: {})
+
+    WebMock.stub_request(:any, 'http://example.com/').to_return(status: 200)
   end
 
   def mock_orcids