Commit d0f4bf3c59c9f25c43b976523faa9a11791ffb26
1 parent
b9c58d94
Exists in
master
and in
23 other branches
ActionItem784: redirect old urls to new articles urls
Showing
4 changed files
with
122 additions
and
0 deletions
Show diff stats
app/controllers/public/content_viewer_controller.rb
| ... | ... | @@ -15,6 +15,14 @@ class ContentViewerController < ApplicationController |
| 15 | 15 | end |
| 16 | 16 | else |
| 17 | 17 | @page = profile.articles.find_by_path(path) |
| 18 | + unless @page | |
| 19 | + page_from_old_path = profile.articles.find_by_old_path(path) | |
| 20 | + if page_from_old_path | |
| 21 | + flash[:notice] = _("Redirected from \"%s\". please update your links and bookmarks.") % request.url | |
| 22 | + redirect_to :profile => profile.identifier, :page => page_from_old_path.explode_path | |
| 23 | + return | |
| 24 | + end | |
| 25 | + end | |
| 18 | 26 | |
| 19 | 27 | # do not show unpublished articles |
| 20 | 28 | if @page && !@page.published | ... | ... |
app/models/article.rb
| ... | ... | @@ -193,6 +193,10 @@ class Article < ActiveRecord::Base |
| 193 | 193 | ['id', 'profile_id', 'parent_id', 'slug', 'path', 'updated_at', 'created_at', 'last_changed_by_id', 'version', 'lock_version', 'type', 'children_count', 'comments_count'] |
| 194 | 194 | end |
| 195 | 195 | |
| 196 | + def self.find_by_old_path(old_path) | |
| 197 | + find(:first, :include => :versions, :conditions => ['article_versions.path = ?', old_path], :order => 'article_versions.id desc') | |
| 198 | + end | |
| 199 | + | |
| 196 | 200 | private |
| 197 | 201 | |
| 198 | 202 | def sanitize_tag_list | ... | ... |
test/functional/content_viewer_controller_test.rb
| ... | ... | @@ -461,4 +461,60 @@ class ContentViewerControllerTest < Test::Unit::TestCase |
| 461 | 461 | assert_response 403 |
| 462 | 462 | end |
| 463 | 463 | |
| 464 | + should 'redirect to new article path under an old path' do | |
| 465 | + p = create_user('test_user').person | |
| 466 | + a = p.articles.create(:name => 'old-name') | |
| 467 | + old_path = a.explode_path | |
| 468 | + a.name = 'new-name' | |
| 469 | + a.save! | |
| 470 | + | |
| 471 | + get :view_page, :profile => p.identifier, :page => old_path | |
| 472 | + | |
| 473 | + assert_response :redirect | |
| 474 | + assert_redirected_to :profile => p.identifier, :page => a.explode_path | |
| 475 | + end | |
| 476 | + | |
| 477 | + should 'load new article name equal of another article old name' do | |
| 478 | + p = create_user('test_user').person | |
| 479 | + a1 = p.articles.create!(:name => 'old-name') | |
| 480 | + old_path = a1.explode_path | |
| 481 | + a1.name = 'new-name' | |
| 482 | + a1.save! | |
| 483 | + a2 = p.articles.create!(:name => 'old-name') | |
| 484 | + | |
| 485 | + get :view_page, :profile => p.identifier, :page => old_path | |
| 486 | + | |
| 487 | + assert_equal a2, assigns(:page) | |
| 488 | + end | |
| 489 | + | |
| 490 | + should 'redirect to article with most recent version with the name if there is no article with the name' do | |
| 491 | + p = create_user('test_user').person | |
| 492 | + a1 = p.articles.create!(:name => 'old-name') | |
| 493 | + old_path = a1.explode_path | |
| 494 | + a1.name = 'new-name' | |
| 495 | + a1.save! | |
| 496 | + a2 = p.articles.create!(:name => 'old-name') | |
| 497 | + a2.name = 'other-new-name' | |
| 498 | + a2.save! | |
| 499 | + | |
| 500 | + get :view_page, :profile => p.identifier, :page => old_path | |
| 501 | + | |
| 502 | + assert_response :redirect | |
| 503 | + assert_redirected_to :profile => p.identifier, :page => a2.explode_path | |
| 504 | + end | |
| 505 | + | |
| 506 | + should 'not return an article of a different user' do | |
| 507 | + p1 = create_user('test_user').person | |
| 508 | + a = p1.articles.create!(:name => 'old-name') | |
| 509 | + old_path = a.explode_path | |
| 510 | + a.name = 'new-name' | |
| 511 | + a.save! | |
| 512 | + | |
| 513 | + p2 = create_user('another_user').person | |
| 514 | + | |
| 515 | + get :view_page, :profile => p2.identifier, :page => old_path | |
| 516 | + | |
| 517 | + assert_response :missing | |
| 518 | + end | |
| 519 | + | |
| 464 | 520 | end | ... | ... |
test/unit/article_test.rb
| ... | ... | @@ -490,4 +490,58 @@ class ArticleTest < Test::Unit::TestCase |
| 490 | 490 | assert_kind_of Folder, b |
| 491 | 491 | end |
| 492 | 492 | |
| 493 | + should 'load article under an old path' do | |
| 494 | + p = create_user('test_user').person | |
| 495 | + a = p.articles.create(:name => 'old-name') | |
| 496 | + old_path = a.explode_path | |
| 497 | + a.name = 'new-name' | |
| 498 | + a.save! | |
| 499 | + | |
| 500 | + page = Article.find_by_old_path(old_path) | |
| 501 | + | |
| 502 | + assert_equal a.path, page.path | |
| 503 | + end | |
| 504 | + | |
| 505 | + should 'load new article name equal of another article old name' do | |
| 506 | + p = create_user('test_user').person | |
| 507 | + a1 = p.articles.create!(:name => 'old-name') | |
| 508 | + old_path = a1.explode_path | |
| 509 | + a1.name = 'new-name' | |
| 510 | + a1.save! | |
| 511 | + a2 = p.articles.create!(:name => 'old-name') | |
| 512 | + | |
| 513 | + page = Article.find_by_old_path(old_path) | |
| 514 | + | |
| 515 | + assert_equal a2.path, page.path | |
| 516 | + end | |
| 517 | + | |
| 518 | + should 'article with most recent version with the name must be loaded if no aritcle with the name' do | |
| 519 | + p = create_user('test_user').person | |
| 520 | + a1 = p.articles.create!(:name => 'old-name') | |
| 521 | + old_path = a1.explode_path | |
| 522 | + a1.name = 'new-name' | |
| 523 | + a1.save! | |
| 524 | + a2 = p.articles.create!(:name => 'old-name') | |
| 525 | + a2.name = 'other-new-name' | |
| 526 | + a2.save! | |
| 527 | + | |
| 528 | + page = Article.find_by_old_path(old_path) | |
| 529 | + | |
| 530 | + assert_equal a2.path, page.path | |
| 531 | + end | |
| 532 | + | |
| 533 | + should 'not return an article of a different user' do | |
| 534 | + p1 = create_user('test_user').person | |
| 535 | + a = p1.articles.create!(:name => 'old-name') | |
| 536 | + old_path = a.explode_path | |
| 537 | + a.name = 'new-name' | |
| 538 | + a.save! | |
| 539 | + | |
| 540 | + p2 = create_user('another_user').person | |
| 541 | + | |
| 542 | + page = p2.articles.find_by_old_path(old_path) | |
| 543 | + | |
| 544 | + assert_nil page | |
| 545 | + end | |
| 546 | + | |
| 493 | 547 | end | ... | ... |