Merge pull request #12 from PFischbeck/problem-model-refactor

Problem model refactor

app/assets/javascripts/translations.js.coffee

@@ -0,0 +1,3 @@
+# Place all the behaviors and hooks related to the matching controller here.
+# All this logic will automatically be available in application.js.
+# You can use CoffeeScript in this file: http://coffeescript.org/

app/assets/stylesheets/problems.css.scss

@@ -1,3 +1,3 @@
-// Place all the styles related to the Problems controller here.
-// They will automatically be included in application.css.scss.
+// Place all the styles related to the problems controller here.
+// They will automatically be included in application.css.
 // You can use Sass (SCSS) here: http://sass-lang.com/

app/assets/stylesheets/translations.css.scss

@@ -0,0 +1,3 @@
+// Place all the styles related to the Translations controller here.
+// They will automatically be included in application.css.scss.
+// You can use Sass (SCSS) here: http://sass-lang.com/

app/controllers/problems_controller.rb

@@ -1,63 +1,13 @@
 class ProblemsController < ApplicationController
-  before_action :set_problem, only: [:show, :edit, :update, :destroy]
+  before_action :set_problem, only: [:show]
 
-  # GET /problems
-  # GET /problems.json
   def index
-    @problems = Problem.paginate(page: params[:page])
+    @problems = Problem.paginate(page: params[:page]).includes(:translation)
   end
 
-  # GET /problems/1
-  # GET /problems/1.json
   def show
-  end
-
-  # GET /problems/new
-  def new
-    @problem = Problem.new
-  end
-
-  # GET /problems/1/edit
-  def edit
-  end
-
-  # POST /problems
-  # POST /problems.json
-  def create
-    @problem = Problem.new(problem_params)
-
-    respond_to do |format|
-      if @problem.save
-        format.html { redirect_to @problem, notice: 'Problem was successfully created.' }
-        format.json { render :show, status: :created, location: @problem }
-      else
-        format.html { render :new }
-        format.json { render json: @problem.errors, status: :unprocessable_entity }
-      end
-    end
-  end
-
-  # PATCH/PUT /problems/1
-  # PATCH/PUT /problems/1.json
-  def update
-    respond_to do |format|
-      if @problem.update(problem_params)
-        format.html { redirect_to @problem, notice: 'Problem was successfully updated.' }
-        format.json { render :show, status: :ok, location: @problem }
-      else
-        format.html { render :edit }
-        format.json { render json: @problem.errors, status: :unprocessable_entity }
-      end
-    end
-  end
-
-  # DELETE /problems/1
-  # DELETE /problems/1.json
-  def destroy
-    @problem.destroy
-    respond_to do |format|
-      format.html { redirect_to problems_url, notice: 'Problem was successfully destroyed.' }
-      format.json { head :no_content }
+    unless @problem.is_translated?
+      render action: "untranslated"
     end
   end
 
@@ -69,6 +19,6 @@ class ProblemsController < ApplicationController
 
   # Never trust parameters from the scary internet, only allow the white list through.
   def problem_params
-      params.require(:problem).permit(:title, :content)
+    params.require(:problem)
   end
 end

app/controllers/translations_controller.rb

@@ -0,0 +1,51 @@
+class TranslationsController < ApplicationController
+  before_action :set_translation, only: :show
+  before_action :set_problem, only: [:new, :create]
+
+  # GET /translations
+  # GET /translations.json
+  def index
+    @translations = Translation.paginate(page: params[:page])
+  end
+
+  # GET /translations/1
+  # GET /translations/1.json
+  def show
+  end
+
+  # GET /translations/new
+  def new
+    @translation = @problem.translations.build
+  end
+
+  # POST /translations
+  # POST /translations.json
+  def create
+    @translation = @problem.translations.new(translation_params)
+
+    respond_to do |format|
+      if @translation.save
+        format.html { redirect_to @translation, notice: 'Translation was successfully created.' }
+        format.json { render :show, status: :created, location: @translation }
+      else
+        format.html { render :new }
+        format.json { render json: @translation.errors, status: :unprocessable_entity }
+      end
+    end
+  end
+
+  private
+    # Use callbacks to share common setup or constraints between actions.
+    def set_translation
+      @translation = Translation.find(params[:id])
+    end
+
+    # Never trust parameters from the scary internet, only allow the white list through.
+    def translation_params
+      params.require(:translation).permit(:title, :content)
+    end
+
+    def set_problem
+      @problem = Problem.find(params[:problem_id])
+    end
+end

app/helpers/translations_helper.rb

@@ -0,0 +1,2 @@
+module TranslationsHelper
+end

app/models/problem.rb

@@ -1,10 +1,18 @@
 class Problem < ActiveRecord::Base
-  validates :title, :content, presence: true
-  validates :title, uniqueness: true
+  # TODO Check whether there is a better relation for this
+  belongs_to :translation
+  delegate :title, :content, to: :translation
+
+  has_many :translations, inverse_of: :problem
   
   self.per_page = 50
 
+  def is_translated?
+    !!self.translation
+  end
+
+
   def original_url
-    "https://projecteuler.net/problem=#{id}"
+    "https://projecteuler.net/problem=#{self.id}"
   end
 end

app/models/translation.rb

@@ -0,0 +1,8 @@
+class Translation < ActiveRecord::Base
+  belongs_to :problem, inverse_of: :translations
+
+  validates :title, :content, :problem_id, presence: true
+  validates :title, uniqueness: true
+
+  self.per_page = 50
+end

app/views/layouts/application.html.erb

@@ -13,7 +13,8 @@
     <% end %>
     <%= horizontal do %>
         <%= nav class: 'navbar-left' do %>
-            <%= link_to 'Probleme', problems_path %>
+            <%= link_to 'Problems', problems_path %>
+            <%= link_to 'Translations', translations_path %>
         <% end %>
     <% end %>
 <% end %>

app/views/problems/edit.html.erb

@@ -1,6 +0,0 @@
-<h1>Editing problem</h1>
-
-<%= render 'form' %>
-
-<%= link_to 'Show', @problem %> |
-<%= link_to 'Back', problems_path %>

app/views/problems/index.html.erb

@@ -1,4 +1,4 @@
-<h1>Listing problems</h1>
+<h1>Listing Problems</h1>
 
 <%= render 'problem_pagination' %>
 <table class="table table-striped table-bordered">
@@ -13,12 +13,20 @@
   <% @problems.each do |problem| %>
       <tr>
         <td><%= problem.id %></td>
-        <td><%= link_to problem.title, problem %></td>
+        <td>
+          <% if problem.is_translated? %>
+            <%= link_to problem.title, problem %>
+          <% else %>
+              <i>Dieses Problem wurde noch nicht übersetzt.</i>
+              <%= link_to new_problem_translation_path(problem), class: 'btn btn-default btn-xs' do %>
+                  <%= icon :pencil %> Übersetzung vorschlagen
+              <% end %>
+          <% end %>
+
+        </td>
       </tr>
   <% end %>
   </tbody>
 </table>
 <%= render 'problem_pagination' %>
 <br>
-
-<%= link_to 'New Problem', new_problem_path, class: 'btn btn-default' %>

app/views/problems/index.json.jbuilder

@@ -1,4 +0,0 @@
-json.array!(@problems) do |problem|
-  json.extract! problem, :id, :title, :content
-  json.url problem_url(problem, format: :json)
-end

app/views/problems/new.html.erb

@@ -1,5 +0,0 @@
-<h1>New problem</h1>
-
-<%= render 'form' %>
-
-<%= link_to 'Back', problems_path %>

app/views/problems/show.html.erb

@@ -5,7 +5,9 @@
 <div class="page-header">
   <h1><%= @problem.title %> <small>Problem <%= @problem.id %></small></h1>
 </div>
-
+<%= link_to new_problem_translation_path(@problem), class: 'btn btn-default btn-sm pull-right' do %>
+    <%= icon :pencil %> Übersetzung verbessern
+<% end %>
 <%= panel do %>
     <div class="panel-body problem-content">
       <%= sanitize @problem.content %>
@@ -14,4 +16,3 @@
 <div class="text-center">
   <%= link_to 'Dieses Problem auf projecteuler.net', @problem.original_url, target: '_blank' %>
 </div>
-<%= link_to 'Edit', edit_problem_path(@problem), class: 'btn btn-default' %>

app/views/problems/show.json.jbuilder

@@ -1 +0,0 @@
-json.extract! @problem, :id, :title, :content, :created_at, :updated_at

app/views/problems/untranslated.html.erb

@@ -0,0 +1 @@
+Dieses Problem wurde noch nicht übersetzt.

app/views/translations/_form.html.erb

@@ -1,10 +1,10 @@
-<%= form_for(@problem) do |f| %>
-  <% if @problem.errors.any? %>
+<%= form_for([@problem, @translation]) do |f| %>
+  <% if @translation.errors.any? %>
     <div id="error_explanation">
-      <h2><%= pluralize(@problem.errors.count, "error") %> prohibited this problem from being saved:</h2>
+      <h2><%= pluralize(@translation.errors.count, "error") %> prohibited this problem from being saved:</h2>
 
       <ul>
-      <% @problem.errors.full_messages.each do |message| %>
+      <% @translation.errors.full_messages.each do |message| %>
         <li><%= message %></li>
       <% end %>
       </ul>

app/views/translations/_translation_pagination.html.erb

@@ -0,0 +1 @@
+<%= will_paginate @translations, renderer: BootstrapPagination::Rails %>

app/views/translations/edit.html.erb

@@ -0,0 +1,6 @@
+<h1>Editing translation</h1>
+
+<%= render 'form' %>
+
+<%= link_to 'Show', @translation %> |
+<%= link_to 'Back', translations_path %>

app/views/translations/index.html.erb

@@ -0,0 +1,22 @@
+<h1>Listing translations</h1>
+
+<%= render 'translation_pagination' %>
+<table class="table table-striped table-bordered">
+  <thead>
+    <tr>
+      <th>ID</th>
+      <th>Title</th>
+    </tr>
+  </thead>
+
+  <tbody>
+    <% @translations.each do |translation| %>
+      <tr>
+        <td><%= translation.id %></td>
+        <td><%= link_to translation.title, translation %></td>
+      </tr>
+    <% end %>
+  </tbody>
+</table>
+<%= render 'translation_pagination' %>
+<br>

app/views/translations/index.json.jbuilder

@@ -0,0 +1,4 @@
+json.array!(@translations) do |translation|
+  json.extract! translation, :id, :title, :content
+  json.url translation_url(translation, format: :json)
+end

app/views/translations/new.html.erb

@@ -0,0 +1,5 @@
+<h1>New translation for problem <%= @problem.id %></h1>
+
+<%= render 'form' %>
+
+<%= link_to 'Back', translations_path %>

app/views/translations/show.html.erb

@@ -0,0 +1,16 @@
+<% if notice %>
+    <p id="notice"><%= notice %></p>
+<% end %>
+
+<div class="page-header">
+  <h1><%= @translation.title %> <small>Problem <%= @translation.problem_id %></small></h1>
+</div>
+
+<%= panel do %>
+    <div class="panel-body problem-content">
+        <%= sanitize @translation.content %>
+    </div>
+<% end %>
+<div class="text-center">
+  <%= link_to 'Dieses Problem auf projecteuler.net', @translation.problem.original_url, target: '_blank' %>
+</div>

app/views/translations/show.json.jbuilder

@@ -0,0 +1 @@
+json.extract! @translation, :id, :title, :content, :created_at, :updated_at

config/routes.rb

@@ -1,5 +1,8 @@
 Rails.application.routes.draw do
-  resources :problems
+  resources :problems, only: [:index, :show] do
+    resources :translations, only: [:new, :create]
+  end
+  resources :translations, only: [:index, :show]
 
   # The priority is based upon order of creation: first created -> highest priority.
   # See how all your routes lay out with "rake routes".

db/migrate/20141213151812_rename_problems_table_to_translations.rb

@@ -0,0 +1,5 @@
+class RenameProblemsTableToTranslations < ActiveRecord::Migration
+  def change
+    rename_table :problems, :translations
+  end
+end

db/migrate/20141214214958_create_problems_again.rb

@@ -0,0 +1,7 @@
+class CreateProblemsAgain < ActiveRecord::Migration
+  def change
+    create_table :problems do |t|
+      t.timestamps
+    end
+  end
+end

db/migrate/20141214220056_add_translation_to_problem.rb

@@ -0,0 +1,5 @@
+class AddTranslationToProblem < ActiveRecord::Migration
+  def change
+    add_reference :problems, :translation, index: true
+  end
+end

db/migrate/20141214221259_add_problem_to_translation.rb

@@ -0,0 +1,5 @@
+class AddProblemToTranslation < ActiveRecord::Migration
+  def change
+    add_reference :translations, :problem, index: true
+  end
+end

db/schema.rb

@@ -11,13 +11,24 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20141130153941) do
+ActiveRecord::Schema.define(version: 20141214221259) do
 
   create_table "problems", force: true do |t|
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.integer  "translation_id"
+  end
+
+  add_index "problems", ["translation_id"], name: "index_problems_on_translation_id"
+
+  create_table "translations", force: true do |t|
     t.string   "title"
     t.text     "content"
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer  "problem_id"
   end
 
+  add_index "translations", ["problem_id"], name: "index_translations_on_problem_id"
+
 end

db/seeds.rb

@@ -6,9 +6,16 @@
 #   cities = City.create([{ name: 'Chicago' }, { name: 'Copenhagen' }])
 #   Mayor.create(name: 'Emanuel', city: cities.first)
 
-for i in 1..103 do
-  Problem.create(
+Translation.delete_all
+Problem.delete_all
+
+for i in 1..10 do
+  translation = Translation.create(
+             problem_id: i,
              title: "Problem Nummer #{i}",
              content: "Das hier ist der Inhalt von <b>Problem #{i}</b>.<br />Hier ist die zweite Zeile."
   )
+  Problem.create(id: i, translation_id: translation.id)
 end
+
+Problem.create(id: 11)

test/controllers/problems_controller_test.rb

@@ -1,53 +1,19 @@
 require 'test_helper'
 
 class ProblemsControllerTest < ActionController::TestCase
-  setup do
-    @problem = problems(:one)
-    @update = {
-        title: 'New title',
-        content: 'This is the new content'
-    }
-  end
-
   test "should get index" do
     get :index
     assert_response :success
-    assert_not_nil assigns(:problems)
   end
 
-  test "should get new" do
-    get :new
+  test "should get show" do
+    get :show, id: 1
     assert_response :success
   end
 
-  test "should create problem" do
-    assert_difference('Problem.count') do
-      post :create, problem: @update
-    end
-
-    assert_redirected_to problem_path(assigns(:problem))
-  end
-
-  test "should show problem" do
-    get :show, id: @problem
+  test "should get untranslated problem" do
+    get :show, id: 3
     assert_response :success
   end
 
-  test "should get edit" do
-    get :edit, id: @problem
-    assert_response :success
-  end
-
-  test "should update problem" do
-    patch :update, id: @problem, problem: @update
-    assert_redirected_to problem_path(assigns(:problem))
-  end
-
-  test "should destroy problem" do
-    assert_difference('Problem.count', -1) do
-      delete :destroy, id: @problem
-    end
-
-    assert_redirected_to problems_path
-  end
 end

test/controllers/translations_controller_test.rb

@@ -0,0 +1,35 @@
+require 'test_helper'
+
+class TranslationsControllerTest < ActionController::TestCase
+  setup do
+    @translation = translations(:translation_one)
+    @update = {
+        title: 'New title',
+        content: 'This is the new content',
+    }
+  end
+
+  test "should get index" do
+    get :index
+    assert_response :success
+    assert_not_nil assigns(:translations)
+  end
+
+  test "should get new" do
+    get :new, problem_id: 1
+    assert_response :success
+  end
+
+  test "should create translation" do
+    assert_difference('Translation.count') do
+      post :create, problem_id: 1, translation: @update
+    end
+
+    assert_redirected_to translation_path(assigns(:translation))
+  end
+
+  test "should show translation" do
+    get :show, id: @translation
+    assert_response :success
+  end
+end

test/fixtures/problems.yml

@@ -1,9 +1,13 @@
 # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
 
 one:
-  title: First title
-  content: The content of the problem
+  id: 1
+  translation: translation_one
 
 two:
-  title: Second title
-  content: The content of the second problem
+  id: 2
+  translation: translation_two
+
+three:
+  id: 3
+

test/fixtures/translations.yml

@@ -0,0 +1,11 @@
+# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
+
+translation_one:
+  problem_id: 1
+  title: First title
+  content: The content of the translation
+
+translation_two:
+  problem_id: 2
+  title: Second title
+  content: The content of the second translation

test/helpers/translations_helper_test.rb

@@ -0,0 +1,4 @@
+require 'test_helper'
+
+class TranslationsHelperTest < ActionView::TestCase
+end

test/models/problem_test.rb

@@ -1,34 +1,20 @@
 require 'test_helper'
 
 class ProblemTest < ActiveSupport::TestCase
-  test "should not save problem without title" do
-    problem = Problem.new(content: 'This is some content')
-    assert_not problem.save
-  end
-
-  test "should not save problem without content" do
-    problem = Problem.new(title: 'Problem title')
-    assert_not problem.save
-  end
-
-  test "should not save problem with duplicate title" do
-    problem = Problem.new(
-        title: problems(:one).title,
-        content: 'This is some content'
-    )
-    assert_not problem.save
-  end
-
   test "should save correct problem" do
-    problem = Problem.new(
-        title: 'A unique title',
-        content: 'Some content'
-    )
+    problem = Problem.new
     assert problem.save
   end
 
+  test "is_translated? should return false for missing translation" do
+    assert_not problems(:three).is_translated?
+  end
+
+  test "is_translated? should return true for existing translation" do
+    assert problems(:one).is_translated?
+  end
+
   test "should have correct original url" do
-    first = Problem.first
-    assert_equal "https://projecteuler.net/problem=#{first.id}", first.original_url
+    assert_equal "https://projecteuler.net/problem=1", problems(:one).original_url
   end
 end

test/models/translation_test.rb

@@ -0,0 +1,31 @@
+require 'test_helper'
+
+class TranslationTest < ActiveSupport::TestCase
+  test "should not save translation without title" do
+    translation = Translation.new(content: 'This is some content')
+    assert_not translation.save
+  end
+
+  test "should not save translation without content" do
+    translation = Translation.new(title: 'Translation title')
+    assert_not translation.save
+  end
+
+  test "should not save translation with duplicate title" do
+    translation = Translation.new(
+        title: translations(:translation_one).title,
+        content: 'This is some content',
+        problem_id: 3
+    )
+    assert_not translation.save
+  end
+
+  test "should save correct translation" do
+    translation = Translation.new(
+        title: 'A unique title',
+        content: 'Some content',
+        problem_id: 1
+    )
+    assert translation.save
+  end
+end