Skip to content
This repository has been archived by the owner on Feb 13, 2018. It is now read-only.

Add notes to history #54

Merged
merged 10 commits into from Feb 28, 2014
16 changes: 16 additions & 0 deletions app/controllers/notes_controller.rb
@@ -0,0 +1,16 @@
class NotesController < ApplicationController
def create
note = Note.new(filtered_params)
if note.save
head :created
else
error 422, message: :invalid_attributes, errors: note.errors.full_messages
end
end

private

def filtered_params
params.except(:action, :controller)
end
end
3 changes: 2 additions & 1 deletion app/decorators/need_with_changesets.rb
Expand Up @@ -6,7 +6,8 @@ class NeedWithChangesets < SimpleDelegator

def changesets
(need.revisions + [nil]).each_cons(2).map {|revision, previous|
Changeset.new(revision, previous)
notes = Note.where(revision: revision.id)
Changeset.new(revision, previous, notes)
}
end

Expand Down
4 changes: 3 additions & 1 deletion app/models/changeset.rb
Expand Up @@ -2,10 +2,12 @@
class Changeset

attr_reader :current, :previous
attr_reader :notes

def initialize(current, previous)
def initialize(current, previous, notes=[])
@current = current
@previous = previous
@notes = notes
end

# This method returns the changes between the current revision and its
Expand Down
29 changes: 29 additions & 0 deletions app/models/note.rb
@@ -0,0 +1,29 @@
class Note
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this class should have some validations

include Mongoid::Document
include Mongoid::Timestamps

field :text, type: String
field :need_id, type: Integer
field :author, type: Hash
field :revision, type: String

default_scope order_by([:created_at, :desc])

validates_presence_of :text, :need_id, :author
validate :validate_need_id

def save
need = Need.where(need_id: need_id).first
self.revision = need.revisions.first.id if need
super
end

private

def validate_need_id
need = Need.where(need_id: need_id).first
if need.nil?
errors.add(:need_id, "A note must have a valid need_id")
end
end
end
9 changes: 8 additions & 1 deletion app/presenters/changeset_presenter.rb
Expand Up @@ -8,7 +8,14 @@ def present
action_type: @changeset.current.action_type,
author: @changeset.current.author,
changes: @changeset.changes,
created_at: @changeset.current.created_at
created_at: @changeset.current.created_at,
notes: @changeset.notes.map { |note|
{
text: note.text,
author: note.author,
created_at: note.created_at
}
}
}
end
end
2 changes: 2 additions & 0 deletions config/routes.rb
@@ -1,6 +1,8 @@
GovukNeedApi::Application.routes.draw do
resources :organisations, :only => :index
resources :needs, :except => [:new, :edit]
resources :notes, :only => [:create]

put '/needs/:id/closed', to: 'needs#closed'
delete '/needs/:id/closed', to: 'needs#reopen'

Expand Down
39 changes: 39 additions & 0 deletions test/functional/notes_controller_test.rb
@@ -0,0 +1,39 @@
require_relative '../test_helper'

class NotesControllerTest < ActionController::TestCase

setup do
login_as_stub_user
@need = FactoryGirl.create(:need)
@note = {
text: "test",
need_id: @need.need_id,
author: {
name: stub_user.name,
email: stub_user.email
}
}
end

context "POST create" do
should "save the note" do
post :create, @note

note = Note.first

assert_equal 201, response.status
assert_equal "test", note.text
assert_equal @need.need_id, note.need_id
assert_equal stub_user.name, note.author["name"]
assert_equal stub_user.email, note.author["email"]
end

should "show errors if a note is not valid" do
post :create, @note.except(:text)

body = JSON.parse(response.body)
assert_equal 422, response.status
assert_equal "invalid_attributes", body["_response_info"]["status"]
end
end
end
16 changes: 16 additions & 0 deletions test/unit/decorators/changeset_test.rb
Expand Up @@ -22,6 +22,22 @@ class ChangesetTest < ActiveSupport::TestCase
assert_equal Time.parse("2013-10-01 10:00:00"), change.current.created_at
end

should "attach notes to a revision" do
notes = [
{
text: "test",
author: {
name: "Winston Smith-Churchill"
},
created_at: Time.parse("2013-10-02 10:00:00")
}
]
change = Changeset.new(@revision, nil, notes)
assert_equal "test", change.notes[0][:text]
assert_equal Time.parse("2013-10-02 10:00:00"), change.notes[0][:created_at]
assert_equal "Winston Smith-Churchill", change.notes[0][:author][:name]
end

context "calculating the changes with a previous revision" do
should "include changes in values for existing keys" do
previous_revision = OpenStruct.new(
Expand Down
9 changes: 9 additions & 0 deletions test/unit/decorators/need_with_changesets_test.rb
Expand Up @@ -38,4 +38,13 @@ class NeedWithChangesetsTest < ActiveSupport::TestCase
]
}
end

should "fetch notes" do
@revisions.map { |revision|
{ revision: revision.id }
}.each do |search_term|
Note.expects(:where).with(search_term)
end
@decorator.changesets
end
end
56 changes: 56 additions & 0 deletions test/unit/note_test.rb
@@ -0,0 +1,56 @@
require_relative '../test_helper'

class NoteTest < ActiveSupport::TestCase
setup do
@need = FactoryGirl.create(:need)
@atts = {
text: "test",
need_id: @need.need_id,
author: {
name: "Winston Smith-Churchill",
email: "winston@alphagov.co.uk",
uid: "win5t0n"
}
}
end

should "be able to create a note" do
Need.expects(:where).twice.returns([@need])

note = Note.new(@atts)
note.save
note.reload

assert_equal "test", note.text
assert_equal "Winston Smith-Churchill", note.author["name"]
assert_equal @need.revisions.first.id.to_s, note.revision
end

should "not save a note without a need_id" do
note = Note.new(@atts.except(:need_id))

refute note.save
assert note.errors.has_key?(:need_id)
end

should "not save a note without a valid need_id" do
note = Note.new(@atts.merge(need_id: :foo))

refute note.save
assert note.errors.has_key?(:need_id)
end

should "not save a note without text" do
note = Note.new(@atts.except(:text))

refute note.save
assert note.errors.has_key?(:text)
end

should "not save a note without an author" do
note = Note.new(@atts.except(:author))

refute note.save
assert note.errors.has_key?(:author)
end
end
24 changes: 23 additions & 1 deletion test/unit/presenters/changeset_presenter_test.rb
Expand Up @@ -16,9 +16,20 @@ class ChangesetPresenterTest < ActiveSupport::TestCase
created_at: Time.parse("2013-01-01")
)

note = OpenStruct.new(
text: "test",
author: {
name: "Sir John Anderson",
email: "jock@alphagov.co.uk",
uid: "j0hn"
},
created_at: Time.parse("2013-01-02")
)

@changeset = OpenStruct.new(
current: current,
changes: { role: [ "user", "home owner" ] }
changes: { role: [ "user", "home owner" ] },
notes: [note]
)
end

Expand All @@ -35,4 +46,15 @@ class ChangesetPresenterTest < ActiveSupport::TestCase

assert_equal ["user", "home owner"], response[:changes][:role]
end

should "return notes as part of the changset" do
response = ChangesetPresenter.new(@changeset).present

note = response[:notes][0]
assert_equal "test", note[:text]
assert_equal "Sir John Anderson", note[:author][:name]
assert_equal "jock@alphagov.co.uk", note[:author][:email]
assert_equal "j0hn", note[:author][:uid]
assert_equal Time.parse("2013-01-02"), note[:created_at]
Copy link
Contributor

Choose a reason for hiding this comment

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

can you split these note assertions into a separate should block ? i feel notes cannot be considered as basic attributes of a need.

end
end