Commit 1e455329edcc3d9e35a9fa82102d38b33251896b
1 parent
2cb08de6
Exists in
master
and in
29 other branches
ActionItem172: generating URL hashes instead of actual URL's, since Profile#url …
…is almost always used in link_to calls git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@1472 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
10 changed files
with
32 additions
and
46 deletions
Show diff stats
app/models/article.rb
| @@ -69,14 +69,6 @@ class Article < ActiveRecord::Base | @@ -69,14 +69,6 @@ class Article < ActiveRecord::Base | ||
| 69 | name | 69 | name |
| 70 | end | 70 | end |
| 71 | 71 | ||
| 72 | - def public_path(with_profile = true) | ||
| 73 | - elements = [path] | ||
| 74 | - if with_profile | ||
| 75 | - elements.unshift(profile.identifier) | ||
| 76 | - end | ||
| 77 | - "/" + elements.join('/') | ||
| 78 | - end | ||
| 79 | - | ||
| 80 | def self.short_description | 72 | def self.short_description |
| 81 | if self == Article | 73 | if self == Article |
| 82 | _('Article') | 74 | _('Article') |
| @@ -98,7 +90,7 @@ class Article < ActiveRecord::Base | @@ -98,7 +90,7 @@ class Article < ActiveRecord::Base | ||
| 98 | end | 90 | end |
| 99 | 91 | ||
| 100 | def url | 92 | def url |
| 101 | - self.profile.url + self.public_path(false) | 93 | + self.profile.url.merge(:page => path.split('/')) |
| 102 | end | 94 | end |
| 103 | 95 | ||
| 104 | def allow_children? | 96 | def allow_children? |
app/models/change_password.rb
| @@ -2,8 +2,6 @@ | @@ -2,8 +2,6 @@ | ||
| 2 | 2 | ||
| 3 | class ChangePassword < Task | 3 | class ChangePassword < Task |
| 4 | 4 | ||
| 5 | - include Noosfero::URL | ||
| 6 | - | ||
| 7 | serialize :data, Hash | 5 | serialize :data, Hash |
| 8 | def data | 6 | def data |
| 9 | self[:data] ||= {} | 7 | self[:data] ||= {} |
| @@ -69,10 +67,11 @@ class ChangePassword < Task | @@ -69,10 +67,11 @@ class ChangePassword < Task | ||
| 69 | _('Your password was changed successfully.') | 67 | _('Your password was changed successfully.') |
| 70 | end | 68 | end |
| 71 | 69 | ||
| 70 | + include ActionController::UrlWriter | ||
| 72 | def task_created_message | 71 | def task_created_message |
| 73 | hostname = self.requestor.environment.default_hostname | 72 | hostname = self.requestor.environment.default_hostname |
| 74 | code = self.code | 73 | code = self.code |
| 75 | - url = generate_url(:host => hostname, :controller => 'account', :action => 'new_password', :code => code) | 74 | + url = url_for(:host => hostname, :controller => 'account', :action => 'new_password', :code => code) |
| 76 | 75 | ||
| 77 | lambda do | 76 | lambda do |
| 78 | _("In order to change your password, please visit the following address:\n\n%s") % url | 77 | _("In order to change your password, please visit the following address:\n\n%s") % url |
app/models/profile.rb
| @@ -183,7 +183,6 @@ class Profile < ActiveRecord::Base | @@ -183,7 +183,6 @@ class Profile < ActiveRecord::Base | ||
| 183 | false | 183 | false |
| 184 | end | 184 | end |
| 185 | 185 | ||
| 186 | - include ActionController::UrlWriter | ||
| 187 | def url | 186 | def url |
| 188 | generate_url(url_options.merge(:controller => 'content_viewer', :action => 'view_page', :page => [])) | 187 | generate_url(url_options.merge(:controller => 'content_viewer', :action => 'view_page', :page => [])) |
| 189 | end | 188 | end |
| @@ -197,14 +196,11 @@ class Profile < ActiveRecord::Base | @@ -197,14 +196,11 @@ class Profile < ActiveRecord::Base | ||
| 197 | end | 196 | end |
| 198 | 197 | ||
| 199 | def generate_url(options) | 198 | def generate_url(options) |
| 200 | - url_for(url_options.merge(options)) | 199 | + url_options.merge(options) |
| 201 | end | 200 | end |
| 202 | 201 | ||
| 203 | def url_options | 202 | def url_options |
| 204 | - options = { :host => self.environment.default_hostname, :profile => self.identifier} | ||
| 205 | - options.merge!(:port => 3000) if ENV['RAILS_ENV'] == 'development' | ||
| 206 | - | ||
| 207 | - options | 203 | + { :host => self.environment.default_hostname, :profile => self.identifier} |
| 208 | end | 204 | end |
| 209 | 205 | ||
| 210 | # FIXME this can be SLOW | 206 | # FIXME this can be SLOW |
app/models/recent_documents_block.rb
app/models/rss_feed.rb
| @@ -52,6 +52,7 @@ class RssFeed < Article | @@ -52,6 +52,7 @@ class RssFeed < Article | ||
| 52 | 'text/xml' | 52 | 'text/xml' |
| 53 | end | 53 | end |
| 54 | 54 | ||
| 55 | + include ActionController::UrlWriter | ||
| 55 | def data | 56 | def data |
| 56 | articles = | 57 | articles = |
| 57 | if (self.include == 'parent_and_children') && self.parent | 58 | if (self.include == 'parent_and_children') && self.parent |
| @@ -68,7 +69,7 @@ class RssFeed < Article | @@ -68,7 +69,7 @@ class RssFeed < Article | ||
| 68 | xml.rss(:version=>"2.0") do | 69 | xml.rss(:version=>"2.0") do |
| 69 | xml.channel do | 70 | xml.channel do |
| 70 | xml.title(_("%s's RSS feed") % (self.profile.name)) | 71 | xml.title(_("%s's RSS feed") % (self.profile.name)) |
| 71 | - xml.link(self.profile.url) | 72 | + xml.link(url_for(self.profile.url)) |
| 72 | xml.description(_("%s's content published at %s") % [self.profile.name, self.profile.environment.name]) | 73 | xml.description(_("%s's content published at %s") % [self.profile.name, self.profile.environment.name]) |
| 73 | xml.language("pt_BR") | 74 | xml.language("pt_BR") |
| 74 | for article in articles | 75 | for article in articles |
| @@ -83,8 +84,8 @@ class RssFeed < Article | @@ -83,8 +84,8 @@ class RssFeed < Article | ||
| 83 | # rfc822 | 84 | # rfc822 |
| 84 | xml.pubDate(article.created_on.rfc2822) | 85 | xml.pubDate(article.created_on.rfc2822) |
| 85 | # link to article | 86 | # link to article |
| 86 | - xml.link(article.url) | ||
| 87 | - xml.guid(article.url) | 87 | + xml.link(url_for(article.url)) |
| 88 | + xml.guid(url_for(article.url)) | ||
| 88 | end | 89 | end |
| 89 | end | 90 | end |
| 90 | end | 91 | end |
app/models/tags_block.rb
| @@ -2,6 +2,7 @@ class TagsBlock < Block | @@ -2,6 +2,7 @@ class TagsBlock < Block | ||
| 2 | 2 | ||
| 3 | include TagsHelper | 3 | include TagsHelper |
| 4 | include BlockHelper | 4 | include BlockHelper |
| 5 | + include ActionController::UrlWriter | ||
| 5 | 6 | ||
| 6 | def self.description | 7 | def self.description |
| 7 | _('Block listing content count by tag') | 8 | _('Block listing content count by tag') |
| @@ -16,7 +17,7 @@ class TagsBlock < Block | @@ -16,7 +17,7 @@ class TagsBlock < Block | ||
| 16 | block_title(_('Tags')) + | 17 | block_title(_('Tags')) + |
| 17 | "\n<div class='tag_cloud'>\n"+ | 18 | "\n<div class='tag_cloud'>\n"+ |
| 18 | tag_cloud( owner.tags, :id, | 19 | tag_cloud( owner.tags, :id, |
| 19 | - owner.generate_url(:controller => 'profile', :action => 'tag') + '/', | 20 | + owner.generate_url(:controller => 'profile', :action => 'tag'), |
| 20 | :max_size => 18, :min_size => 9 ) + | 21 | :max_size => 18, :min_size => 9 ) + |
| 21 | "\n</div><!-- end class='tag_cloud' -->\n"; | 22 | "\n</div><!-- end class='tag_cloud' -->\n"; |
| 22 | end | 23 | end |
app/views/cms/view.rhtml
| @@ -59,7 +59,7 @@ | @@ -59,7 +59,7 @@ | ||
| 59 | <%= _('"%{article}", last changed by %{author}') % { :article => @article.name, :author => (@article.last_changed_by ? @article.last_changed_by.name : _('Unknown User')) } %> | 59 | <%= _('"%{article}", last changed by %{author}') % { :article => @article.name, :author => (@article.last_changed_by ? @article.last_changed_by.name : _('Unknown User')) } %> |
| 60 | </li> | 60 | </li> |
| 61 | <li> | 61 | <li> |
| 62 | - <%= _('Public address of this article: %s') % link_to(@article.public_path, @article.public_path) %> | 62 | + <%= _('Public address of this article: %s') % link_to(@article.url, @article.url) %> |
| 63 | </li> | 63 | </li> |
| 64 | <li> | 64 | <li> |
| 65 | <%= _('Tags:') %> <%= @article.tag_list %> | 65 | <%= _('Tags:') %> <%= @article.tag_list %> |
test/unit/article_test.rb
| @@ -106,19 +106,6 @@ class ArticleTest < Test::Unit::TestCase | @@ -106,19 +106,6 @@ class ArticleTest < Test::Unit::TestCase | ||
| 106 | assert a4.errors.invalid?(:slug) | 106 | assert a4.errors.invalid?(:slug) |
| 107 | end | 107 | end |
| 108 | 108 | ||
| 109 | - should 'calculate public path' do | ||
| 110 | - # top level | ||
| 111 | - a = profile.articles.build(:name => 'aaa') | ||
| 112 | - a.save! | ||
| 113 | - assert_equal "/#{profile.identifier}/aaa", a.public_path | ||
| 114 | - | ||
| 115 | - # child articles | ||
| 116 | - b = profile.articles.build(:name => 'bbb') | ||
| 117 | - b.parent = a | ||
| 118 | - b.save! | ||
| 119 | - assert_equal "/#{profile.identifier}/aaa/bbb", b.public_path | ||
| 120 | - end | ||
| 121 | - | ||
| 122 | should 'record who did the last change' do | 109 | should 'record who did the last change' do |
| 123 | a = profile.articles.build(:name => 'test') | 110 | a = profile.articles.build(:name => 'test') |
| 124 | 111 | ||
| @@ -172,7 +159,14 @@ class ArticleTest < Test::Unit::TestCase | @@ -172,7 +159,14 @@ class ArticleTest < Test::Unit::TestCase | ||
| 172 | article = profile.articles.build(:name => 'myarticle') | 159 | article = profile.articles.build(:name => 'myarticle') |
| 173 | article.save! | 160 | article.save! |
| 174 | 161 | ||
| 175 | - assert_equal(profile.url + "/myarticle", article.url) | 162 | + assert_equal(profile.url.merge(:page => ['myarticle']), article.url) |
| 163 | + end | ||
| 164 | + | ||
| 165 | + should 'provide a url to itself having a parent topic' do | ||
| 166 | + parent = profile.articles.build(:name => 'parent'); parent.save! | ||
| 167 | + child = profile.articles.build(:name => 'child', :parent => parent); child.save! | ||
| 168 | + | ||
| 169 | + assert_equal(profile.url.merge(:page => [ 'parent', 'child']), child.url) | ||
| 176 | end | 170 | end |
| 177 | 171 | ||
| 178 | should 'associate with categories' do | 172 | should 'associate with categories' do |
test/unit/profile_test.rb
| @@ -247,23 +247,23 @@ class ProfileTest < Test::Unit::TestCase | @@ -247,23 +247,23 @@ class ProfileTest < Test::Unit::TestCase | ||
| 247 | should 'provide url to itself' do | 247 | should 'provide url to itself' do |
| 248 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) | 248 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) |
| 249 | 249 | ||
| 250 | - assert_equal 'http://mycolivre.net/testprofile', profile.url | 250 | + assert_equal({ :host => 'mycolivre.net', :profile => 'testprofile', :controller => 'content_viewer', :action => 'view_page', :page => []}, profile.url) |
| 251 | end | 251 | end |
| 252 | 252 | ||
| 253 | should 'provide URL to admin area' do | 253 | should 'provide URL to admin area' do |
| 254 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) | 254 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) |
| 255 | - assert_equal 'http://mycolivre.net/myprofile/testprofile', profile.admin_url | 255 | + assert_equal({ :host => 'mycolivre.net', :profile => 'testprofile', :controller => 'profile_editor', :action => 'index'}, profile.admin_url) |
| 256 | end | 256 | end |
| 257 | 257 | ||
| 258 | should 'provide URL to public profile' do | 258 | should 'provide URL to public profile' do |
| 259 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) | 259 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) |
| 260 | - assert_equal 'http://mycolivre.net/profile/testprofile', profile.public_profile_url | 260 | + assert_equal({ :host => 'mycolivre.net', :profile => 'testprofile', :controller => 'profile', :action => 'index' }, profile.public_profile_url) |
| 261 | end | 261 | end |
| 262 | 262 | ||
| 263 | should 'generate URL' do | 263 | should 'generate URL' do |
| 264 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) | 264 | profile = Profile.create!(:name => "Test Profile", :identifier => 'testprofile', :environment_id => create_environment('mycolivre.net').id) |
| 265 | 265 | ||
| 266 | - assert_equal 'http://mycolivre.net/profile/testprofile/friends', profile.generate_url(:controller => 'profile', :action => 'friends') | 266 | + assert_equal({ :host => 'mycolivre.net', :profile => 'testprofile', :controller => 'profile', :action => 'friends' }, profile.generate_url(:controller => 'profile', :action => 'friends')) |
| 267 | end | 267 | end |
| 268 | 268 | ||
| 269 | should 'provide URL options' do | 269 | should 'provide URL options' do |
test/unit/rss_feed_test.rb
| @@ -32,7 +32,7 @@ class RssFeedTest < Test::Unit::TestCase | @@ -32,7 +32,7 @@ class RssFeedTest < Test::Unit::TestCase | ||
| 32 | feed = RssFeed.new(:name => 'testfeed') | 32 | feed = RssFeed.new(:name => 'testfeed') |
| 33 | feed.profile = profile | 33 | feed.profile = profile |
| 34 | feed.save! | 34 | feed.save! |
| 35 | - | 35 | + |
| 36 | rss = feed.data | 36 | rss = feed.data |
| 37 | assert_match /<item><title>article 1<\/title>/, rss | 37 | assert_match /<item><title>article 1<\/title>/, rss |
| 38 | assert_match /<item><title>article 2<\/title>/, rss | 38 | assert_match /<item><title>article 2<\/title>/, rss |
| @@ -48,7 +48,7 @@ class RssFeedTest < Test::Unit::TestCase | @@ -48,7 +48,7 @@ class RssFeedTest < Test::Unit::TestCase | ||
| 48 | feed = RssFeed.new(:name => 'testfeed') | 48 | feed = RssFeed.new(:name => 'testfeed') |
| 49 | feed.profile = profile | 49 | feed.profile = profile |
| 50 | feed.save! | 50 | feed.save! |
| 51 | - | 51 | + |
| 52 | rss = feed.data | 52 | rss = feed.data |
| 53 | assert_no_match /<item><title>testfeed<\/title>/, rss | 53 | assert_no_match /<item><title>testfeed<\/title>/, rss |
| 54 | end | 54 | end |
| @@ -112,7 +112,9 @@ class RssFeedTest < Test::Unit::TestCase | @@ -112,7 +112,9 @@ class RssFeedTest < Test::Unit::TestCase | ||
| 112 | feed.profile = profile | 112 | feed.profile = profile |
| 113 | feed.save! | 113 | feed.save! |
| 114 | 114 | ||
| 115 | - assert_match "<link>#{profile.url}</link>", feed.data | 115 | + profile.environment.expects(:default_hostname).returns('mysite.net').at_least_once |
| 116 | + | ||
| 117 | + assert_match "<link>http://mysite.net/testuser</link>", feed.data | ||
| 116 | end | 118 | end |
| 117 | 119 | ||
| 118 | should 'provide link to each article' do | 120 | should 'provide link to each article' do |
| @@ -123,8 +125,8 @@ class RssFeedTest < Test::Unit::TestCase | @@ -123,8 +125,8 @@ class RssFeedTest < Test::Unit::TestCase | ||
| 123 | feed.save! | 125 | feed.save! |
| 124 | 126 | ||
| 125 | data = feed.data | 127 | data = feed.data |
| 126 | - assert_match "<link>#{art.url}</link>", data | ||
| 127 | - assert_match "<guid>#{art.url}</guid>", data | 128 | + assert_match "<link>http://#{art.profile.environment.default_hostname}/testuser/myarticle</link>", data |
| 129 | + assert_match "<guid>http://#{art.profile.environment.default_hostname}/testuser/myarticle</guid>", data | ||
| 128 | end | 130 | end |
| 129 | 131 | ||
| 130 | should 'be able to indicate maximum number of items' do | 132 | should 'be able to indicate maximum number of items' do |