Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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: [] },
Expand All @@ -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
3 changes: 2 additions & 1 deletion app/helpers/search_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion app/models/concerns/searchable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Copilot AI Jul 1, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This branch forces all searches for Trainer/Profile to only show public records, even for admins. Document this behavior or refine the condition so that admins can still see private items when intended.

Copilot uses AI. Check for mistakes.
# `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
Expand Down
48 changes: 7 additions & 41 deletions app/models/profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
4 changes: 0 additions & 4 deletions app/models/trainer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 3 additions & 4 deletions app/views/users/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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'] %>
Expand All @@ -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',
Expand Down
4 changes: 4 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion test/controllers/trainers_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 24 additions & 2 deletions test/controllers/users_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
7 changes: 7 additions & 0 deletions test/fixtures/profiles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down