Skip to content
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.

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

Add doc for explaining Finder Content Item #155

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class ApplicationController < ActionController::Base
# For APIs, you may want to use :null_session instead.
protect_from_forgery with: :exception
helper :application
rescue_from GdsApi::HTTPNotFound, with: :error_not_found

private
def set_slimmer_headers
Expand All @@ -31,4 +32,7 @@ def finder_slug
raise NotImplementedError
end

def error_not_found
render status: :not_found, text: "404 error not found"
end
end
16 changes: 3 additions & 13 deletions app/controllers/email_alert_subscriptions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ class EmailAlertSubscriptionsController < ApplicationController
protect_from_forgery except: :create

def new
if content
@signup = signup_presenter
else
error_not_found
end
@signup = signup_presenter
end

def create
Expand All @@ -34,15 +30,15 @@ def signup_presenter
end

def content
@content ||= content_store.content_item(request.path)
@content ||= content_store.content_item!(request.path)
end

def finder_slug
params[:slug]
end

def finder
FinderPresenter.new(content_store.content_item("/#{finder_slug}"))
FinderPresenter.new(content_store.content_item!("/#{finder_slug}"))
end

def finder_format
Expand Down Expand Up @@ -73,10 +69,4 @@ def email_signup_attributes
"filter" => chosen_options,
}
end

def error_not_found
render status: :not_found, text: "404 error not found"
end

end

2 changes: 1 addition & 1 deletion app/controllers/finders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def show
private
def finder
@finder ||= FinderPresenter.new(
content_store.content_item("/#{finder_slug}"),
content_store.content_item!("/#{finder_slug}"),
facet_params,
keywords,
)
Expand Down
61 changes: 61 additions & 0 deletions docs/finder-content-item.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# The Finder Content Item Format

A Finder Content Item is a specialisation of the [Content Item](https://github.com/alphagov/content-store/blob/master/doc/content_item_fields.md). This guide explains what goes in the details hash of the ContentItem and why.

The Finder Content Item is used by Finder Frontend to render the Finder page. Most of what it uses goes in the `details` hash.

# The `details` hash

## `beta`

A boolean. Required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this required? Can't we just default it to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but I thought I'd pass this on what currently is required and change it when I refactor the App.


A flag used to decide if the Beta banner should be rendered.

## `beta_message`

A string. Optional. Can be set to `null`.

Can contain HTML. If `beta` is true, `beta_message` will be passed to the beta banner.

## `document_noun`

A string. Required.

The lowercase singular version of whatever format the Finder is using. For example: [`/cma-cases`](https://www.gov.uk/cma-cases) has a `document_noun` of `case`, [`/aaib-report`](https://www.gov.uk/aaib-reports) has a `document_noun` of `report`. This is used to construct the sentence descriving the current search by the user.

## `document_type`

A string. Required.

[snake_case](http://en.wikipedia.org/wiki/Snake_case) string which tells Finder Frontend what doctype to limit the search to in Rummager. It must match the name of the file describing the doctype [in Rummager](https://github.com/alphagov/rummager/tree/master/config/schema/default/doctypes).
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that it needs to match the doctype in Rummager, we probably don't need to stipulate here that should be snake_case.


## `email_signup_enabled`

A boolean. Required.

Used to decide if the link to the email alert signup page should be displayed

## `format_name`

A string. Optional.

Not specifically used by the Finder, but used by [Specialist Frontend](https://github.com/alphagov/specialist-frontend) to link back to the Finder.
Usually a singularised version of the title of the Finder - `"Competition and Markets Authority case"` for [`/cma-cases`](https://www.gov.uk/cma-cases) for example.
However there are edge cases where it's not the same such as `"Medical safety alert"` for [Alerts and recalls for drugs and medical devices](https://www.gov.uk/drug-device-alerts).

## `signup_link`

A string. Optional.

If `email_signup_enabled` is set to true, the link being displayed will point to `base_path/email-signup` where `base_path` is from the Finder object. `signup_link` allows you to point it at a different URL, [Drug Safety Update](https://www.gov.uk/drug-safety-update) and [Drug Device Alerts](https://www.gov.uk/drug-device-alerts) are the two which currently use this feature.

## `show_summaries`

A boolean. Required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we default this to false rather than requiring it, as that's the behaviour for most finders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I thought I'd explain what the current case is and alter this when I refactor it.


Used to decide if the summaries for Documents should be displayed in the results list. It will truncate the summary at the end of the first sentence.

## facets

__TO DO__
11 changes: 11 additions & 0 deletions spec/controllers/email_alert_subscriptions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@

describe EmailAlertSubscriptionsController do

describe 'GET #new' do
describe "finder email signup item doesn't exist" do
it 'returns a 404, rather than 5xx' do
content_store_does_not_have_item('/does-not-exist/email-signup')

get :new, slug: 'does-not-exist'
expect(response.status).to eq(404)
end
end
end

describe 'POST "#create"' do
let(:alert_name) { double(:alert_name) }
let(:alert_identifier) { double(:alert_identifier) }
Expand Down
17 changes: 17 additions & 0 deletions spec/controllers/finders_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require 'spec_helper'
require 'gds_api/test_helpers/content_store'
include GdsApi::TestHelpers::ContentStore
include FixturesHelper

describe FindersController do
describe "GET show" do
describe "finder item doesn't exist" do
it 'returns a 404, rather than 5xx' do
content_store_does_not_have_item('/does-not-exist')

get :show, slug: 'does-not-exist'
expect(response.status).to eq(404)
end
end
end
end