Skip to content

Commit ed66d9b

Browse files
committed
Remove GeoData from active tab logic
Why these changes are being introduced: With the implementation of the 'all' tab, which includes a `valid_tab?` method, it's become confusing as to how GeoData fits into the tab system. We currently treat it as a tab, but it's also treated as a feature. Relevant ticket(s): - [USE-176](https://mitlibraries.atlassian.net/browse/USE-176) - [USE-177](https://mitlibraries.atlassian.net/browse/USE-177) How this addresses that need: This normalizes the GeoData feature flagging in the controller, such that we only check for `Feature.enabled?` when toggling GeoData, rather than a combination of `active_tab` and the `Feature` model. Side effects of this change: Remaining instances where 'GeoData/geodata' was referred to as 'GDT/gdt' have been updated to use the actual property name.
1 parent 29f8bd9 commit ed66d9b

15 files changed

+49
-57
lines changed

app/controllers/application_controller.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ class ApplicationController < ActionController::Base
66
# We set this in a session cookie to persist user preference across searches.
77
# Clicking on a different tab will update the cookie.
88
def set_active_tab
9-
@active_tab = if Feature.enabled?(:geodata)
10-
# Determine which tab to load - default to primo unless gdt is enabled
11-
'geodata'
12-
elsif params[:tab].present? && valid_tab?(params[:tab])
9+
# GeoData doesn't use the tab system.
10+
return if Feature.enabled?(:geodata)
11+
12+
@active_tab = if params[:tab].present? && valid_tab?(params[:tab])
1313
# If params[:tab] is set and valid, use it and set session
1414
cookies[:last_tab] = params[:tab]
1515
elsif cookies[:last_tab].present? && valid_tab?(cookies[:last_tab])

app/controllers/search_controller.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ def results
1717

1818
@enhanced_query = Enhancer.new(params).enhanced_query
1919

20+
# Load GeoData results if applicable
21+
if Feature.enabled?(:geodata)
22+
load_geodata_results
23+
render 'results_geo'
24+
return
25+
end
26+
2027
# Route to appropriate search based on active tab
2128
case @active_tab
2229
when 'primo'
@@ -25,9 +32,6 @@ def results
2532
load_timdex_results
2633
when 'all'
2734
load_all_results
28-
when 'geodata'
29-
load_gdt_results
30-
render 'results_geo'
3135
end
3236
end
3337

@@ -52,7 +56,7 @@ def sleep_if_too_fast
5256
sleep(1 - duration)
5357
end
5458

55-
def load_gdt_results
59+
def load_geodata_results
5660
query = QueryBuilder.new(@enhanced_query).query
5761

5862
response = query_timdex(query)
@@ -170,7 +174,7 @@ def query_timdex(query)
170174

171175
# Builder hands off to wrapper which returns raw results here.
172176
Rails.cache.fetch("#{cache_key}/#{@active_tab}", expires_in: 12.hours) do
173-
raw = if @active_tab == 'geodata'
177+
raw = if Feature.enabled?(:geodata)
174178
execute_geospatial_query(query)
175179
elsif @active_tab == 'timdex' || @active_tab == 'all'
176180
TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query)

app/helpers/filter_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def nice_labels
3333
}
3434
end
3535

36-
def gdt_sources(value, category)
36+
def geodata_sources(value, category)
3737
return value if category != :sourceFilter
3838

3939
return 'Non-MIT institutions' if value == 'opengeometadata gis resources'

app/javascript/search_form.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function updateKeywordPlaceholder() {
5757
}
5858
}
5959

60-
// Add event listeners for all panels in the DOM. For GDT, this is currently both geospatial panels and the advanced
60+
// Add event listeners for all panels in the DOM. For GeoData, this is currently both geospatial panels and the advanced
6161
// panel. In all other TIMDEX UI apps, it's just the advanced panel.
6262
if (Array.from(allPanels).includes(geoboxPanel && geodistancePanel)) {
6363
document.getElementById('geobox-summary').addEventListener('click', () => {

app/models/feature.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111
#
1212
# @example Setting Flags in Environment
1313
# # In local ENV or Heroku config:
14-
# FEATURE_GEODATA=true # Enables the GDT feature
14+
# FEATURE_GEODATA=true # Enables the GeoData feature
1515
# FEATURE_BOOLEAN_PICKER=true # Enables the boolean picker feature
1616
#
1717
# # Any non-true value or not setting ENV does not enable the feature
18-
# FEATURE_GEODATA=false # Does not enable the GDT feature
19-
# FEATURE_GEODATA=1 # Does not enable the GDT feature
20-
# FEATURE_GEODATA=on # Does not enable the GDT feature
18+
# FEATURE_GEODATA=false # Does not enable the GeoData feature
19+
# FEATURE_GEODATA=1 # Does not enable the GeoData feature
20+
# FEATURE_GEODATA=on # Does not enable the GeoData feature
2121
#
2222
# @example Usage in Different Contexts
2323
# # In models/controllers:

app/views/record/_record_geo.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
</ul>
4242
<% end %>
4343

44-
<!-- The publishers field also includes location and date subfields, but these are unused in GDT. -->
44+
<!-- The publishers field also includes location and date subfields, but these are unused in GeoData. -->
4545
<% if @record['publishers'].present? %>
4646
<h3 class="section-title">Publishers</h3>
4747
<ul>

app/views/search/_filter.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
<span class="sr">Apply filter:</span>
1717
<% end %>
1818
<% if Feature.enabled?(:geodata) %>
19-
<span class="name"><%= gdt_sources(term['key'], category) %></span>
19+
<span class="name"><%= geodata_sources(term['key'], category) %></span>
2020
<% else %>
2121
<span class="name"><%= term['key'] %></span>
2222
<% end %>

app/views/search/_search_summary_geo.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
href="<%= results_path(remove_filter(@enhanced_query, filter.keys[0], filter.values[0])) %>">
5252
<%= "#{nice_labels[filter.keys[0]] || filter.keys[0]}:" %>
5353
<% if Feature.enabled?(:geodata) %>
54-
<%= "#{gdt_sources(filter.values[0], filter.keys[0])}" %>
54+
<%= "#{geodata_sources(filter.values[0], filter.keys[0])}" %>
5555
<% else %>
5656
<%= "#{filter.values[0]}" %>
5757
<% end %>

test/controllers/application_controller_test.rb

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,6 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
2727
end
2828
end
2929

30-
test 'set_active_tab sets to geodata when feature flag enabled even if param is passed' do
31-
skip 'Geodata uses a different form'
32-
ClimateControl.modify FEATURE_GEODATA: 'true' do
33-
get root_path, params: { tab: 'primo' }
34-
assert_select '#tab-to-target' do
35-
refute_select '[value=?]', 'primo'
36-
assert_select '[value=?]', 'all'
37-
refute_select '[value=?]', 'timdex'
38-
end
39-
end
40-
end
41-
4230
test 'set_active_tab sets to param tab when provided' do
4331
get root_path, params: { tab: 'timdex' }
4432

test/controllers/record_controller_geo_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
require 'test_helper'
22

33
class RecordControllerGeoTest < ActionDispatch::IntegrationTest
4-
test 'no access button displays if GDT feature is disabled' do
4+
test 'no access button displays if GeoData feature is disabled' do
55
gis_record_id = 'gismit:CAMBRIDGEMEMPOLES09'
66
VCR.use_cassette('gis record mit free',
77
allow_playback_repeats: true,

0 commit comments

Comments
 (0)