Enable TACOS integration #228

matt-bernhardt · 2025-10-02T15:56:22Z

This establishes a connection from USE UI to TACOS, allowing all instances of the application to separate their traffic for analysis within TACOS using a TACOS_SOURCE environment variable.

The strategy is similar to other integrations, defining a separate route that the UI will query via Hotwire. For now, the response is only an HTML comment, but USE-65 will make that into something real.

The Tacos model added here is similar to - but not identical to - the Tacos model being used in Bento. The two big changes are:

The ability to define the sourceSystem via an ENV, since we have multiple instances of this application and only one Bento.
ENV is read via private convenience methods, and not assigned via constants. This allows us to consistently use one approach (all convenience methods), and also to test the behavior by varying the env during testing (I ran into issues with ClimateControl trying to modify frozen constants when this wasn't the first time that code had been executed in a test run)

Developer

Ticket: https://mitlibraries.atlassian.net/browse/USE-66

Accessibility

To be clear, because the only response from TACOS for now is meant to be invisible to the user, I've not checked any a11y considerations to that panel appearing.

ANDI or WAVE has been run in accordance to our guide.
This PR contains no changes to the view layer.
New issues flagged by ANDI or WAVE have been resolved.
New issues flagged by ANDI or WAVE have been ticketed (link in the Pull Request details above).
No new accessibility issues have been flagged.

New ENV

The ENV is defined in the PR build, and documented in the README - but I've not yet modified the pipeline. That will come after PR approval.

All new ENV is documented in README.
All new ENV has been added to Heroku Pipeline, Staging and Prod.
ENV has not changed.

Approval beyond code review

UXWS/stakeholder approval has been confirmed.
UXWS/stakeholder review will be completed retroactively.
UXWS/stakeholder review is not needed.

Additional context needed to review

The PR build associated with this branch has been configured to communicate with the staging TACOS, if you want to poke around a bit.

Code Reviewer

Code

I have confirmed that the code works as intended.
Any CodeClimate issues have been fixed or confirmed as
added technical debt.

Documentation

The commit message is clear and follows our guidelines
(not just this pull request message).
The documentation has been updated or is unnecessary.
New dependencies are appropriate or there were no changes.

Testing

There are appropriate tests covering any new functionality.
No additional test coverage is required.

coveralls · 2025-10-02T15:58:01Z

Pull Request Test Coverage Report for Build 18319309158

Details

30 of 30 (100.0%) changed or added relevant lines in 3 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.09%) to 98.905%

Files with Coverage Reduction	New Missed Lines	%
app/controllers/search_controller.rb	1	98.25%

Totals
Change from base Build 18288714704:	-0.09%
Covered Lines:	813
Relevant Lines:	822

💛 - Coveralls

** Why are these changes being introduced: The search UI needs to be connected with TACOS, in order to get insight about interventions and allow term categorization to continue. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-66 ** How does this address that need: This adds a route, controller, and model for the integration with TACOS, along with defining some new environment variables. The approach is similar, but not identical, to the integration present in the Bento app. ** Document any side effects to this change: One oddity I can't explain is why the "tacos" cassette doesn't have any record of a call to TACOS - only the call to TIMDEX for the requested search is present. The related test passes, so everything worked and the UI element is present - but no interaction is present in the cassette (and thus made by the application when tested).

** Why are these changes being introduced: I realized that the Tacos model as originally proposed had no error handling when I refreshed a results page without my local TACOS app running. The result was a pretty ugly blob shown to the user. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-66 ** How does this address that need: This adds two catch statements to the call to TACOS - one for an HTTP error response, and one for a JSON parsing error. It also refactors the Tacos model to allow a separately-defined HTTP object, which we craft in the test suite to always return those two error conditions. ** Document any side effects to this change: This is yet another pattern for error handling, not using VCR cassettes or webmock, neither of which I could get working this afternoon. I'm not happy about that, but this does work.

JPrevost · 2025-10-06T18:49:20Z

app/helpers/application_helper.rb

@@ -1,4 +1,9 @@
 module ApplicationHelper
+  def tacos_enabled?
+    ENV.fetch('TACOS_URL', '').length > 0


ENV.fetch('TACOS_URL', '').present? is more idiomatic in our codebase. (A blank string is not present, yeah it's sort of weird but also kind of makes sense)

I'm generally happy to go with the idiomatic approach, unless it doesn't work

JPrevost · 2025-10-06T18:56:12Z

app/models/tacos.rb

+  # The tacos_client argument here is unused in production - it is provided for
+  # our test suite so that we can mock various error conditions to ensure that
+  # error handling happens as we intend.
+  def self.call(term, tacos_client = nil)


I'm not a fan of the method name call here. Running any method is calling it so naming something call feels like we are calling the call method and my brain stops working.

Alternative ideas:

align it with the Controller name of analyze

align it with TACOS graphQL method logSearchEvent

align it with an abbreviated TACOS log

I think I prefer the first of those, analyze because the logging returns data based on analysis. So naming this Tacos.analyze feels like saying Hey Tacos, can you analyze this string which feels pretty good to me.

This is not blocking per se, but I would encourage you to consider if there are better name options (whether it is one I suggested or not).

I'm not particularly devoted to .call as a method name - I think I inherited this when I grabbed the class from Bento. I'll take a quick look in context, but initially I like .analyze

JPrevost · 2025-10-06T19:03:40Z

app/models/tacos.rb

+  end
+
+  def self.tacos_source
+    ENV.fetch('TACOS_SOURCE', 'unset')


What are you thoughts on the default here being timdexui_unset or useui_unset instead of just unset?

My thinking is this may help us in the future understand if we are seeing a lot of odd source data at least which code base it is coming from so we can more quickly track down which deployed system has incorrect config.

Non-blocking.

Ooh, I like this suggestion - having a value that's specific to the codebase as a fallback should an instance-specific value not be defined feels pretty good. Thanks, will change.

* Use more idiomatic .present? method in tacos_enabled? helper * Update the default TACOS_SOURCE value to be specific to this codebase, rather than a generic "unset" * Rename the Tacos model method .call to .analyze

matt-bernhardt · 2025-10-07T16:34:19Z

Okay @JPrevost - this is ready for another review. Thanks for the suggestions, I liked all of them.

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 2, 2025 15:58 Inactive

matt-bernhardt force-pushed the use-66-tacos branch from 196be47 to 6a14fb5 Compare October 3, 2025 14:57

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 3, 2025 14:58 Inactive

matt-bernhardt force-pushed the use-66-tacos branch from 6a14fb5 to 7e79cd3 Compare October 3, 2025 15:04

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 3, 2025 15:05 Inactive

matt-bernhardt force-pushed the use-66-tacos branch from 7e79cd3 to 425e623 Compare October 3, 2025 15:22

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 3, 2025 15:22 Inactive

matt-bernhardt marked this pull request as ready for review October 3, 2025 16:29

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 3, 2025 19:17 Inactive

matt-bernhardt requested review from JPrevost and jazairi October 3, 2025 19:18

matt-bernhardt added 2 commits October 6, 2025 14:17

matt-bernhardt force-pushed the use-66-tacos branch from f570f78 to 720ff31 Compare October 6, 2025 18:18

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 6, 2025 18:18 Inactive

JPrevost self-assigned this Oct 6, 2025

JPrevost reviewed Oct 6, 2025

View reviewed changes

Respond to code review feedback

a11f6ff

* Use more idiomatic .present? method in tacos_enabled? helper * Update the default TACOS_SOURCE value to be specific to this codebase, rather than a generic "unset" * Rename the Tacos model method .call to .analyze

mitlib temporarily deployed to timdex-ui-pi-use-66-tac-x087ir October 7, 2025 16:24 Inactive

JPrevost approved these changes Oct 7, 2025

View reviewed changes

matt-bernhardt merged commit 5ec7532 into main Oct 7, 2025
5 checks passed

matt-bernhardt deleted the use-66-tacos branch October 7, 2025 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enable TACOS integration #228

Enable TACOS integration #228

Uh oh!

matt-bernhardt commented Oct 2, 2025 •

edited

Loading

Uh oh!

coveralls commented Oct 2, 2025 •

edited

Loading

Uh oh!

JPrevost Oct 6, 2025

Uh oh!

matt-bernhardt Oct 7, 2025

Uh oh!

JPrevost Oct 6, 2025

Uh oh!

matt-bernhardt Oct 7, 2025

Uh oh!

JPrevost Oct 6, 2025

Uh oh!

matt-bernhardt Oct 7, 2025

Uh oh!

matt-bernhardt commented Oct 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Enable TACOS integration #228

Enable TACOS integration #228

Uh oh!

Conversation

matt-bernhardt commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Developer

Accessibility

New ENV

Approval beyond code review

Additional context needed to review

Code Reviewer

Code

Documentation

Testing

Uh oh!

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

Uh oh!

Pull Request Test Coverage Report for Build 18319309158

Details

💛 - Coveralls

Uh oh!

JPrevost Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

matt-bernhardt Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

JPrevost Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

matt-bernhardt Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

JPrevost Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

matt-bernhardt Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

matt-bernhardt commented Oct 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matt-bernhardt commented Oct 2, 2025 •

edited

Loading

coveralls commented Oct 2, 2025 •

edited

Loading