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 notes to history #85

Merged
merged 7 commits into from
Mar 3, 2014
Merged
Show file tree
Hide file tree
Changes from 4 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
18 changes: 18 additions & 0 deletions app/controllers/notes_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
require 'gds_api/need_api'
require 'plek'
require 'json'

class NotesController < ApplicationController
def create
text = params["notes"]["text"]
need_id = params["need_id"]
@note = Note.new(text, need_id, current_user)

if @note.save
flash[:notice] = "Note saved"
else
flash[:error] = "Error saving note"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit cryptic; now that the API returns specific errors, could these be displayed to the user?

end
redirect_to revisions_need_path(need_id)
end
end
32 changes: 32 additions & 0 deletions app/models/note.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
class Note

attr_reader :text, :need_id, :author

def initialize(text, need_id, author)
@text = text
@need_id = need_id
@author = author
end

def save
note_atts = {
"text" => text,
"need_id" => need_id,
"author" => author_atts(author)
}
Maslow.need_api.create_note(note_atts)
true
rescue GdsApi::HTTPErrorResponse => err
false
end

private

def author_atts(author)
{
"name" => author.name,
"email" => author.email,
"uid" => author.uid
}
end
end
2 changes: 1 addition & 1 deletion app/views/needs/_need_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"View" => need_path(@need),
"Edit" => edit_need_path(@need),
"Actions" => actions_need_path(@need),
"History" => revisions_need_path(@need)
"History & Notes" => revisions_need_path(@need)
}
tabs.delete("Edit") if @need.duplicate_of.present?
bootstrap_flavour_tabs(tabs, active: active) %>
12 changes: 12 additions & 0 deletions app/views/needs/_revision.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
<% revision.notes.each do |n| %>
<li class="revision">
<h3 class="revision-title">
<%= Time.parse(n.created_at).strftime("%-d %B %Y, %H:%M") %> &mdash;
Note by <%= n.author.name %>
<% if n.author.email.present? %>
&lt;<%= n.author.email %>&gt;
<% end %>
</h3>
<%= n.text %>
</li>
<% end %>

<li class="revision">
<h3 class="revision-title">
Expand Down
31 changes: 24 additions & 7 deletions app/views/needs/revisions.html.erb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<% content_for :page_title, "History" %>
<% content_for :page_title, "History & Notes" %>

<article class="need">
<nav class="add-top-margin">
<ul class="breadcrumb">
<li><%= link_to "All needs", needs_path %> <span class="divider">/</span></li>
<li><%= link_to "#{@need.need_id}: #{format_need_goal @need.goal}", need_path(@need) %> <span class="divider">/</span></li>
<li class="active">History</li>
<li class="active">History &amp; Notes</li>
</ul>
</nav>

Expand All @@ -15,13 +15,30 @@
need_path(@need.duplicate_of) %>
</p>
<% else %>
<%= render :partial => "need_header", locals: { active: "History" } %>
<%= render :partial => "need_header", locals: { active: "History & Notes" } %>
<% end %>

<h3>Recent changes</h3>
<ul class="revisions">
<%= render partial: "revision", collection: @need.revisions %>
</ul>
<div class="row-fluid">
<div id="notes" class="span6">
<%= semantic_form_for(:notes, url: notes_url) do |f| %>
<%= f.inputs do %>
<%= hidden_field_tag :need_id, @need.need_id %>
<%= f.input :text, :label => 'Note', :as => :text, :hint => "Don't forget to save your note when you have finished.", input_html: { class: "span12" } %>
<% end %>
<%= f.action :submit,
:button_html => {
:value => "Add note",
:class => "btn btn-primary"
} %>
<% end %>
</div>
<div id="revision-history" class="span6 accordion">
<h3>Recent changes</h3>
<ul class="revisions">
<%= render partial: "revision", collection: @need.revisions %>
</ul>
</div>
</div>

<%= render partial: "workflow_buttons", locals: { edit_page: false } %>
</article>
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

get :bookmarklet, controller: :bookmarklet, path: 'bookmarklet'

resources :notes, only: [:create]

resources :needs, except: [:destroy], constraints: { id: /[0-9]+/ } do
member do
get :revisions
Expand Down
39 changes: 36 additions & 3 deletions test/fixtures/needs/101350.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,37 @@
"goal": [ "apply for a secondary school place" ,"apply for a primary school place" ],
"role": [ null, "parent" ]
},
"created_at": "2013-05-01T13:00:00+00:00"
"created_at": "2013-05-01T13:00:00+00:00",
"notes": [
{
"text": "hello",
"author": {
"name": "Donald Duck",
"email": null,
"uid": null
},
"created_at": "2013-05-02T13:00:00+00:00"
}
]
},
{
"action_type": "update",
"author": null,
"changes": {
"legislation": [ "foo", "bar" ]
},
"created_at": "2013-04-01T13:00:00+00:00"
"created_at": "2013-04-01T13:00:00+00:00",
"notes": [
{
"text": "goodbye",
"author": {
"name": "Minnie Mouse",
"email": null,
"uid": null
},
"created_at": "2013-04-03T13:00:00+00:00"
}
]
},
{
"action_type": "create",
Expand All @@ -67,7 +89,18 @@
"goal": [ "apply for a school place", "apply for a secondary school place" ],
"role": [ "grandparent", null ]
},
"created_at": "2013-01-01T13:00:00+00:00"
"created_at": "2013-01-01T13:00:00+00:00",
"notes": [
{
"text": "oops",
"author": {
"name": "Goofy",
"email": null,
"uid": null
},
"created_at": "2013-01-02T13:00:00+00:00"
}
]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm really not a fan of using fixtures for test data; this ought to be created dynamically by builder code. But that's a bigger change and not part of this PR.

}
]
}
45 changes: 45 additions & 0 deletions test/functional/notes_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require_relative '../integration_test_helper'
require 'gds_api/test_helpers/need_api'

class NotesControllerTest < ActionController::TestCase
include GdsApi::TestHelpers::NeedApi

setup do
login_as stub_user
@note_atts = {
"notes" => {
"text" => "test"
},
"need_id" => "100001"
}
end

context "POST create" do
should "be successful" do
GdsApi::NeedApi.any_instance.expects(:create_note).with(
"text" => "test",
"need_id" => "100001",
"author" => {
"name" => stub_user.name,
"email" => stub_user.email,
"uid" => stub_user.uid
}
)

post :create, @note_atts

assert_redirected_to revisions_need_path("100001")
assert_equal "Note saved", @controller.flash[:notice]
refute @controller.flash[:error]
end

should "return an error message if the save fails" do
Note.any_instance.expects(:save).returns(false)

post :create, @note_atts

assert_equal "Error saving note", @controller.flash[:error]
refute @controller.flash[:notice]
end
end
end
30 changes: 24 additions & 6 deletions test/integration/view_a_need_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ViewANeedTest < ActionDispatch::IntegrationTest

within ".nav-tabs" do
assert page.has_link?("Edit", href: "/needs/101350/edit")
assert page.has_link?("History", href: "/needs/101350/revisions")
assert page.has_link?("History & Notes", href: "/needs/101350/revisions")
end

within ".the-need" do
Expand Down Expand Up @@ -92,12 +92,12 @@ class ViewANeedTest < ActionDispatch::IntegrationTest
visit "/needs"

click_on "101350"
click_on "History"
click_on "History & Notes"

within ".breadcrumb" do
assert page.has_link?("All needs", href: "/needs")
assert page.has_link?("101350: Book a driving test", href: "/needs/101350")
assert page.has_content?("History")
assert page.has_content?("History & Notes")
end

within ".need" do
Expand All @@ -119,9 +119,15 @@ class ViewANeedTest < ActionDispatch::IntegrationTest
end

within ".revisions" do
assert_equal 3, page.all(".revision").count
assert_equal 6, page.all(".revision").count

within ".revision:nth-child(1)" do
assert page.has_content?("Note by Donald Duck")
assert page.has_content?("2 May 2013, 13:00")
assert page.has_content?("hello")
end

within ".revision:nth-child(2)" do
assert page.has_content?("Update by Mickey Mouse <mickey.mouse@test.com>")
assert page.has_content?("1 May 2013, 13:00")

Expand All @@ -133,13 +139,25 @@ class ViewANeedTest < ActionDispatch::IntegrationTest
end
end

within ".revision:nth-child(2)" do
within ".revision:nth-child(3)" do
assert page.has_content?("Note by Minnie Mouse")
assert page.has_content?("3 April 2013, 13:00")
assert page.has_content?("goodbye")
end

within ".revision:nth-child(4)" do
assert page.has_content?("Update by unknown author")
assert page.has_no_content?("<>") # catch missing email
assert page.has_content?("1 April 2013, 13:00")
end

within ".revision:nth-child(3)" do
within ".revision:nth-child(5)" do
assert page.has_content?("Note by Goofy")
assert page.has_content?("2 January 2013, 13:00")
assert page.has_content?("oops")
end

within ".revision:nth-child(6)" do
assert page.has_content?("Create by Donald Duck")
assert page.has_no_content?("<>") # catch an empty email string
assert page.has_content?("1 January 2013, 13:00")
Expand Down
23 changes: 23 additions & 0 deletions test/unit/note_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
require_relative '../test_helper'

class NoteTest < ActiveSupport::TestCase
should "send note data to the need api" do
@author = OpenStruct.new(
name: "Winston Smith-Churchill",
email: "winston@alphagov.co.uk",
uid: "win5t0n"
)

GdsApi::NeedApi.any_instance.expects(:create_note).with(
"text" => "test",
"need_id" => "100001",
"author" => {
"name" => "Winston Smith-Churchill",
"email" => "winston@alphagov.co.uk",
"uid" => "win5t0n"
}
)

Note.new("test", "100001", @author).save
end
end