Skip to content

Commit

Permalink
Finish implementing confirmation workflow
Browse files Browse the repository at this point in the history
** Why are these changes being introduced:

A previous PR started the implementation of our confirmation workflow,
but did not include a form in the view template, nor the controller
logic to receive the form submissions.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/tco-101

** How does this address that need:

This finishes the job of the previous PR, completing the confirmation
workflow. More specifically, it:

* Updates the routes information, moving the "index" display to a better
route name, and defining Confirmations as resources under Terms. This
makes dealing with routing more logical, and takes advantage of Rails'
magic.

* Moves the confirmation form out of the Terms controller, and into a
new Confirmation controller, which is where records like this should be
managed, in the Confirmation#new method.

* Form submissions are received in Confirmation#create, which is helped
by two new private methods for dealing with category values (which are
either a category_id or a "flag" boolean), and for user feedback based
on the create result.

* The site_nav and tests are updated to reflect the new controller and
route information

* The view templates are updated, most notably the new confirmation
template, which now has a working form using Rails' built-in form tool.

** Document any side effects to this change:

The confirmation form has some inline styles to quickly get the UI into
something not-awful.

Rubocop is complaining about lack of translation in the feedback
messages in the Confirmation controller, and for the complexity of the
feedback_for method.
  • Loading branch information
matt-bernhardt committed Nov 20, 2024
1 parent 74bd38b commit 05c193d
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 35 deletions.
42 changes: 42 additions & 0 deletions app/controllers/confirmation_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

class ConfirmationController < ApplicationController
def new
@term = Term.find(params[:term_id])
@categories = Category.all
@confirmation = Confirmation.new
end

def create
confirmation = Confirmation.new({
term_id: params[:term_id],
user: current_user
})
set_category(confirmation, params[:confirmation][:category])
result = confirmation.save
feedback_for(result)
end

private

def feedback_for(result)
if result == true && params[:confirmation][:category] == 'flag'
flash[:success] = 'Term flagged for review'
redirect_to terms_unconfirmed_path
elsif result == true
flash[:success] = "Term confirmed as #{Category.find_by(id: params[:confirmation][:category]).name}"
redirect_to terms_unconfirmed_path
else
flash[:warning] = 'Unable to finish confirming this term. Please try again, or try a different term.'
redirect_back_or_to terms_unconfirmed_path
end
end

def set_category(confirmation, category)
if category == 'flag'
confirmation.flag = true
else
confirmation.category_id = category
end
end
end
7 changes: 1 addition & 6 deletions app/controllers/term_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
class TermController < ApplicationController
include Pagy::Backend

def confirm_index
def unconfirmed
@pagy, @records = pagy(Term.user_unconfirmed)
end

def confirm_term
@term = Term.find(params[:id])
@categories = Category.all
end
end
36 changes: 36 additions & 0 deletions app/views/confirmation/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<h3>Term confirmation</h3>

<div class="well">
<p><%= @term.phrase %></p>
</div>
<%= form_for @confirmation, url: term_confirmation_index_path(@term) do |f| %>
<fieldset>
<legend>Please put the above term into one of the following categories:</legend>
<ul class="list-unbulleted categories">
<% @categories.all.each do |cat| %>
<li>
<%= f.radio_button :category, cat.id %>
<%= f.label(:category, cat.name, :value => cat.id) do
raw("<h4>#{cat.name}</h4><p>#{cat.description}</p>")
end %>
</li>
<% end %>
<li>
<%= f.radio_button :category, 'flag' %>
<%= f.label(:category, 'flag', :value => 'flag') do
raw("<h4>Flag for review</h4><p>Does this term need to be reviewed for removal from TACOS?</p>")
end %>
</li>
</ul>
</fieldset>
<%= f.submit %>
<% end %>

<style type="text/css">
ul.categories li {
display: flex;
flex-flow: row nowrap;
align-items: baseline;
column-gap: 1rem;
}
</style>
2 changes: 1 addition & 1 deletion app/views/layouts/_site_nav.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<%= nav_link_to("Home", root_path) %>
<% if user_signed_in? %>
<% if can? :manage, Confirmation %>
<%= link_to('Confirm terms', confirm_index_path, class: 'nav-item') %>
<%= link_to('Confirm terms', terms_unconfirmed_path, class: 'nav-item') %>
<% end %>
<% if can? :index, Term %>
<%= link_to('Admin', admin_root_path, class: 'nav-item') %>
Expand Down
19 changes: 0 additions & 19 deletions app/views/term/confirm_term.html.erb

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ take you to the form for submitting this information.</p>

<ul class="list-unbulleted">
<% @records.each do |t| %>
<li><%= link_to(t.phrase, confirm_term_path(t)) %></li>
<li><%= link_to(t.phrase, new_term_confirmation_path(t)) %></li>
<% end %>
</ul>

Expand Down
6 changes: 4 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@
get '/report/algorithm_metrics', to: 'report#algorithm_metrics'

# Confirmation interface
get '/confirm', to: 'term#confirm_index', as: 'confirm_index'
get '/confirm/:id', to: 'term#confirm_term', as: 'confirm_term'
get '/terms/unconfirmed', to: 'term#unconfirmed', as: 'terms_unconfirmed'
resources :terms do
resources :confirmation
end

# Defines the root path route ("/")
root to: 'static#index'
Expand Down
12 changes: 6 additions & 6 deletions test/controllers/term_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

class TermControllerTest < ActionDispatch::IntegrationTest
test 'confirmation index is not accessible without authentication' do
get confirm_index_path
get terms_unconfirmed_path

assert_redirected_to '/'
follow_redirect!
Expand All @@ -14,20 +14,20 @@ class TermControllerTest < ActionDispatch::IntegrationTest

test 'confirmation index is accessible to basic users when authenticated' do
sign_in users(:basic)
get confirm_index_path
get terms_unconfirmed_path

assert_response :success
end

test 'confirmation index is accessible to admin users when authenticated' do
sign_in users(:admin)
get confirm_index_path
get terms_unconfirmed_path

assert_response :success
end

test 'confirmation form is not accessible without authentication' do
get confirm_term_path(terms(:doi))
get new_term_confirmation_path(terms(:doi))

assert_redirected_to '/'
follow_redirect!
Expand All @@ -37,14 +37,14 @@ class TermControllerTest < ActionDispatch::IntegrationTest

test 'confirmation form is accessible to basic users when authenticated' do
sign_in users(:basic)
get confirm_term_path(terms(:doi))
get new_term_confirmation_path(terms(:doi))

assert_response :success
end

test 'confirmation form is accessible to admin users when authenticated' do
sign_in users(:admin)
get confirm_term_path(terms(:doi))
get new_term_confirmation_path(terms(:doi))

assert_response :success
end
Expand Down

0 comments on commit 05c193d

Please sign in to comment.