[ingestor] Extended ical ingestor to .ics and Indico #1161

kennethrioja · 2025-10-14T09:08:22Z

Summary of changes

From the basis of lib/ingestors/ical_ingestor.rb I extended it to be able to ingest:

Direct .ics links (e.g., https://indico.cern.ch/event/1596327/event.ics or https://indico.cern.ch/category/19377/events.ics)
Indico /event links (e.g., https://indico.cern.ch/event/1596327/)
Indico /category links (e.g., https://indico.cern.ch/category/19377/)

Motivation and context

training.cern.ch needs indico ingestion

Screenshots

✅ 100% test covered

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.

Copilot

Pull Request Overview

This PR extends the iCalendar ingestor to support direct .ics links and Indico event/category URLs. The changes refactor the existing IcalIngestor class to handle multiple URL formats, extract reusable functionality into concerns, and add comprehensive test coverage for the new URL conversion logic.

Key Changes:

Refactored IcalIngestor to process direct .ics files, Indico /event URLs, and Indico /category URLs
Extracted sitemap parsing logic into a reusable SitemapHelpers concern
Created IcalIngestorExportUrl concern to handle URL-to-export conversion for different URL formats

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
lib/ingestors/ical_ingestor.rb	Core refactoring: split monolithic methods into smaller, focused methods; integrated new concerns for sitemap and URL handling
lib/ingestors/concerns/sitemap_helpers.rb	New concern: extracts sitemap parsing (XML/TXT) into reusable module
lib/ingestors/concerns/ical_ingestor_export_url.rb	New concern: handles conversion of various URL formats to their `.ics` export equivalents
test/unit/ingestors/ical_ingestor_test.rb	Added comprehensive tests for error handling, URL conversion logic, and edge cases

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-16T12:00:20Z

lib/ingestors/ical_ingestor.rb

+    def process_description(input)
+      return input if input.nil?
+
+      desc = input.to_s.gsub('', '<br />')


The gsub pattern is empty (''), which will not match anything. This should likely be /\\R/ to match line breaks, as in the original implementation.

Suggested change

desc = input.to_s.gsub('', '<br />')

desc = input.to_s.gsub(/<br\s*\/?>/i, "\n")

Indeed – process_description is useless. I deleted it.

Copilot · 2025-10-16T12:00:20Z

lib/ingestors/ical_ingestor.rb

-        events = Icalendar::Event.parse(open_url(file_url, raise: true).set_encoding('utf-8'))
+    # Assigns to event: venue, online, city.
+    def assign_location_info(event, location)
+      return if location.blank? || !location.present?


The condition location.blank? || !location.present? is redundant. blank? and present? are opposites, so this simplifies to just return if location.blank?.

Suggested change

return if location.blank? || !location.present?

return if location.blank?

Copilot · 2025-10-16T12:00:20Z

lib/ingestors/ical_ingestor.rb

+        [
+          location['suburb'],
+          location['postcode'],
+          location['country']
+        ]


The process_location method attempts to call location['suburb'] but location is a string (from calevent.location), not a hash. This will cause a TypeError. The original code called convert_location(calevent.location) which likely returned a hash.

I kept the old code logic. I am now checking whether location is an array before returning this array of location variables, see changes here.

Copilot · 2025-10-16T12:00:20Z

test/unit/ingestors/ical_ingestor_test.rb

+    def ingestor.assign_basic_info(*)
+      raise StandardError, 'test failure'
+    end
+
+    ingestor.send(:process_calevent, calevent)
+


[nitpick] Reopening the singleton class to stub a method is fragile. Consider using a mocking framework like Mocha or Minitest's stub mechanism for cleaner and more maintainable test doubles.

Suggested change

def ingestor.assign_basic_info(*)

raise StandardError, 'test failure'

end

ingestor.send(:process_calevent, calevent)

ingestor.stub(:assign_basic_info, ->(*) { raise StandardError, 'test failure' }) do

ingestor.send(:process_calevent, calevent)

end

Copilot · 2025-10-16T12:00:21Z

lib/ingestors/concerns/ical_ingestor_export_url.rb

+          uri.path = File.join(uri.path, 'event.ics') unless uri.path.end_with?('/event.ics')
+        elsif uri.path&.include?('category')
+          uri.path = File.join(uri.path, 'events.ics') unless uri.path.end_with?('/events.ics')


Using File.join can produce incorrect paths when uri.path ends with a trailing slash, as it may create paths like /event//event.ics. Consider using uri.path.chomp('/') + '/event.ics' or similar string manipulation to ensure proper path formatting.

Suggested change

uri.path = File.join(uri.path, 'event.ics') unless uri.path.end_with?('/event.ics')

elsif uri.path&.include?('category')

uri.path = File.join(uri.path, 'events.ics') unless uri.path.end_with?('/events.ics')

uri.path = uri.path.chomp('/') + '/event.ics' unless uri.path.end_with?('/event.ics')

elsif uri.path&.include?('category')

uri.path = uri.path.chomp('/') + '/events.ics' unless uri.path.end_with?('/events.ics')

That's not true, when I register : https://indico.cern.ch/category/19377/ with trailing slash, export_url == https://indico.cern.ch/category/19377/events.ics no //...

lib/ingestors/concerns/ical_ingestor_export_url.rb

…tact

kennethrioja · 2025-10-27T15:07:10Z

f3f16b9 -> adds the possibility to use Indico API token to have an authenticated access to restricted events, only works for one Indico domain.

fbacall

I think the Indico ingestor should be a separate class, maybe a subclass of the Ical ingestor.

kennethrioja · 2025-12-04T10:21:04Z

Tests are in the making

fbacall

Just one minor change

fbacall · 2025-12-16T16:51:50Z

lib/ingestors/concerns/sitemap_helpers.rb

+
+      # Reads either a sitemap.{xml|txt} or a single URL
+      # Returns a list of URLs from 1 to n URLs
+      def get_sources(source_url)


Can we rename this method to parse_sitemap?

Just so it doesn't get confused with TeSS' concept of "sources"

feat(indico-ingestor): extended ical ingestor to .ics and Indico

979e51e

fbacall requested a review from Copilot October 16, 2025 11:58

Copilot AI reviewed Oct 16, 2025

View reviewed changes

kennethrioja added 2 commits October 27, 2025 14:07

review(ElixirTeSS#1161): path more precise, location check, added con…

5979514

…tact

feat(indico-ingestor): API token auth

f3f16b9

fixtests(indico-ingestor): typo

529009b

fbacall requested changes Oct 31, 2025

View reviewed changes

kennethrioja added 4 commits December 4, 2025 11:11

revert(ical_ingestor): to before the indico changes

5b9ab3f

review(ElixirTeSS#1161): Indico ingestor as a separate class

af243e3

chore(indico_ingestor): removed references to ical

19fe8c2

fix(indico_ingestor): presence, stderr

5263990

kennethrioja added 2 commits December 8, 2025 17:25

test(indico_ingestor): added tests

ab659e0

Merge branch 'master' into indico

8b0fa71

fbacall requested changes Dec 16, 2025

View reviewed changes

chore(sitemap helpers): rename get_sources to parse_sitemap

6290e70

fbacall approved these changes Dec 18, 2025

View reviewed changes

fbacall merged commit 3d70654 into ElixirTeSS:master Dec 18, 2025
7 checks passed

kennethrioja deleted the indico branch December 19, 2025 08:34

	desc = input.to_s.gsub('', '<br />')
	desc = input.to_s.gsub(/<br\s*\/?>/i, "\n")

	return if location.blank? \|\| !location.present?
	return if location.blank?

[ingestor] Extended ical ingestor to .ics and Indico #1161

[ingestor] Extended ical ingestor to .ics and Indico #1161

Uh oh!

Conversation

kennethrioja commented Oct 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kennethrioja Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kennethrioja Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kennethrioja Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kennethrioja Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kennethrioja Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kennethrioja commented Oct 27, 2025

Uh oh!

fbacall left a comment

Choose a reason for hiding this comment

Uh oh!

kennethrioja commented Dec 4, 2025

Uh oh!

fbacall left a comment

Choose a reason for hiding this comment

Uh oh!

fbacall Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kennethrioja Oct 27, 2025 •

edited

Loading

kennethrioja Oct 27, 2025 •

edited

Loading

kennethrioja Oct 27, 2025 •

edited

Loading

kennethrioja Oct 27, 2025 •

edited

Loading