Skip to content

Commit

Permalink
Merge pull request #825 from DFE-Digital/pagination-bulk-search
Browse files Browse the repository at this point in the history
Add pagination for the bulk search results
  • Loading branch information
felixclack authored Sep 17, 2024
2 parents 7d75b40 + e93a2f6 commit 9fccd6c
Show file tree
Hide file tree
Showing 16 changed files with 171 additions and 18 deletions.
5 changes: 5 additions & 0 deletions app/assets/stylesheets/check_records.scss
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ h1 .govuk-tag {
}
}

.app-pagination {
display: flex;
justify-content: center;
}

@media print {
.app__no-print {
display: none;
Expand Down
25 changes: 25 additions & 0 deletions app/controllers/check_records/bulk_searches_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
require 'pagy/extras/array'

module CheckRecords
class BulkSearchesController < CheckRecordsController
include Pagy::Backend

def new
@bulk_search = BulkSearch.new
@organisation_name = current_dsi_user.dsi_user_sessions.last&.organisation_name
Expand All @@ -9,6 +13,13 @@ def create
@bulk_search = BulkSearch.new(file: bulk_search_params[:file])
@total, @results, @not_found = @bulk_search.call
if @results
@bulk_search_response = BulkSearchResponse.create!(
dsi_user: current_dsi_user,
body: { results: @results, not_found: @not_found },
total: @total,
expires_at: 30.minutes.from_now
)

BulkSearchLog.create!(
csv: @bulk_search.csv,
dsi_user_id: current_dsi_user.id,
Expand All @@ -19,7 +30,21 @@ def create
send_error_analytics_event
render :new
end
end

def show
@bulk_search_response = current_dsi_user.bulk_search_responses.find(params[:id])
if @bulk_search_response.expires_at&.past?
redirect_to new_check_records_bulk_search_path, alert: "Bulk search expired"
end

data = @bulk_search_response.body
@total = @bulk_search_response.total
@results ||= data.fetch("results", []).map do |teacher|
QualificationsApi::Teacher.new(teacher['api_data'])
end
@not_found = data['not_found'].map {|record| Hashie::Mash.new(record) }
@pagy, @results = pagy_array(@results, limit: 4)
end

private
Expand Down
2 changes: 2 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module ApplicationHelper
include Pagy::Frontend

def current_namespace
request.path.split("/").second
end
Expand Down
13 changes: 13 additions & 0 deletions app/models/bulk_search_response.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class BulkSearchResponse < ApplicationRecord
belongs_to :dsi_user

before_create :expire_other_responses

private

def expire_other_responses
BulkSearchResponse.where(dsi_user:)
.where('expires_at > ?', Time.current)
.update_all(expires_at: Time.current)
end
end
1 change: 1 addition & 0 deletions app/models/dsi_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class DsiUser < ApplicationRecord

has_many :search_logs
has_many :dsi_user_sessions, dependent: :destroy
has_many :bulk_search_responses

CURRENT_TERMS_AND_CONDITIONS_VERSION = "1.0".freeze

Expand Down
20 changes: 5 additions & 15 deletions app/views/check_records/bulk_searches/create.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,15 @@
<% content_for :breadcrumbs do %>
<%= govuk_breadcrumbs(breadcrumbs: { "Home" => check_records_search_path, "Find multiple records" => new_check_records_bulk_search_path, "Results" => nil }) %>

<%= render ActionAtComponent.new(action: "searched") %>
<% end %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<h1 class="govuk-heading-l"><%= pluralize(@total, 'teacher record') %> found</h1>
<%= govuk_tabs do |tabs|
if @results.any?
tabs.with_tab(label: "Records found", id: "records-found") do %>
<%= render "check_records/search/results_table", teachers: @results %>
<% end %>
<% end %>
<% if @not_found.any? %>
<% tabs.with_tab(label: "Not found", id: "records-not-found") do %>
<%= render "check_records/search/not_found", not_found: @not_found %>
<% end %>
<% end %>
<% end %>
<h1 class="govuk-heading-l">File summary</h1>
<p class="govuk-body">
Your CSV file includes the details of <%= pluralize(@total, 'person') %>:
</p>

<%= govuk_link_to "Search again", new_check_records_bulk_search_path %>
<%= govuk_link_to "Find records", check_records_bulk_search_path(@bulk_search_response), class: "govuk-button" %>
</div>
</div>
25 changes: 25 additions & 0 deletions app/views/check_records/bulk_searches/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<% content_for :page_title, "Bulk Search Results" %>
<% content_for :breadcrumbs do %>
<%= govuk_breadcrumbs(breadcrumbs: { "Home" => check_records_search_path, "Find multiple records" => new_check_records_bulk_search_path, "Results" => nil }) %>
<%= render ActionAtComponent.new(action: "searched") %>
<% end %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<h1 class="govuk-heading-l"><%= pluralize(@total, 'teacher record') %> found</h1>
<%= govuk_tabs do |tabs|
if @results.any?
tabs.with_tab(label: "Records found", id: "records-found") do %>
<%= render "check_records/search/results_table", teachers: @results %>
<% end %>
<% end %>
<% if @not_found.any? %>
<% tabs.with_tab(label: "Not found", id: "records-not-found") do %>
<%= render "check_records/search/not_found", not_found: @not_found %>
<% end %>
<% end %>
<% end %>

<%= govuk_link_to "Search again", new_check_records_bulk_search_path %>
</div>
</div>
2 changes: 1 addition & 1 deletion app/views/check_records/search/_not_found.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
not_found.each do |record|
body.with_row do |row|
row.with_cell(text: record.trn)
row.with_cell(text: record.date_of_birth.to_fs(:long_uk))
row.with_cell(text: record.date_of_birth.to_date.to_fs(:long_uk))
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions app/views/check_records/search/_results_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,11 @@
end
end %>
<% end %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<div class="app-pagination">
<%= govuk_pagination(pagy: @pagy) if @pagy.pages.positive? %>
</div>
</div>
</div>
</div>
8 changes: 8 additions & 0 deletions config/analytics_blocklist.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,11 @@
- internal
- created_at
- updated_at
:bulk_search_responses:
- id
- body
- expires_at
- dsi_user_id
- total
- created_at
- updated_at
2 changes: 1 addition & 1 deletion config/routes/check_records.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
patch "/terms-and-conditions" => "terms_and_conditions#update"

resources :teachers, only: %i[show]
resources :bulk_searches, only: %i[new create], path: 'bulk-searches'
resources :bulk_searches, only: %i[new create show], path: 'bulk-searches'

scope "/feedback" do
get "/", to: "feedbacks#new", as: :feedbacks
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20240917065402_create_bulk_search_responses.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreateBulkSearchResponses < ActiveRecord::Migration[7.2]
def change
create_table :bulk_search_responses, id: :uuid do |t|
t.jsonb :body
t.datetime :expires_at
t.references :dsi_user, null: false, foreign_key: true
t.integer :total

t.timestamps
end
end
end
13 changes: 12 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2024_08_29_103510) do
ActiveRecord::Schema[7.2].define(version: 2024_09_17_065402) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

Expand Down Expand Up @@ -63,6 +63,16 @@
t.index ["dsi_user_id"], name: "index_bulk_search_logs_on_dsi_user_id"
end

create_table "bulk_search_responses", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.jsonb "body"
t.datetime "expires_at"
t.bigint "dsi_user_id", null: false
t.integer "total"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["dsi_user_id"], name: "index_bulk_search_responses_on_dsi_user_id"
end

create_table "console1984_commands", force: :cascade do |t|
t.text "statements"
t.bigint "sensitive_access_id"
Expand Down Expand Up @@ -211,6 +221,7 @@
add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
add_foreign_key "bulk_search_logs", "dsi_users"
add_foreign_key "bulk_search_responses", "dsi_users"
add_foreign_key "date_of_birth_changes", "users"
add_foreign_key "search_logs", "dsi_users"
end
8 changes: 8 additions & 0 deletions spec/factories/bulk_search_responses.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FactoryBot.define do
factory :bulk_search_response do
association :dsi_user
body { { results: [], not_found: [] } }
total { 0 }
expires_at { 30.minutes.from_now }
end
end
33 changes: 33 additions & 0 deletions spec/models/bulk_search_response_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
require 'rails_helper'

RSpec.describe BulkSearchResponse, type: :model do
describe 'callbacks' do
describe '#expire_other_responses' do
let(:dsi_user) { create(:dsi_user) }
let!(:old_response) { create(:bulk_search_response, dsi_user:, expires_at: 1.hour.from_now) }

it 'expires other responses for the same user' do
expect {
create(:bulk_search_response, dsi_user:)
}.to change { old_response.reload.expires_at }.to be_within(1.second).of(Time.current)
end

it 'does not expire responses for other users' do
other_user = create(:dsi_user)
other_response = create(:bulk_search_response, dsi_user: other_user, expires_at: 1.hour.from_now)

expect {
create(:bulk_search_response, dsi_user:)
}.not_to(change { other_response.reload.expires_at })
end

it 'does not affect already expired responses' do
expired_response = create(:bulk_search_response, dsi_user:, expires_at: 1.hour.ago)

expect {
create(:bulk_search_response, dsi_user:)
}.not_to(change { expired_response.reload.expires_at })
end
end
end
end
13 changes: 13 additions & 0 deletions spec/system/check_records/user_searches_with_a_valid_csv_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
when_i_sign_in_via_dsi
and_search_with_a_valid_csv
then_i_see_the_results_summary
when_i_click_find_records
then_i_see_results
and_my_search_is_logged
end

Expand All @@ -22,7 +24,18 @@ def and_search_with_a_valid_csv
end

def then_i_see_the_results_summary
expect(page).to have_content "Your CSV file includes the details of 1 person"
end

def when_i_click_find_records
click_link "Find records"
end

def then_i_see_results
expect(page).to have_content "1 teacher record found"
expect(page).to have_content "Terry Walsh"
expect(page).to have_content "Restriction"
expect(page).to have_content "No induction"
end

def and_my_search_is_logged
Expand Down

0 comments on commit 9fccd6c

Please sign in to comment.