ActionItem862: small enhancements and bugfixes

Antonio Terceiro
1 parent 3ddde8cf
Showing 22 changed files with 102 additions and 54 deletions Show diff stats
app/controllers/my_profile/cms_controller.rb
app/controllers/public/search_controller.rb
app/helpers/application_helper.rb
app/models/article.rb
app/models/comment.rb
app/models/profile.rb
app/views/account/_signup_form.rhtml
app/views/account/accept_terms.rhtml
app/views/blocks/profile_image.rhtml
app/views/blocks/profile_info_actions/person.rhtml
app/views/cms/_textile_article.rhtml
app/views/cms/_tiny_mce_article.rhtml
app/views/comment/notifier/mail.rhtml
app/views/profile_editor/index.rhtml
app/views/tasks/index.rhtml
public/stylesheets/controller_profile_editor.css
public/stylesheets/forms.css
test/functional/cms_controller_test.rb
test/unit/article_test.rb
test/unit/comment_notifier_test.rb
@@ -22,7 +22,6 @@ class CmsController &lt; MyProfileController
     articles = [
       TinyMceArticle,
       TextileArticle,
-      UploadedFile,
       Event
     ]
     parent_id = params ? params[:parent_id] : nil
@@ -39,7 +38,7 @@ class CmsController &lt; MyProfileController
   end
  
   def special_article_types
-    [Blog]
+    [Blog, UploadedFile]
   end
  
   def view
@@ -195,7 +194,7 @@ class CmsController &lt; MyProfileController
  
   def record_creating_from_public_view
     referer = request.referer
-    if (referer =~ Regexp.new("^#{(url_for(profile.url).sub('https:', 'https?:'))}"))
+    if (referer =~ Regexp.new("^#{(url_for(profile.url).sub('https:', 'https?:'))}")) || params[:back_to] == 'public_view'
       @back_to = 'public_view'
       @back_url = referer
     end
@@ -116,7 +116,7 @@ class SearchController &lt; PublicController
   def limit
     searching = @searching.values.select{|v|v}
     if params[:display] == 'map'
-      100
+      1200
     else
       (searching.size == 1) ? 20 : 6
     end
@@ -432,7 +432,8 @@ module ApplicationHelper
             profile.url,
             :onclick => 'document.location.href = this.href', # work-arround for ie.
             :class => 'profile_link url',
-            :help => _('Click on this icon to go to the <b>%s</b>\'s home page') % profile.name ),
+            :help => _('Click on this icon to go to the <b>%s</b>\'s home page') % profile.name,
+            :title => profile.name ),
         :class => 'vcard'
   end
  
@@ -4,9 +4,10 @@ class Article &lt; ActiveRecord::Base
   before_save :sanitize_tag_list
  
   belongs_to :profile
-  validates_presence_of :profile_id, :name, :slug, :path
+  validates_presence_of :profile_id, :name
+  validates_presence_of :slug, :path, :if => lambda { |article| !article.name.blank? }
  
-  validates_uniqueness_of :slug, :scope => ['profile_id', 'parent_id'], :message => _('%{fn} (the code generated from the article name) is already being used by another article.')
+  validates_uniqueness_of :slug, :scope => ['profile_id', 'parent_id'], :message => _('%{fn} (the code generated from the article name) is already being used by another article.'), :if => lambda { |article| !article.slug.blank? }
  
   belongs_to :last_changed_by, :class_name => 'Person', :foreign_key => 'last_changed_by_id'
  
@@ -15,6 +16,15 @@ class Article &lt; ActiveRecord::Base
   has_many :article_categorizations, :conditions => [ 'articles_categories.virtual = ?', false ]
   has_many :categories, :through => :article_categorizations
  
+  def self.human_attribute_name(attrib)
+    case attrib.to_sym
+    when :name
+      _('Title')
+    else
+      _(self.superclass.human_attribute_name(attrib))
+    end
+  end
+
   def pending_categorizations
     @pending_categorizations ||= []
   end
@@ -31,6 +31,10 @@ class Comment &lt; ActiveRecord::Base
     author ? author.email : email
   end
  
+  def author_link
+    author ? author.url : email
+  end
+
   def url
     article.url.merge(:anchor => anchor)
   end
@@ -58,15 +62,18 @@ class Comment &lt; ActiveRecord::Base
   class Notifier < ActionMailer::Base
     def mail(comment)
       profile = comment.article.profile
-      recipients profile.email
+      # FIXME hardcoded
+      email = profile.person? ? profile.email : profile.contact_email
+      return unless email
+      recipients email
+
       from "#{profile.environment.name} <#{profile.environment.contact_email}>"
-      subject _("%s - New comment in '%s'") % [profile.environment.name, comment.article.title]
-      headers['Reply-To'] = comment.author_email
-      body :name => comment.author_name,
-        :email => comment.author_email,
-        :title => comment.title,
-        :body => comment.body,
-        :article_url => comment.url,
+      subject _("[%s] you got a new comment!") % [profile.environment.name, comment.article.title]
+      body :recipient => profile.nickname || profile.name,
+        :sender => comment.author_name,
+        :sender_link => comment.author_link,
+        :article_title => comment.article.name,
+        :comment_url => comment.url,
         :environment => profile.environment.name,
         :url => profile.environment.top_url
     end
@@ -454,7 +454,7 @@ class Profile &lt; ActiveRecord::Base
   include ActionView::Helpers::TextHelper
   def short_name
     if self[:nickname].blank?
-      truncate self.identifier, 15, '...'
+      truncate self.name, 15, '...'
     else
       self[:nickname]
     end
@@ -39,9 +39,9 @@ in this environment.&#39;) % [environment.name, __(&#39;communities&#39;), __(&#39;enterprises&#39;)
 <%= icaptcha_field() %>
  
 <% if @terms_of_use %>
-  <p>
+  <div style='height: 200px; overflow: auto;'>
     <%= @terms_of_use %>
-  </p>
+  </div>
   <p>
     <%= check_box 'user', 'terms_accepted' %> 
     <%= _('I accept the terms of use') %>
@@ -13,7 +13,7 @@
 <div class=activation-box>
   <h2><%= _('Enterprise activation') + ' - ' + (logged_in? ? _('part 2 of 2') : _(' part 2 of 3')) %></h2>
  
-  <div id='terms-of-enterprise-use'><%= @terms_of_enterprise_use %></div>
+  <div id='terms-of-enterprise-use' class='height: 200px; overflow: auto;'><%= @terms_of_enterprise_use %></div>
  
   <% form_tag :action => 'activate_enterprise' do %>
     <%= hidden_field_tag :enterprise_code, params[:enterprise_code] %>
@@ -10,4 +10,10 @@
   </div>
 </div>
  
+<% if !user.nil? and user.has_permission?('edit_profile', profile) %>
+  <div style='text-align: center; font-size: 75%; clear: both'>
+    <%= link_to _('Control panel'), :controller => 'profile_editor' %>
+  </div>
+<% end %>
+
 </div><!-- end class="vcard" -->
 <ul>
   <%if logged_in? && (user != profile) %>
-    <%if user.friends.include?(profile) %>
-      <li> <%= link_to content_tag('span', _('Send request')), {:profile => user.identifier, :controller => 'tasks', :action => 'new', :target_id => profile.id}, :class => 'button with-text icon-menu-mail' %> </li>
-    <% elsif !user.already_request_friendship?(profile) %>
+    <% if !user.already_request_friendship?(profile) %>
       <li><%= link_to content_tag('span', __('Add friend')), { :profile => user.identifier, :controller => 'friends', :action => 'add', :id => profile.id }, :class => 'button with-text icon-add' %></li>
     <% end %>
   <% end %>
@@ -2,7 +2,7 @@
  
 <%# TODO add Textile help here %>
  
-<%= required f.text_field('name', :size => '64') %>
+<%= required labelled_form_field(_('Title'), text_field(:article, 'name', :size => '64')) %>
  
-<%= f.text_area('body', :cols => 64) %>
+<%= labelled_form_field(_('Text'), text_area(:article, 'body', :cols => 64)) %>
  
@@ -2,7 +2,7 @@
  
 <%= render :file => 'shared/tiny_mce' %>
  
-<%= required f.text_field('name', :size => '64') %>
+<%= required labelled_form_field(_('Title'), text_field(:article, 'name', :size => '64')) %>
  
-<%= f.text_area('body', :cols => 40, :style => 'width:99%') %>
+<%= labelled_form_field(_('Text'), text_area(:article, 'body', :cols => 40, :style => 'width:99%')) %>
  
-<%= _('Name: %s') % @name %>
-<%= _('e-Mail: %s') % @email %>
-<%= _('Title: %s') % @title %>
-<%= _('Comment:') %>
---
+<%= _('Hi, %{recipient}!') % { :recipient => @recipient } %>
  
-<%= @body %>
+<%= word_wrap(_('%{sender} (%{sender_link}) created a new comment on your article "%{article_title}".') % { :sender => @sender, :sender_link => url_for(@sender_link), :article_title => @article_title }) %>
  
---
-<%= url_for @article_url %>
+<%= _('Access the address below to view this comment:') %>
+<%= url_for @comment_url %>
  
 --
 <%= _('%s environment system') % @environment %>
 <div id="profile-editor-index">
  
-<h1 class="block-title"><%= _("%s's profile: control panel") % profile.name %></h1>
+  <h1 class="block-title">
+    <div class='control-panel-title'><%= profile.name %></div>
+    <div class='control-panel-subtitle'><%= _('Control Panel') %></div>
+  </h1>
  
 <%= render :partial => 'pending_tasks' %>
  
@@ -8,7 +11,7 @@
  
 <% file_manager do %>
  
-  <%= file_manager_button(_('Profile settings'), 'icons-app/edit-profile.png', :controller => 'profile_editor', :action => 'edit') %>
+  <%= file_manager_button(_('Info and settings'), 'icons-app/edit-profile.png', :controller => 'profile_editor', :action => 'edit') %>
  
   <%= file_manager_button(_('Mail settings'), 'icons-app/mail.png', :controller => 'mailconf') if profile.person? && MailConf.enabled? %>
  
@@ -14,7 +14,7 @@
  
 <% button_bar do %>
   <%= button(:edit, _('View processed tasks'), :action => 'processed') %>
-  <%= button(:edit, _('View my requests'), :action => 'list_requested') %>
-  <%= button('menu-mail',  _('Send request'), :action => 'new') %>
+  <%# FiXME button(:edit, _('View my requests'), :action => 'list_requested') %>
+  <%# FIXME button('menu-mail',  _('Send request'), :action => 'new') %>
   <%= button(:back, _('Back to control panel'), :controller => 'profile_editor') %>
 <% end %>
@@ -60,3 +60,10 @@
 .file-manager-button span {
   display: block;
 }
+
+.control-panel-title {
+}
+.control-panel-subtitle {
+  font-size: 80%;
+  font-style: italic;
+}
@@ -76,10 +76,11 @@
  
 /*** REQUIRED FIELDS ***/
  
-#content form .required-field label.formlabel {
+.required-field label {
   font-weight: bold;
+  color: #c00;
 }
  
-#content form .required-field label.formlabel:after {
+.required-field label:after {
   content: ' (*)';
 }
@@ -544,6 +544,12 @@ class CmsControllerTest &lt; Test::Unit::TestCase
     assert_redirected_to @profile.articles.find_by_name('new-article-from-public-view').url
   end
  
+  should 'keep the back_to hint in unsuccessfull saves' do
+    post :new, :profile => 'testinguser', :type => 'TextileArticle', :back_to => 'public_view', :article => { }
+    assert_response :success
+    assert_tag :tag => "input", :attributes => { :type => 'hidden', :name => 'back_to', :value => 'public_view' }
+  end
+
   should 'create a private article child of private folder' do
     folder = Folder.new(:name => 'my intranet', :public_article => false); profile.articles << folder; folder.save!
  
@@ -19,17 +19,29 @@ class ArticleTest &lt; Test::Unit::TestCase
     assert !a.errors.invalid?(:profile_id)
   end
  
-  should 'require values for name, slug and path' do
+  should 'require value for name' do
     a = Article.new
     a.valid?
     assert a.errors.invalid?(:name)
-    assert a.errors.invalid?(:slug)
-    assert a.errors.invalid?(:path)
  
     a.name = 'my article'
     a.valid?
     assert !a.errors.invalid?(:name)
-    assert !a.errors.invalid?(:name)
+  end
+
+  should 'require value for slug and path if name is filled' do
+    a = Article.new(:name => 'test article')
+    a.slug = nil
+    a.path = nil
+    a.valid?
+    assert a.errors.invalid?(:slug)
+    assert a.errors.invalid?(:path)
+  end
+
+  should 'not require value for slug and path if name is blank' do
+    a = Article.new
+    a.valid?
+    assert !a.errors.invalid?(:slug)
     assert !a.errors.invalid?(:path)
   end
  
@@ -37,14 +37,6 @@ class CommentNotifierTest &lt; Test::Unit::TestCase
     assert_match /user_comment_test/, sent.body
   end
  
-  should "add a Reply-To header set to the commenter e-mail" do
-    p = create_user('user_comment_test', :email => 'my@email.com').person
-    a = Article.create!(:name => 'Article test', :profile => p, :notify_comments => true)
-    a.comments << Comment.new(:author => p, :title => 'test comment', :body => 'you suck!')
-    sent = ActionMailer::Base.deliveries.first
-    assert_equal ['my@email.com'], sent.reply_to
-  end
-
   should 'display unauthenticated author name and email in delivered mail' do
     p = create_user('user_comment_test').person
     a = Article.create!(:name => 'Article test', :profile => p, :notify_comments => true)
@@ -87,6 +87,16 @@ class CommentTest &lt; Test::Unit::TestCase
     assert_equal 'my@email.com', Comment.new(:email => 'my@email.com').author_email
   end
  
+  should 'provide author link for authenticated author' do
+    author = Person.new
+    author.expects(:url).returns('http://blabla.net/author')
+    assert_equal  'http://blabla.net/author', Comment.new(:author => author).author_link
+  end
+
+  should 'provide author e-mail as author link for unauthenticated author' do
+    assert_equal 'my@email.com', Comment.new(:email => 'my@email.com').author_link
+  end
+
   should 'provide url to comment' do
     art = Article.new
     art.expects(:url).returns({ :controller => 'lala', :action => 'something' })
@@ -758,8 +758,8 @@ class ProfileTest &lt; Test::Unit::TestCase
     assert_equal 'code', p.nickname
   end
  
-  should 'short_name return truncated identifier if nickname is blank' do
-    p = Profile.new(:identifier => 'a123456789abcdefghij')
+  should 'return truncated name in short_name if nickname is blank' do
+    p = Profile.new(:name => 'a123456789abcdefghij')
     assert_equal 'a123456789ab...', p.short_name
   end
...	...	@@ -22,7 +22,6 @@ class CmsController < MyProfileController
22	22	articles = [
23	23	TinyMceArticle,
24	24	TextileArticle,
25		- UploadedFile,
26	25	Event
27	26	]
28	27	parent_id = params ? params[:parent_id] : nil
...	...	@@ -39,7 +38,7 @@ class CmsController < MyProfileController
39	38	end
40	39
41	40	def special_article_types
42		- [Blog]
	41	+ [Blog, UploadedFile]
43	42	end
44	43
45	44	def view
...	...	@@ -195,7 +194,7 @@ class CmsController < MyProfileController
195	194
196	195	def record_creating_from_public_view
197	196	referer = request.referer
198		- if (referer =~ Regexp.new("^#{(url_for(profile.url).sub('https:', 'https?:'))}"))
	197	+ if (referer =~ Regexp.new("^#{(url_for(profile.url).sub('https:', 'https?:'))}")) \|\| params[:back_to] == 'public_view'
199	198	@back_to = 'public_view'
200	199	@back_url = referer
201	200	end
...	...
...	...	@@ -116,7 +116,7 @@ class SearchController < PublicController
116	116	def limit
117	117	searching = @searching.values.select{\|v\|v}
118	118	if params[:display] == 'map'
119		- 100
	119	+ 1200
120	120	else
121	121	(searching.size == 1) ? 20 : 6
122	122	end
...	...
...	...	@@ -432,7 +432,8 @@ module ApplicationHelper
432	432	profile.url,
433	433	:onclick => 'document.location.href = this.href', # work-arround for ie.
434	434	:class => 'profile_link url',
435		- :help => _('Click on this icon to go to the <b>%s</b>\'s home page') % profile.name ),
	435	+ :help => _('Click on this icon to go to the <b>%s</b>\'s home page') % profile.name,
	436	+ :title => profile.name ),
436	437	:class => 'vcard'
437	438	end
438	439
...	...
...	...	@@ -4,9 +4,10 @@ class Article < ActiveRecord::Base
4	4	before_save :sanitize_tag_list
5	5
6	6	belongs_to :profile
7		- validates_presence_of :profile_id, :name, :slug, :path
	7	+ validates_presence_of :profile_id, :name
	8	+ validates_presence_of :slug, :path, :if => lambda { \|article\| !article.name.blank? }
8	9
9		- validates_uniqueness_of :slug, :scope => ['profile_id', 'parent_id'], :message => _('%{fn} (the code generated from the article name) is already being used by another article.')
	10	+ validates_uniqueness_of :slug, :scope => ['profile_id', 'parent_id'], :message => _('%{fn} (the code generated from the article name) is already being used by another article.'), :if => lambda { \|article\| !article.slug.blank? }
10	11
11	12	belongs_to :last_changed_by, :class_name => 'Person', :foreign_key => 'last_changed_by_id'
12	13
...	...	@@ -15,6 +16,15 @@ class Article < ActiveRecord::Base
15	16	has_many :article_categorizations, :conditions => [ 'articles_categories.virtual = ?', false ]
16	17	has_many :categories, :through => :article_categorizations
17	18
	19	+ def self.human_attribute_name(attrib)
	20	+ case attrib.to_sym
	21	+ when :name
	22	+ _('Title')
	23	+ else
	24	+ _(self.superclass.human_attribute_name(attrib))
	25	+ end
	26	+ end
	27	+
18	28	def pending_categorizations
19	29	@pending_categorizations \|\|= []
20	30	end
...	...
...	...	@@ -31,6 +31,10 @@ class Comment < ActiveRecord::Base
31	31	author ? author.email : email
32	32	end
33	33
	34	+ def author_link
	35	+ author ? author.url : email
	36	+ end
	37	+
34	38	def url
35	39	article.url.merge(:anchor => anchor)
36	40	end
...	...	@@ -58,15 +62,18 @@ class Comment < ActiveRecord::Base
58	62	class Notifier < ActionMailer::Base
59	63	def mail(comment)
60	64	profile = comment.article.profile
61		- recipients profile.email
	65	+ # FIXME hardcoded
	66	+ email = profile.person? ? profile.email : profile.contact_email
	67	+ return unless email
	68	+ recipients email
	69	+
62	70	from "#{profile.environment.name} <#{profile.environment.contact_email}>"
63		- subject _("%s - New comment in '%s'") % [profile.environment.name, comment.article.title]
64		- headers['Reply-To'] = comment.author_email
65		- body :name => comment.author_name,
66		- :email => comment.author_email,
67		- :title => comment.title,
68		- :body => comment.body,
69		- :article_url => comment.url,
	71	+ subject _("[%s] you got a new comment!") % [profile.environment.name, comment.article.title]
	72	+ body :recipient => profile.nickname \|\| profile.name,
	73	+ :sender => comment.author_name,
	74	+ :sender_link => comment.author_link,
	75	+ :article_title => comment.article.name,
	76	+ :comment_url => comment.url,
70	77	:environment => profile.environment.name,
71	78	:url => profile.environment.top_url
72	79	end
...	...
...	...	@@ -454,7 +454,7 @@ class Profile < ActiveRecord::Base
454	454	include ActionView::Helpers::TextHelper
455	455	def short_name
456	456	if self[:nickname].blank?
457		- truncate self.identifier, 15, '...'
	457	+ truncate self.name, 15, '...'
458	458	else
459	459	self[:nickname]
460	460	end
...	...
...	...	@@ -39,9 +39,9 @@ in this environment.') % [environment.name, __('communities'), __('enterprises')
39	39	<%= icaptcha_field() %>
40	40
41	41	<% if @terms_of_use %>
42		- <p>
	42	+ <div style='height: 200px; overflow: auto;'>
43	43	<%= @terms_of_use %>
44		- </p>
	44	+ </div>
45	45	<p>
46	46	<%= check_box 'user', 'terms_accepted' %>
47	47	<%= _('I accept the terms of use') %>
...	...
...	...	@@ -13,7 +13,7 @@
13	13	<div class=activation-box>
14	14	<h2><%= _('Enterprise activation') + ' - ' + (logged_in? ? _('part 2 of 2') : _(' part 2 of 3')) %></h2>
15	15
16		- <div id='terms-of-enterprise-use'><%= @terms_of_enterprise_use %></div>
	16	+ <div id='terms-of-enterprise-use' class='height: 200px; overflow: auto;'><%= @terms_of_enterprise_use %></div>
17	17
18	18	<% form_tag :action => 'activate_enterprise' do %>
19	19	<%= hidden_field_tag :enterprise_code, params[:enterprise_code] %>
...	...
...	...	@@ -10,4 +10,10 @@
10	10	</div>
11	11	</div>
12	12
	13	+<% if !user.nil? and user.has_permission?('edit_profile', profile) %>
	14	+ <div style='text-align: center; font-size: 75%; clear: both'>
	15	+ <%= link_to _('Control panel'), :controller => 'profile_editor' %>
	16	+ </div>
	17	+<% end %>
	18	+
13	19	</div><!-- end class="vcard" -->
...	...
1	1	<ul>
2	2	<%if logged_in? && (user != profile) %>
3		- <%if user.friends.include?(profile) %>
4		- <li> <%= link_to content_tag('span', _('Send request')), {:profile => user.identifier, :controller => 'tasks', :action => 'new', :target_id => profile.id}, :class => 'button with-text icon-menu-mail' %> </li>
5		- <% elsif !user.already_request_friendship?(profile) %>
	3	+ <% if !user.already_request_friendship?(profile) %>
6	4	<li><%= link_to content_tag('span', __('Add friend')), { :profile => user.identifier, :controller => 'friends', :action => 'add', :id => profile.id }, :class => 'button with-text icon-add' %></li>
7	5	<% end %>
8	6	<% end %>
...	...