diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index bf6272797..36a2e73f9 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index ed522f600..e7788b5cc 100644 --- a/app/helpers/search_helper.rb +++ b/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 diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 047ba1059..22bfaafbc 100644 --- a/app/models/concerns/searchable.rb +++ b/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 diff --git a/app/models/profile.rb b/app/models/profile.rb index d07189bfa..d85d244aa 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -9,6 +9,7 @@ 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) @@ -16,41 +17,6 @@ class Profile < ApplicationRecord 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 diff --git a/app/models/trainer.rb b/app/models/trainer.rb index b7d7d31af..510acc5d4 100644 --- a/app/models/trainer.rb +++ b/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 diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index de1357746..4f7f24960 100644 --- a/app/views/users/_form.html.erb +++ b/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', diff --git a/config/locales/en.yml b/config/locales/en.yml index 90cca4f89..9ac0dabb6 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/test/controllers/trainers_controller_test.rb b/test/controllers/trainers_controller_test.rb index aa2db72cc..50e723c8b 100644 --- a/test/controllers/trainers_controller_test.rb +++ b/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 diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 17ef23a2f..dea63e67d 100644 --- a/test/controllers/users_controller_test.rb +++ b/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 diff --git a/test/fixtures/profiles.yml b/test/fixtures/profiles.yml index a305172f4..483bd2f39 100644 --- a/test/fixtures/profiles.yml +++ b/test/fixtures/profiles.yml @@ -83,3 +83,10 @@ trainer_two_profile: expertise_technical: ['java', 'python', 'ruby'] public: false type: Profile + +admin_trainer_profile: + user: admin_trainer + firstname: Adminson + surname: Trainer + public: true + type: Trainer \ No newline at end of file diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 67108e270..8c68c68c0 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -158,3 +158,11 @@ learning_path_curator: encrypted_password: 'learning_paths_curator_encrypted_password' confirmed_at: <%= Time.zone.now %> role: learning_path_curator + +admin_trainer: + username: 'Trainer Admin' + email: 'adam-trainer@notarealdomain.org' + encrypted_password: 'admin_encrypted_password' + confirmed_at: <%= Time.zone.now %> + role: admin + check_broken_scrapers: true diff --git a/test/test_helper.rb b/test/test_helper.rb index 326a3fca2..dba0f829c 100644 --- a/test/test_helper.rb +++ b/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