Skip to content

Conversation

@kennethrioja
Copy link
Contributor

Summary of changes

  • trainers/users_controller.rb: added @bioschemas for app/views/layouts/_head.html.erb to display the list of materials and events made by the trainer/user
  • app/views/common/_registrations_list.html.erb: refactored this from app/views/sers/show.html.erb and placed it under /common to be able to use it in the trainers and users' view
  • Respective views now feature a small sentence Added a total of <n> training resources in <instance-name>
  • Added tests in both _controller_test.rb

Motivation and context

This is one step towards the overall framework of recognition of trainers in EVERSE (mTeSS-X (view))

Screenshots

Screenshot 2026-01-07 at 16 29 57

Checklist

  • I have read and followed the CONTRIBUTING guide.
  • I confirm that I have the authority necessary to make this contribution on behalf of its copyright owner and agree
    to license it to the TeSS codebase under the
    BSD license.

Copy link
Contributor

Copilot AI left a 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 adds bioschemas JSON-LD metadata for materials and events to user and trainer profile pages, supporting the EVERSE framework for trainer recognition. It refactors the registrations list into a shared partial and displays a summary count of training resources.

  • Adds @bioschemas instance variable to both UsersController and TrainersController to generate JSON-LD metadata for materials and events
  • Refactors the registrations list from users/show.html.erb into a reusable common/_registrations_list.html.erb partial
  • Adds "Added a total of N training resources" summary text to trainer profile pages
  • Includes comprehensive test coverage for bioschemas JSON-LD rendering in both controllers

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
app/controllers/users_controller.rb Adds registrations method to fetch user's events and materials, and sets @bioschemas for JSON-LD generation
app/controllers/trainers_controller.rb Adds registrations method to fetch trainer's events and materials, and sets @bioschemas for JSON-LD generation
app/views/users/show.html.erb Replaces inline registrations list with shared partial
app/views/trainers/show.html.erb Adds resource count summary and renders shared registrations partial
app/views/common/_registrations_list.html.erb New shared partial containing refactored registrations list with tabs for events, materials, collections, and workflows
test/controllers/users_controller_test.rb Adds comprehensive test for bioschemas JSON-LD metadata in user profile pages
test/controllers/trainers_controller_test.rb Adds setup method and comprehensive test for bioschemas JSON-LD metadata in trainer profile pages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +5
<% materials_count = @trainer.user.materials.in_current_space.count %>
<% events_count = @trainer.user.events.in_current_space.from_verified_users.not_disabled.count %>
<% workflows_count = @trainer.user.workflows.in_current_space.visible_by(current_user).count %>
<% collections_count = @trainer.user.collections.in_current_space.visible_by(current_user).count %>
<% total = materials_count.to_i + events_count.to_i + workflows_count.to_i + collections_count.to_i %>
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

These count calculations assume @trainer.user is always present. If the trainer's associated user is nil, this will raise a NoMethodError. Consider adding a nil check or ensuring the user association is always present through validation.

Suggested change
<% materials_count = @trainer.user.materials.in_current_space.count %>
<% events_count = @trainer.user.events.in_current_space.from_verified_users.not_disabled.count %>
<% workflows_count = @trainer.user.workflows.in_current_space.visible_by(current_user).count %>
<% collections_count = @trainer.user.collections.in_current_space.visible_by(current_user).count %>
<% total = materials_count.to_i + events_count.to_i + workflows_count.to_i + collections_count.to_i %>
<% if @trainer.user.present? %>
<% materials_count = @trainer.user.materials.in_current_space.count %>
<% events_count = @trainer.user.events.in_current_space.from_verified_users.not_disabled.count %>
<% workflows_count = @trainer.user.workflows.in_current_space.visible_by(current_user).count %>
<% collections_count = @trainer.user.collections.in_current_space.visible_by(current_user).count %>
<% total = materials_count.to_i + events_count.to_i + workflows_count.to_i + collections_count.to_i %>
<% else %>
<% total = 0 %>
<% end %>

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +5
<% materials_count = @trainer.user.materials.in_current_space.count %>
<% events_count = @trainer.user.events.in_current_space.from_verified_users.not_disabled.count %>
<% workflows_count = @trainer.user.workflows.in_current_space.visible_by(current_user).count %>
<% collections_count = @trainer.user.collections.in_current_space.visible_by(current_user).count %>
<% total = materials_count.to_i + events_count.to_i + workflows_count.to_i + collections_count.to_i %>
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

These count queries are executed at the view level and then recalculated again inside the _registrations_list.html.erb partial (lines 6-17). This results in duplicate database queries. Consider calculating these counts once in the controller and passing them to the view, or passing the already-calculated counts to the partial as local variables.

Copilot uses AI. Check for mistakes.
Comment on lines +158 to +159
events = @user.events.in_current_space
materials = @user.materials.in_current_space
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The registrations method is duplicated between UsersController and TrainersController with identical logic. Consider extracting this to a shared concern or helper module to improve maintainability and follow the DRY principle.

Suggested change
events = @user.events.in_current_space
materials = @user.materials.in_current_space
user_registrations(@user)
end
private
def user_registrations(user)
events = user.events.in_current_space
materials = user.materials.in_current_space

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +53
events = @trainer.user.events.in_current_space
materials = @trainer.user.materials.in_current_space
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The registrations method is duplicated between UsersController and TrainersController with identical logic (except for accessing through @trainer.user vs @user). Consider extracting this to a shared concern or helper module to improve maintainability and follow the DRY principle.

Suggested change
events = @trainer.user.events.in_current_space
materials = @trainer.user.materials.in_current_space
user_registrations(@trainer.user)
end
def user_registrations(user)
events = user.events.in_current_space
materials = user.materials.in_current_space

Copilot uses AI. Check for mistakes.
assert_equal event.title, json_event['name']
assert_equal event.url, json_event['url']
assert_equal event.scientific_topic_uris.first, json_event['about'].first['@id']
assert_equal event.external_resources.first.url, json_event['mentions'].first['url']
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The test assumes event.external_resources.first is not nil without explicitly setting it up or asserting its existence. If the test event fixture doesn't have external resources, this will raise a NoMethodError. Consider either explicitly creating an external resource in the test setup or adding a nil check before the assertion.

Suggested change
assert_equal event.external_resources.first.url, json_event['mentions'].first['url']
external_resource = event.external_resources.first
assert_not_nil external_resource
assert_equal external_resource.url, json_event['mentions'].first['url']

Copilot uses AI. Check for mistakes.
</h2>
<!-- Field: registrations -->
<% if total.positive? %>
<p>Added a total of <b><%= total %></b> training resources in <%= TeSS::Config.site['title_short'] %>.
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The opening <p> tag on line 21 is missing its closing </p> tag. This will result in invalid HTML.

Suggested change
<p>Added a total of <b><%= total %></b> training resources in <%= TeSS::Config.site['title_short'] %>.
<p>Added a total of <b><%= total %></b> training resources in <%= TeSS::Config.site['title_short'] %>.</p>

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +53
events = @trainer.user.events.in_current_space
materials = @trainer.user.materials.in_current_space
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The registrations method assumes @trainer.user is always present. If the trainer's associated user is nil, this will raise a NoMethodError when trying to call .events or .materials on nil. Consider adding a nil check or ensuring the user association is always present through validation.

Suggested change
events = @trainer.user.events.in_current_space
materials = @trainer.user.materials.in_current_space
user = @trainer.user
return [] unless user
events = user.events.in_current_space
materials = user.materials.in_current_space

Copilot uses AI. Check for mistakes.
doc = Nokogiri::HTML(response.body)
jsonld = doc
.css('script[type="application/ld+json"]')
.map { |s| JSON.parse(s.text) rescue nil }
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Using rescue nil is overly broad and will silently swallow all exceptions, not just JSON parsing errors. This can hide bugs and make debugging difficult. Consider using a more specific rescue clause like rescue JSON::ParserError => nil or rescue StandardError => nil.

Suggested change
.map { |s| JSON.parse(s.text) rescue nil }
.map do |s|
begin
JSON.parse(s.text)
rescue JSON::ParserError
nil
end
end

Copilot uses AI. Check for mistakes.
doc = Nokogiri::HTML(response.body)
jsonld = doc
.css('script[type="application/ld+json"]')
.map { |s| JSON.parse(s.text) rescue nil }
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Using rescue nil is overly broad and will silently swallow all exceptions, not just JSON parsing errors. This can hide bugs and make debugging difficult. Consider using a more specific rescue clause like rescue JSON::ParserError => nil or rescue StandardError => nil.

Suggested change
.map { |s| JSON.parse(s.text) rescue nil }
.map { |s| JSON.parse(s.text) rescue JSON::ParserError nil }

Copilot uses AI. Check for mistakes.
assert_equal event.title, json_event['name']
assert_equal event.url, json_event['url']
assert_equal event.scientific_topic_uris.first, json_event['about'].first['@id']
assert_equal event.external_resources.first.url, json_event['mentions'].first['url']
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The test assumes event.external_resources.first is not nil without explicitly setting it up or asserting its existence. If the test event fixture doesn't have external resources, this will raise a NoMethodError. Consider either explicitly creating an external resource in the test setup or adding a nil check before the assertion.

Suggested change
assert_equal event.external_resources.first.url, json_event['mentions'].first['url']
external_resource = event.external_resources.first
if external_resource
assert_equal external_resource.url, json_event['mentions'].first['url']
end

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant