Merge branch 'tess-1.5'

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/helpers/search_helper.rb

@@ -10,7 +10,8 @@ def clear_filters_path
   end
 
   def facet_title(name, value, html_options = {})
-    return render_language_name(value) if name == 'language'
+    lang = render_language_name(value) if name.to_s == 'language'
+    return lang unless lang.blank?
 
     html_options.delete(:title) || truncate(value.to_s, length: 50)
   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 Rails.env.production?
-      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
 

app/views/users/_form.html.erb

@@ -37,7 +37,7 @@
         <% end %>
 
         <% if @user.is_admin?%>
-          <%= f.input :check_broken_scrapers, label: 'Receive emails with scrapers that have not found anything last period.' %>
+          <%= user_f.input :check_broken_scrapers %>
         <% end %>
 
         <% if TeSS::Config.feature['trainers'] %>
@@ -57,9 +57,8 @@
                       label: 'Training Experience', prompt: 'Select a level of experience...',
                       input_html: { title: t('profile.hints.experience') } %>
 
-          <%= f.input :language, options: language_options_for_select,
-                         label: 'Languages Spoken', model_name: 'user[profile_attributes]', as: :select,
-             input_html: { class: 'js-select2' },
+          <%= f.dropdown :language, options: language_options_for_select,
+                         label: t('simple_form.labels.profile.language'), model_name: 'user[profile_attributes]', as: :select,
                          errors: @user.profile.errors[:language], title: t('profile.hints.language') %>
 
           <%= f.multi_input :expertise_academic, label: 'Academic expertise',

config/initializers/version.rb

@@ -1 +1 @@
-APP_VERSION = '1.5.0'
+APP_VERSION = '1.5.1'

config/locales/en.yml

@@ -970,3 +970,7 @@ en:
         visible: Show this event?
       material:
         visible: Show this material?
+      user:
+        check_broken_scrapers: Receive emails with scrapers that have not found anything last period.
+      profile:
+        language: Languages Spoken

test/controllers/trainers_controller_test.rb

@@ -8,7 +8,8 @@ class TrainersControllerTest < ActionController::TestCase
     assert_response :success
     trainers = assigns(:trainers)
     assert_not_nil trainers
-    assert_equal 1, trainers.size
+    assert_equal 2, trainers.size
+    assert_includes trainers, users(:trainer_user).profile
   end
 
 end

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
@@ -458,4 +468,16 @@ class UsersControllerTest < ActionController::TestCase
     assert_response :success
     refute assigns(:users).include?(users(:basic_user))
   end
+
+  test "should get edit for trainers feature enabled" do
+    user = users(:trainer_user)
+    sign_in(user)
+    get :edit, params: { id: user }
+    assert_response :success
+
+    user = users(:admin_trainer)
+    sign_in(user)
+    get :edit, params: { id: user }
+    assert_response :success
+  end
 end