From 883ea2b96860fa5b4fa6f8250e191fe5479e8bba Mon Sep 17 00:00:00 2001 From: Matt Rogers Date: Thu, 2 May 2013 22:29:21 -0500 Subject: [PATCH] Move Project#create_from_todo to its own class The point of this is to keep as many things out of the ActiveRecord objects as possible and use them as just a thin database abstraction layer. --- app/controllers/todos_controller.rb | 2 +- app/models/project.rb | 15 --------------- lib/project_from_todo.rb | 27 +++++++++++++++++++++++++++ test/unit/project_from_todo_test.rb | 22 ++++++++++++++++++++++ test/unit/project_test.rb | 8 -------- 5 files changed, 50 insertions(+), 24 deletions(-) create mode 100644 lib/project_from_todo.rb create mode 100644 test/unit/project_from_todo_test.rb diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 7b3aa58e..17dc4aa0 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -776,7 +776,7 @@ class TodosController < ApplicationController def convert_to_project todo = current_user.todos.find(params[:id]) - @project = Project.create_from_todo(todo) + @project = ProjectFromTodo.new(todo).create if @project.valid? redirect_to project_url(@project) diff --git a/app/models/project.rb b/app/models/project.rb index 5428a7fe..a0e1bb77 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -47,21 +47,6 @@ class Project < ActiveRecord::Base NullProject.new end - def self.create_from_todo(todo) - project = Project.new(:name => todo.description, - :description => todo.notes, - :default_context => todo.context) - - project.user = todo.user - - if project.valid? - todo.destroy - project.save! - end - - project - end - def hide_todos todos.each do |t| unless t.completed? || t.deferred? diff --git a/lib/project_from_todo.rb b/lib/project_from_todo.rb new file mode 100644 index 00000000..db91030b --- /dev/null +++ b/lib/project_from_todo.rb @@ -0,0 +1,27 @@ +class ProjectFromTodo + attr_reader :todo + + def initialize(todo) + @todo = todo + end + + def create + project = build_project + + if project.valid? + todo.destroy + project.save! + end + + project + end + + def build_project + project = Project.new.tap do |p| + p.name = todo.description + p.description = todo.notes + p.default_context = todo.context + p.user = todo.user + end + end +end diff --git a/test/unit/project_from_todo_test.rb b/test/unit/project_from_todo_test.rb new file mode 100644 index 00000000..755df908 --- /dev/null +++ b/test/unit/project_from_todo_test.rb @@ -0,0 +1,22 @@ +require_relative '../test_helper' +require_relative '../../lib/project_from_todo' + +class ProjectFromTodoTest < ActiveSupport::TestCase + fixtures :todos + + def test_create_project_from_valid_todo + todo = todos(:upgrade_rails) + project = ProjectFromTodo.new(todo).create + assert_equal project.name, todo.description + assert_equal project.description, todo.notes + assert_equal project.default_context, todo.context + end + + def test_invalid_project_from_invalid_todo + todo = todos(:upgrade_rails) + todo.description = "" + project = ProjectFromTodo.new(todo).create + assert_not_nil project + assert_equal false, project.valid? + end +end diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb index e4738921..ded79039 100644 --- a/test/unit/project_test.rb +++ b/test/unit/project_test.rb @@ -167,14 +167,6 @@ class ProjectTest < ActiveSupport::TestCase assert_equal 3, @moremoney.todos.not_completed.count end - def test_convert_from_todo - todo = todos(:upgrade_rails) - project = Project.create_from_todo(todo) - assert_equal project.name, todo.description - assert_equal project.description, todo.notes - assert_equal project.default_context, todo.context - end - def test_new_record_before_save assert !@timemachine.new_record_before_save?, "existing records should not be new_record" p = Project.where(:name => "I do not exist").first_or_create