Skip to content

Add support for NLS link to multisearch widget #200

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.

Sign up for GitHub

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

Merged
matt-bernhardt merged 3 commits into from
Jul 2, 2026
+107 −5
Merged

Add support for NLS link to multisearch widget #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5ce18c
Update NLS link based on cookie state
matt-bernhardt Jun 30, 2026
aa2f9db
Address automated code review feedback
matt-bernhardt Jul 2, 2026
3f29d44
Stakeholder review
matt-bernhardt Jul 2, 2026
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
99 changes: 99 additions & 0 deletions web/app/plugins/mitlib-multisearch-widget/class-multisearch-widget.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ public function widget( $args, $instance ) {

if ( $instance['targets'] == 'use' ) {

// Determine whether to enable NLS based on widget settings and the user's cookie.
$nls_enabled = $this->readCookie( $instance['nls_default'] );

$nls_link_toggle = $this->setToggleValue( $nls_enabled );

$nls_included = $instance['nls_included'];
Comment on lines +122 to +127

@matt-bernhardt matt-bernhardt Jul 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As with the other piece of feedback, I'm choosing to accept the outcome of a handful of PHP notices launched into the ether in the handful of seconds between placing this widget and the first time the configuration form is saved.

This particular block has the advantage of being self-contained, so code consistency isn't as big of a concern - but I'm also not particularly bothered given the scale of use this code will see in its lifespan.

I'm open to pushback on this during code review, but I'm going to propose this as-is to start with.


echo '<div id="search-all" class="r-tabs-panel r-tabs-state-active use" aria-labelledby="tab-all">';
include( $all_template );
echo '</div>';
Expand Down Expand Up @@ -162,6 +169,11 @@ public function form( $instance ) {
if ( '' == $instance['bento_url'] ) {
$bento_url = 'https://lib.mit.edu/';
}
$nls_default = $instance['nls_default'];
if ( '' == $instance['nls_default'] ) {
$nls_default = 'off';
}
$nls_included = $instance['nls_included'];
Comment on lines +172 to +176

@matt-bernhardt matt-bernhardt Jul 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

While I appreciate this feedback, I'm going to choose to keep these initialization assignments the same as the other values defined in this class, rather than introduce an inconsistency between these two values and the rest of what we've built.

Given that this widget will be defined once, on one site, which already swallows PHP notices, I'm willing to accept a few messages like this being thrown into the ether for the moment.

There is a future cleanup of this plugin after the new platform launches that will remove a lot of logic that will no longer be needed. It may be more appropriate to revisit which patterns we implement at that point.

?>
<p>
<label for="<?php echo esc_attr( $this->get_field_id( 'banner_text' ) ); ?>">
Expand Down Expand Up @@ -233,6 +245,56 @@ class="widefat"
name="<?php echo esc_attr( $this->get_field_name( 'bento_url' ) ); ?>"
value="<?php echo esc_html( $bento_url ); ?>">
</p>
<h3>Natural language search</h3>
<p>
Should the natural language option be shown?<br>
<label>
<input
type="checkbox"
name="<?php echo esc_attr( $this->get_field_name( 'nls_included' ) ); ?>"
value="included"
<?php
if ( 'included' == $nls_included ) {
echo "checked='checked'";
}
?>
>
Yes, include this feature.
</label>
</p>
<p>Which is the default query mode?</p>
<ul>
<li>
<label>
<input
type="radio"
name="<?php echo esc_attr( $this->get_field_name( 'nls_default' ) ); ?>"
value="off"
<?php
if ( 'off' == $nls_default ) {
echo "checked='checked'";
}
?>
>
Keyword
</label>
</li>
<li>
<label>
<input
type="radio"
name="<?php echo esc_attr( $this->get_field_name( 'nls_default' ) ); ?>"
value="on"
<?php
if ( 'on' == $nls_default ) {
echo "checked='checked'";
}
?>
>
Natural Language
</label>
</li>
</ul>
<?php
}

Expand All @@ -249,9 +311,46 @@ public function update( $new_instance, $old_instance ) {
$instance['banner_text'] = $new_instance['banner_text'];
$instance['targets'] = $new_instance['targets'];
$instance['bento_url'] = $new_instance['bento_url'];
$instance['nls_default'] = $new_instance['nls_default'];
$instance['nls_included'] = $new_instance['nls_included'];
Comment on lines +314 to +315

@matt-bernhardt matt-bernhardt Jul 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure I agree with this feedback. The nls_default field is a radio input, and is constrained to specific values already. The nls_included field is a checkbox, and is also constrained. Duplicating those constraints here introduced unhelpful replication of business logic that will need to be kept in sync if our needs ever change, without adding any security benefit.

I'm open to counter arguments during code review, but my inclination right now is to leave this as is, as with the other field handling logic in the update method.

return $instance;
}

/**
* ReadCookie looks for the 'STYXKEY_nls_enabled' domain cookie in the $_COOKIE superglobal. If no value is found, it
* returns the default value - which is defined via the widget settings form. (We use the STYXKEY prefix in the cookie
* name because this is what Pantheon allows to pass through its caching layers).
*
* @see https://docs.pantheon.io/cookies#cache-varying-cookies
*
* @param string $nls_enabled Either 'on' (for hybrid) or 'off' (for keyword) - defined in widget settings form.
*/
private function readCookie( $nls_enabled ) {
if ( array_key_exists( 'STYXKEY_nls_enabled', $_COOKIE ) ) {
if ( 'true' == $_COOKIE['STYXKEY_nls_enabled'] ) {
$nls_enabled = 'on';
} else {
$nls_enabled = 'off';
}
}

return $nls_enabled;
}

/**
* SetToggleValue determines what the state of the toggle value should be if the user decides to activate (or
* deactivate) the natural language feature.
*
* @param string $nls_enabled Either 'on' (for hybrid) or 'off' (for keyword) - by this point defined by readCookie.
*/
private function setToggleValue( $nls_enabled ) {
if ( 'on' == $nls_enabled ) {
return 'false';
}

return 'true';
}

/**
* The classes applied to the widget depend on if the banner_text property
* is set.
Expand Down
2 changes: 1 addition & 1 deletion web/app/plugins/mitlib-multisearch-widget/mitlib-multisearch-widget.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/**
* Plugin Name: MITlib Multisearch Widget
* Description: This plugin provides a widget that provides searches against multiple targets.
* Version: 1.6.0
* Version: 1.7.0
* Author: MIT Libraries
* License: GPL2
*
Expand Down
11 changes: 7 additions & 4 deletions web/app/plugins/mitlib-multisearch-widget/templates/tab-all-use.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

?>
<!-- nls state: _<?php echo esc_attr( $nls_enabled ); ?>_ -->

@matt-bernhardt matt-bernhardt Jul 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We are intentionally including this for now, for debugging purposes if it turns out we've missed something on launch day.

<form
class="form search-bento"
action="https://search.libraries.mit.edu/results"
Expand All @@ -31,8 +32,10 @@ class="field field-text"
<div>
<a href="/search-advanced/">Advanced search</a> | <a href="/search/">More ways to search</a>
</div>
<div class="nls-toggle">
<a class="toggle on" href="#">Try natural language search</a>
<a class="learn-more" href="https://search.libraries.mit.edu/about-natural-language-search">Learn more</a>
</div>
<?php if ( 'included' == $nls_included ) { ?>
<div class="nls-toggle">
<a class="toggle <?php echo esc_attr( $nls_enabled ); ?>" href="<?php echo esc_url( 'https://search.libraries.mit.edu/natural_language_search_optin?natural_language_search_optin=' . $nls_link_toggle . '&return_to=/' ); ?>">Natural language search</a>
<a class="learn-more" href="https://search.libraries.mit.edu/about-natural-language-search">Learn more</a>
</div>
<?php } ?>
</div>