From d0f4bf3c59c9f25c43b976523faa9a11791ffb26 Mon Sep 17 00:00:00 2001 From: Moises Machado Date: Wed, 15 Oct 2008 20:17:32 -0300 Subject: [PATCH] ActionItem784: redirect old urls to new articles urls --- app/controllers/public/content_viewer_controller.rb | 8 ++++++++ app/models/article.rb | 4 ++++ test/functional/content_viewer_controller_test.rb | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ test/unit/article_test.rb | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 0 deletions(-) diff --git a/app/controllers/public/content_viewer_controller.rb b/app/controllers/public/content_viewer_controller.rb index 2dc5ff2..49cf3f9 100644 --- a/app/controllers/public/content_viewer_controller.rb +++ b/app/controllers/public/content_viewer_controller.rb @@ -15,6 +15,14 @@ class ContentViewerController < ApplicationController end else @page = profile.articles.find_by_path(path) + unless @page + page_from_old_path = profile.articles.find_by_old_path(path) + if page_from_old_path + flash[:notice] = _("Redirected from \"%s\". please update your links and bookmarks.") % request.url + redirect_to :profile => profile.identifier, :page => page_from_old_path.explode_path + return + end + end # do not show unpublished articles if @page && !@page.published diff --git a/app/models/article.rb b/app/models/article.rb index 14228bf..38da5d0 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -193,6 +193,10 @@ class Article < ActiveRecord::Base ['id', 'profile_id', 'parent_id', 'slug', 'path', 'updated_at', 'created_at', 'last_changed_by_id', 'version', 'lock_version', 'type', 'children_count', 'comments_count'] end + def self.find_by_old_path(old_path) + find(:first, :include => :versions, :conditions => ['article_versions.path = ?', old_path], :order => 'article_versions.id desc') + end + private def sanitize_tag_list diff --git a/test/functional/content_viewer_controller_test.rb b/test/functional/content_viewer_controller_test.rb index 2f3a2a8..6b54e41 100644 --- a/test/functional/content_viewer_controller_test.rb +++ b/test/functional/content_viewer_controller_test.rb @@ -461,4 +461,60 @@ class ContentViewerControllerTest < Test::Unit::TestCase assert_response 403 end + should 'redirect to new article path under an old path' do + p = create_user('test_user').person + a = p.articles.create(:name => 'old-name') + old_path = a.explode_path + a.name = 'new-name' + a.save! + + get :view_page, :profile => p.identifier, :page => old_path + + assert_response :redirect + assert_redirected_to :profile => p.identifier, :page => a.explode_path + end + + should 'load new article name equal of another article old name' do + p = create_user('test_user').person + a1 = p.articles.create!(:name => 'old-name') + old_path = a1.explode_path + a1.name = 'new-name' + a1.save! + a2 = p.articles.create!(:name => 'old-name') + + get :view_page, :profile => p.identifier, :page => old_path + + assert_equal a2, assigns(:page) + end + + should 'redirect to article with most recent version with the name if there is no article with the name' do + p = create_user('test_user').person + a1 = p.articles.create!(:name => 'old-name') + old_path = a1.explode_path + a1.name = 'new-name' + a1.save! + a2 = p.articles.create!(:name => 'old-name') + a2.name = 'other-new-name' + a2.save! + + get :view_page, :profile => p.identifier, :page => old_path + + assert_response :redirect + assert_redirected_to :profile => p.identifier, :page => a2.explode_path + end + + should 'not return an article of a different user' do + p1 = create_user('test_user').person + a = p1.articles.create!(:name => 'old-name') + old_path = a.explode_path + a.name = 'new-name' + a.save! + + p2 = create_user('another_user').person + + get :view_page, :profile => p2.identifier, :page => old_path + + assert_response :missing + end + end diff --git a/test/unit/article_test.rb b/test/unit/article_test.rb index f9429b3..8bef813 100644 --- a/test/unit/article_test.rb +++ b/test/unit/article_test.rb @@ -490,4 +490,58 @@ class ArticleTest < Test::Unit::TestCase assert_kind_of Folder, b end + should 'load article under an old path' do + p = create_user('test_user').person + a = p.articles.create(:name => 'old-name') + old_path = a.explode_path + a.name = 'new-name' + a.save! + + page = Article.find_by_old_path(old_path) + + assert_equal a.path, page.path + end + + should 'load new article name equal of another article old name' do + p = create_user('test_user').person + a1 = p.articles.create!(:name => 'old-name') + old_path = a1.explode_path + a1.name = 'new-name' + a1.save! + a2 = p.articles.create!(:name => 'old-name') + + page = Article.find_by_old_path(old_path) + + assert_equal a2.path, page.path + end + + should 'article with most recent version with the name must be loaded if no aritcle with the name' do + p = create_user('test_user').person + a1 = p.articles.create!(:name => 'old-name') + old_path = a1.explode_path + a1.name = 'new-name' + a1.save! + a2 = p.articles.create!(:name => 'old-name') + a2.name = 'other-new-name' + a2.save! + + page = Article.find_by_old_path(old_path) + + assert_equal a2.path, page.path + end + + should 'not return an article of a different user' do + p1 = create_user('test_user').person + a = p1.articles.create!(:name => 'old-name') + old_path = a.explode_path + a.name = 'new-name' + a.save! + + p2 = create_user('another_user').person + + page = p2.articles.find_by_old_path(old_path) + + assert_nil page + end + end -- libgit2 0.21.2