Vfep 958 - add ability to maintain keywords associated with accrediat…

…ion types (#1015)

* refactor tests to run faster

* further refactor tests for performance

* Add accreditation action keyword functionality

* fix rubocop error

---------

Co-authored-by: nfstern02 <[email protected]>

.rubocop.yml

@@ -161,9 +161,10 @@ Metrics/ModuleLength:
 Metrics/BlockLength:
   Max: 40
   Exclude:
-    - "spec/**/*"
-    - "rakelib/**/*.rake"
     - "config/**/*.rb"
+    - "lib/tasks/**/*.rake"
+    - "rakelib/**/*.rake"
+    - "spec/**/*"
 
 Metrics/CyclomaticComplexity:
   Exclude:
@@ -234,8 +235,7 @@ RSpec/DescribeClass:
 RSpec/ExampleLength:
   Max: 15
   Exclude:
-    - "spec/models/institution_program_spec.rb"
-    - "spec/models/institution_builder_spec.rb"
+    - "spec/models/institution_builder/*.rb"
 
 # Determined to be too benign and/or numerous to justify changing
 RSpec/LeakyConstantDeclaration:

app/controllers/accreditation_type_keywords_controller.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+class AccreditationTypeKeywordsController < ApplicationController
+  before_action :set_accreditation_type, only: %i[index new destroy]
+
+  # GET /accreditation_type_keywords
+  def index
+    @accreditation_type_keywords =
+      AccreditationTypeKeyword
+      .where(accreditation_type: @accreditation_type)
+      .order(:keyword_match)
+  end
+
+  # GET /accreditation_type_keywords/new
+  def new
+    @accreditation_type_keyword = AccreditationTypeKeyword.new
+  end
+
+  # POST /accreditation_type_keywords
+  def create
+    @accreditation_type_keyword = AccreditationTypeKeyword.new(accreditation_type_keyword_params)
+    @accreditation_type = @accreditation_type_keyword.accreditation_type
+
+    if @accreditation_type_keyword.valid?
+      @accreditation_type_keyword.save
+      respond_to do |format|
+        format.html { redirect_to accreditation_type_keywords_path }
+        format.js
+      end
+    else
+      respond_to do |format|
+        format.html { redirect_to accreditation_type_keywords_path }
+        format.js { render action: 'new_with_errors' }
+      end
+    end
+  end
+
+  # DELETE /accreditation_type_keywords/1
+  def destroy
+    @accreditation_type_keyword = AccreditationTypeKeyword.find(params[:id])
+    @accreditation_type = @accreditation_type_keyword.accreditation_type
+    @accreditation_type_keyword.destroy
+    respond_to do |format|
+      format.html { redirect_to accreditation_type_keywords_path, notice: 'Keyword was successfully destroyed.' }
+      format.js
+    end
+  end
+
+  private
+
+  # Use callbacks to share common setup or constraints between actions.
+  def set_accreditation_type
+    @accreditation_type = params[:accreditation_type]
+  end
+
+  # Only allow a list of trusted parameters through.
+  def accreditation_type_keyword_params
+    params.require(:accreditation_type_keyword).permit(:accreditation_type, :keyword_match)
+  end
+end

app/helpers/application_helper.rb

@@ -1,6 +1,13 @@
 # frozen_string_literal: true
 
 module ApplicationHelper
+  def controller_label_for_header
+    return controller.controller_name.humanize.singularize unless
+           controller.controller_name.eql?('accreditation_type_keywords')
+
+    'Accreditation keyword'
+  end
+
   def active_link?(path, method = 'GET')
     begin
       h = Rails.application.routes.recognize_path(path, method: method)

app/helpers/dashboards_helper.rb

@@ -59,7 +59,11 @@ def locked_fetches_exist?
     Upload.locked_fetches_exist?
   end
 
-  def formatted_keywords(value)
-    value.gsub('[', '').gsub(']', '').gsub('/i', '').gsub('/', '')
+  def formatted_keywords(accreditation_type)
+    AccreditationTypeKeyword
+      .where(accreditation_type: accreditation_type)
+      .order(:keyword_match)
+      .pluck(:keyword_match)
+      .join(', ')
   end
 end

app/models/accreditation_record.rb

@@ -4,6 +4,8 @@ class AccreditationRecord < ImportableRecord
   belongs_to(:accreditation_institute_campus, foreign_key: 'dapip_id', primary_key: :dapip_id,
                                               inverse_of: :accreditation_records)
 
+  belongs_to(:accreditation_type_keyword, inverse_of: :accreditation_records)
+
   CSV_CONVERTER_INFO = {
     'dapipid' => { column: :dapip_id, converter: NumberConverter },
     'agencyid' => { column: :agency_id, converter: NumberConverter },
@@ -20,44 +22,38 @@ class AccreditationRecord < ImportableRecord
     'endingactionid' => { column: :ending_action_id, converter: NumberConverter }
   }.freeze
 
-  # The ACCREDITATIONS hash maps accreditation types (Regional, National, or
-  # Hybrid) to substrings in the name of the accrediting body. So, for example,
-  # if the accrediting agency is the "New England Medical Association", then
-  # the accreditation is 'Regional'.
-
-  # vfep-439 - add northwest
-  ACCREDITATIONS = {
-    'regional' => [/middle/i, /new england/i, /north central/i, /southern/i, /western/i,
-                   /higher learning commission/i, /wasc/i, /northwest/i],
-    'national' => [/career schools/i, /continuing education/i, /independent colleges/i,
-                   /biblical/i, /occupational/i, /distance/i, /new york/i, /transnational/i],
-    'hybrid' => [/acupuncture/i, /nursing/i, /health education/i, /liberal/i, /legal/i,
-                 /funeral/i, /osteopathic/i, /pediatric/i, /theological/i, /massage/i, /radiologic/i,
-                 /midwifery/i, /montessori/i, /career arts/i, /design/i, /dance/i, /music/i,
-                 /theatre/i, /chiropractic/i]
-  }.freeze
-
   validates :dapip_id, presence: true
   validates :agency_id, presence: true
   validates :agency_name, presence: true
   validates :program_id, presence: true
   validates :program_name, presence: true
 
+  delegate :accreditation_type, to: :accreditation_type_keyword, allow_nil: true
+
   after_initialize :set_accreditation_type
 
+  def type_of_accreditation
+    accreditation_type_keyword&.accreditation_type
+  end
+
   private
 
   def set_accreditation_type
-    self.accreditation_type = to_accreditation_type
-  end
-
-  def to_accreditation_type
     return if agency_name.blank?
 
-    ACCREDITATIONS.each_pair do |type, regexp_array|
-      return type if regexp_array.find { |regexp| agency_name.match(regexp) }
+    # The order matters. regional, national, hybrid - stop at the first match.
+    AccreditationTypeKeyword::ACCREDITATION_TYPES.each do |accreditation_type|
+      find_accreditation_type(AccreditationTypeKeyword.where(accreditation_type: accreditation_type))
+      break if accreditation_type_keyword
     end
+  end
 
-    nil
+  def find_accreditation_type(accreditation_type_keywords)
+    accreditation_type_keywords.each do |atk|
+      next unless agency_name.downcase.include?(atk.keyword_match)
+
+      self.accreditation_type_keyword = atk
+      break
+    end
   end
 end

app/models/accreditation_type_keyword.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class AccreditationTypeKeyword < ApplicationRecord
+  has_many(:accreditation_records, inverse_of: :accreditation_type_keyword, dependent: :nullify)
+
+  # Order matters. The user should be asked about where to add any new ones.
+  ACCREDITATION_TYPES = %w[regional national hybrid].freeze
+
+  validates :accreditation_type, presence: true
+  validates :accreditation_type, inclusion: { in: ACCREDITATION_TYPES }
+  validates :keyword_match, presence: true
+  validates :keyword_match, uniqueness: { scope: :accreditation_type }
+end

app/models/institution_builder.rb

@@ -236,21 +236,21 @@ def self.add_accreditation(version_id)
 
       # Set the `accreditation_type`
       str = <<-SQL
-        UPDATE institutions SET
-          accreditation_type = accreditation_records.accreditation_type
-        FROM accreditation_institute_campuses, accreditation_records
-        WHERE institutions.ope = accreditation_institute_campuses.ope
-          AND accreditation_institute_campuses.dapip_id = accreditation_records.dapip_id
-          AND institutions.ope IS NOT NULL
-          AND accreditation_records.accreditation_end_date IS NULL
-          AND accreditation_records.program_id = 1
-          AND institutions.version_id = #{version_id}
-          AND accreditation_records.accreditation_type = {{ACC_TYPE}};
+        UPDATE institutions
+           SET accreditation_type = accreditation_type_keywords.accreditation_type
+          FROM accreditation_institute_campuses
+             , accreditation_records
+             , accreditation_type_keywords
+         WHERE institutions.version_id = #{version_id}
+           AND institutions.ope IS NOT NULL
+           AND institutions.ope = accreditation_institute_campuses.ope
+           AND accreditation_institute_campuses.dapip_id = accreditation_records.dapip_id
+           AND accreditation_records.accreditation_end_date IS NULL
+           AND accreditation_records.program_id = 1
+           AND accreditation_records.accreditation_type_keyword_id = accreditation_type_keywords.id
       SQL
 
-      %w[hybrid national regional].each do |acc_type|
-        Institution.connection.update(str.gsub('{{ACC_TYPE}}', "'#{acc_type}'"))
-      end
+      Institution.connection.update(str)
 
       where_clause = <<-SQL
           WHERE institutions.ope = accreditation_institute_campuses.ope

app/views/accreditation_type_keywords/_form.html.erb

@@ -0,0 +1,22 @@
+<%= form_for @accreditation_type_keyword, remote: true do |form| %>
+  <div class="field">
+    <%= form.hidden_field :accreditation_type, value: @accreditation_type %>
+  </div>
+
+
+  <div>
+    <%= form.label :keyword %>
+    <%= form.text_field :keyword_match %>
+    <%= form.submit 'Save', class: 'btn btn-primary' %>
+  </div>
+
+  <% if @accreditation_type_keyword.errors.any? %>
+    <div>
+      <ul>
+        <% @accreditation_type_keyword.errors.each do |error| %>
+          <li><%= error.full_message %></li>
+        <% end %>
+      </ul>
+    </div>
+  <% end %>
+<% end %>

app/views/accreditation_type_keywords/_keyword.html.erb

@@ -0,0 +1,4 @@
+<tr id="accreditation-type-keyword-<%= accreditation_type_keyword.id %>">
+  <td><%= accreditation_type_keyword.keyword_match %></td>
+  <td>&nbsp;&nbsp;<%= link_to 'Delete', accreditation_type_keyword, method: :delete, data: { confirm: 'Are you sure?', remote: true } %></td>
+</tr>

app/views/accreditation_type_keywords/create.js.erb

@@ -0,0 +1,3 @@
+$('#new_accreditation_type_keyword').remove();
+$('#add-keyword-link').show();
+$('#keywords').append('<%= j render partial: 'keyword', locals: {accreditation_type_keyword: @accreditation_type_keyword} %>');

app/views/accreditation_type_keywords/destroy.js.erb

@@ -0,0 +1 @@
+$('#accreditation-type-keyword-<%= @accreditation_type_keyword.id %>').remove();

app/views/accreditation_type_keywords/index.html.erb

@@ -0,0 +1,20 @@
+<p><%= link_to 'Back', dashboard_accreditation_issues_path %></p>
+
+<h1>Keywords for <%= @accreditation_type %> accreditation</h1>
+
+<table>
+  <tbody id="keywords">
+    <% @accreditation_type_keywords.each do |accreditation_type_keyword| %>
+      <%= render partial: 'keyword', locals: {accreditation_type_keyword: accreditation_type_keyword} %>
+    <% end %>
+  </tbody>
+</table>
+
+<br>
+
+<%= 
+  link_to 'Add Keyword', 
+    new_accreditation_type_keyword_path(accreditation_type: @accreditation_type = params[:accreditation_type]),
+      id: 'add-keyword-link',
+      remote: true 
+%>

app/views/accreditation_type_keywords/new.js.erb

@@ -0,0 +1 @@
+$('#add-keyword-link').hide().after('<%= j render("form") %>');

app/views/accreditation_type_keywords/new_with_errors.js.erb

@@ -0,0 +1 @@
+$('#new_accreditation_type_keyword').replaceWith('<%= j render("form") %>');

app/views/dashboards/accreditation_issues.html.erb

@@ -10,10 +10,13 @@
         - accreditation types are determined by joining Institutions to AccreditationInstituteCampuses on ope<br/>
         - AccreditationInstituteCampuses are joined to AccreditationRecords on dapip_id<br/>
         - the AccreditationRecord must not have an accreditation_end_date and have a program_id of 1<br/>
-        - the agency_name must contain one of the following keywords:<br/>
-           <% AccreditationRecord::ACCREDITATIONS.each_pair do |key, value| %>
-               <b>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<%= "#{key}: " %></b>
-               <%= "#{formatted_keywords(value.to_s)}" %><br/>
+        - the agency_name must contain one of the following keywords:<br/><br/>
+           <% AccreditationTypeKeyword::ACCREDITATION_TYPES.each do |accreditation_type| %>
+              &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
+              <%= link_to 'Maintain', accreditation_type_keywords_path(accreditation_type: accreditation_type),
+                  class: "btn dashboard-btn-success btn-xs", role: "button" %>
+              <b><%= "#{accreditation_type}:" %></b>
+              <%= "#{formatted_keywords(accreditation_type)}" %><br/><br/>
            <% end %>
         - if any of the above conditions are not met, the institution will appear on this report
     </div><br/>

app/views/layouts/_header.html.erb

@@ -1,3 +1,3 @@
 <div class="page-header">
-  <h1>GI Bill Comparison Tool <small class="dashboard-header"><%=controller.controller_name.humanize.singularize %></small></h1>
+  <h1>GI Bill Comparison Tool <small class="dashboard-header"><%= controller_label_for_header %></small></h1>
 </div>

config/environment.rb

@@ -1,5 +1,11 @@
 # Load the Rails application.
 require_relative 'application'
 
+# get rid of annoying field_with_errors wrapper
+# https://coderwall.com/p/s-zwrg/remove-rails-field_with_errors-wrapper
+ActionView::Base.field_error_proc = Proc.new do |html_tag, instance|
+  html_tag.html_safe
+end
+
 # Initialize the Rails application.
 Rails.application.initialize!

config/puma.rb

@@ -25,10 +25,8 @@
 # the given timeout. If not the worker process will be restarted. Default
 # value is 60 seconds.
 #
-worker_timeout 60
 
-if ENV['RACK_ENV'] == 'development'
-  worker_timeout 3600
-end
+worker_timeout(60)
+worker_timeout(3600) if ENV['RAILS_ENV'].eql?('development')
 
 preload_app!

config/routes.rb

@@ -25,6 +25,8 @@
   get '/dashboards/accreditation_issues' => 'dashboards#accreditation_issues', as: :dashboard_accreditation_issues
   get '/unlock_fetches' => 'dashboards#unlock_fetches', as: :unlock_fetches
 
+  resources :accreditation_type_keywords, only: [:index, :new, :create, :destroy]
+
   resources :uploads, except: [:new, :destroy, :edit, :update] do
     get '(:csv_type)' => 'uploads#new', on: :new, as: ''
   end

db/migrate/20231118074354_create_accreditation_type_keywords.rb

@@ -0,0 +1,12 @@
+class CreateAccreditationTypeKeywords < ActiveRecord::Migration[6.1]
+  def change
+    create_table :accreditation_type_keywords do |t|
+      t.string :accreditation_type
+      t.string :keyword_match
+
+      t.timestamps
+    end
+
+    add_index :accreditation_type_keywords, [:accreditation_type, :keyword_match], name: 'index_type_and_keyword_match', unique: true
+  end
+end

db/migrate/20231118091720_add_type_keyword_to_acrreditation_record.rb

@@ -0,0 +1,14 @@
+class AddTypeKeywordToAcrreditationRecord < ActiveRecord::Migration[6.1]
+  disable_ddl_transaction!
+
+  def change
+    add_column :accreditation_records, :accreditation_type_keyword_id, :bigint, null: true
+    add_index :accreditation_records, :accreditation_type_keyword_id, algorithm: :concurrently
+
+    add_foreign_key :accreditation_records,
+                    :accreditation_type_keywords,
+                    column: :accreditation_type_keyword_id,
+                    validate: false,
+                    on_delete: :nullify
+  end
+end

db/migrate/20231121083148_remove_accreditation_type_from_accreditation_record.rb

@@ -0,0 +1,9 @@
+class RemoveAccreditationTypeFromAccreditationRecord < ActiveRecord::Migration[6.1]
+  # The strong migrations gem indicated to be safe that we need to add
+  # self.ignored_columns = ["accreditation_type"] to the AccreditationRecord model.
+  # I think we can safely ignore that as the table is not heavily used.
+  # However, safety_assured is still required.
+  def change
+    safety_assured { remove_column :accreditation_records, :accreditation_type, :string }
+  end
+end