-
Notifications
You must be signed in to change notification settings - Fork 20
Profile form fixes #1119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Profile form fixes #1119
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refines profile updating, trainer filtering, and search indexing to prevent duplicate records, correct filter options, and optimize Solr reindexing.
- Adds a profile ID check to user updates to block creation of new profile records
- Fixes language select options and conditional display of trainer filters
- Refactors Solr reindex callbacks from individual models into a single
after_commitinProfile
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/controllers/users_controller.rb | Added check_profile_id before_action to forbid mismatched IDs |
| app/views/users/_form.html.erb | Updated scraper-email checkbox and replaced language select input |
| app/models/profile.rb & trainer.rb | Removed per-model callbacks; unified reindex_trainer callback |
| app/models/concerns/searchable.rb | Revised public filtering logic for Trainer/Profile searches |
| app/helpers/search_helper.rb | Enhanced facet_title to gracefully handle blank language names |
| config/locales/en.yml | Added translations for scraper emails and profile language label |
| test/test_helper.rb | Stubbed external HTTP requests to example.com |
| test/fixtures/users.yml | Introduced admin_trainer fixture for admin tests |
| test/fixtures/profiles.yml | Added admin_trainer_profile fixture |
| test/controllers/users_controller_test.rb | Updated profile update tests and added forbidden-ID test |
| test/controllers/trainers_controller_test.rb | Adjusted expected trainer count and inclusion assertion |
Comments suppressed due to low confidence (2)
config/locales/en.yml:974
- These keys live under
en.useranden.profile, but SimpleForm will look undersimple_form.labels.user.check_broken_scrapersandsimple_form.labels.profile.language. Consider nesting these translations undersimple_form.labelsto match SimpleForm’s lookup path.
check_broken_scrapers: Receive emails with scrapers that have not found anything last period.
app/views/users/_form.html.erb:40
- The variable
user_fisn’t defined in this form; it looks like you intended to usef.inputfor the user builder. Replaceuser_f.inputwithf.inputor properly defineuser_fviafields_for.
<%= user_f.input :check_broken_scrapers %>
| 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
There was a problem hiding this comment.
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary of changes
Motivation and context
User noticed trainer language select on profile edit page was showing Yes/No options instead of languages, ran into other issues whilst debugging.
Checklist
to license it to the TeSS codebase under the
BSD license.