Commit 339df62d33ed7e1fd046e68821b7928f06bd822b
1 parent
2e89b866
Exists in
master
and in
8 other branches
Enhancements on privacy settings
Added tests Added background on edition of profile to be easier to know which field the checkbox is related (ActionItem1717)
Showing
8 changed files
with
124 additions
and
24 deletions
Show diff stats
app/helpers/profile_editor_helper.rb
| @@ -147,7 +147,7 @@ module ProfileEditorHelper | @@ -147,7 +147,7 @@ module ProfileEditorHelper | ||
| 147 | 147 | ||
| 148 | def unchangeable_privacy_field(profile) | 148 | def unchangeable_privacy_field(profile) |
| 149 | if profile.public? | 149 | if profile.public? |
| 150 | - labelled_check_box(_('Public'), '', '', true, :disabled => true) | 150 | + labelled_check_box(_('Public'), '', '', true, :disabled => true, :title => _('This field must be public'), :class => 'disabled') |
| 151 | else | 151 | else |
| 152 | '' | 152 | '' |
| 153 | end | 153 | end |
app/helpers/profile_helper.rb
| 1 | module ProfileHelper | 1 | module ProfileHelper |
| 2 | 2 | ||
| 3 | def display_field(title, profile, field, force = false) | 3 | def display_field(title, profile, field, force = false) |
| 4 | - if (!force && field.to_s != 'email' && !profile.active_fields.include?(field.to_s)) || | ||
| 5 | - ((profile.active_fields.include?(field.to_s) || field.to_s == 'email') && !profile.public_fields.include?(field.to_s) && (!user || (user != profile && !user.is_a_friend?(profile)))) | 4 | + if (!force && !profile.active_fields.include?(field.to_s)) || |
| 5 | + (profile.active_fields.include?(field.to_s) && !profile.public_fields.include?(field.to_s) && (!user || (user != profile && !user.is_a_friend?(profile)))) | ||
| 6 | return '' | 6 | return '' |
| 7 | end | 7 | end |
| 8 | value = profile.send(field) | 8 | value = profile.send(field) |
| @@ -28,4 +28,14 @@ module ProfileHelper | @@ -28,4 +28,14 @@ module ProfileHelper | ||
| 28 | end | 28 | end |
| 29 | end | 29 | end |
| 30 | 30 | ||
| 31 | + def display_work_info(profile) | ||
| 32 | + organization = display_field(_('Organization:'), profile, :organization) | ||
| 33 | + organization_site = display_field(_('Organization website:'), profile, :organization_website) { |url| link_to(url, url) } | ||
| 34 | + if organization.blank? && organization_site.blank? | ||
| 35 | + '' | ||
| 36 | + else | ||
| 37 | + content_tag('tr', content_tag('th', _('Work'), { :colspan => 2 })) + organization + organization_site | ||
| 38 | + end | ||
| 39 | + end | ||
| 40 | + | ||
| 31 | end | 41 | end |
app/models/person.rb
| @@ -178,7 +178,8 @@ class Person < Profile | @@ -178,7 +178,8 @@ class Person < Profile | ||
| 178 | include MaybeAddHttp | 178 | include MaybeAddHttp |
| 179 | 179 | ||
| 180 | def active_fields | 180 | def active_fields |
| 181 | - environment ? environment.active_person_fields : [] | 181 | + fields = environment ? environment.active_person_fields : [] |
| 182 | + fields << 'email' | ||
| 182 | end | 183 | end |
| 183 | 184 | ||
| 184 | def required_fields | 185 | def required_fields |
app/views/profile/_person_profile.rhtml
| @@ -16,14 +16,7 @@ | @@ -16,14 +16,7 @@ | ||
| 16 | <%= display_contact profile %> | 16 | <%= display_contact profile %> |
| 17 | 17 | ||
| 18 | <% cache_timeout(profile.relationships_cache_key, 4.hours) do %> | 18 | <% cache_timeout(profile.relationships_cache_key, 4.hours) do %> |
| 19 | - <% if !(profile.organization.blank? && profile.organization_website.blank?) && (profile.active_fields.include?('organization') || profile.active_fields.include?('organization_website')) %> | ||
| 20 | - <tr> | ||
| 21 | - <th colspan='2'><%= _('Work')%></th> | ||
| 22 | - </tr> | ||
| 23 | - <% end %> | ||
| 24 | - <%= display_field(_('Organization:'), profile, :organization) %> | ||
| 25 | - <%= display_field(_('Organization website:'), profile, :organization_website) { |url| link_to(url, url) }%> | ||
| 26 | - | 19 | + <%= display_work_info profile %> |
| 27 | 20 | ||
| 28 | <% if !environment.enabled?('disable_asset_enterprises') && !profile.enterprises.empty? %> | 21 | <% if !environment.enabled?('disable_asset_enterprises') && !profile.enterprises.empty? %> |
| 29 | <tr> | 22 | <tr> |
| @@ -51,6 +44,6 @@ | @@ -51,6 +44,6 @@ | ||
| 51 | 44 | ||
| 52 | <%= render :partial => 'common' %> | 45 | <%= render :partial => 'common' %> |
| 53 | 46 | ||
| 54 | - </table> | ||
| 55 | -<% end %> | 47 | + <% end %> |
| 48 | +</table> | ||
| 56 | 49 |
public/stylesheets/application.css
| @@ -6107,14 +6107,17 @@ li.profile-activity-item.upload_image .activity-gallery-images-count-1 img { | @@ -6107,14 +6107,17 @@ li.profile-activity-item.upload_image .activity-gallery-images-count-1 img { | ||
| 6107 | display: table-row; | 6107 | display: table-row; |
| 6108 | } | 6108 | } |
| 6109 | 6109 | ||
| 6110 | +.field-with-privacy-selector:hover { | ||
| 6111 | + background-color: #F0F0F0; | ||
| 6112 | +} | ||
| 6113 | + | ||
| 6110 | .controller-profile_editor #profile-data .field-with-privacy-selector .formfieldline { | 6114 | .controller-profile_editor #profile-data .field-with-privacy-selector .formfieldline { |
| 6111 | - display: table-cell; | ||
| 6112 | width: auto; | 6115 | width: auto; |
| 6113 | } | 6116 | } |
| 6114 | 6117 | ||
| 6115 | .field-privacy-selector { | 6118 | .field-privacy-selector { |
| 6116 | display: table-cell; | 6119 | display: table-cell; |
| 6117 | - vertical-align: bottom; | 6120 | + vertical-align: middle; |
| 6118 | text-align: center; | 6121 | text-align: center; |
| 6119 | width: 100px; | 6122 | width: 100px; |
| 6120 | } | 6123 | } |
test/unit/application_helper_test.rb
| @@ -379,8 +379,10 @@ class ApplicationHelperTest < ActiveSupport::TestCase | @@ -379,8 +379,10 @@ class ApplicationHelperTest < ActiveSupport::TestCase | ||
| 379 | controller.stubs(:action_name).returns('edit') | 379 | controller.stubs(:action_name).returns('edit') |
| 380 | 380 | ||
| 381 | profile = Person.new | 381 | profile = Person.new |
| 382 | - profile.expects(:active_fields).returns(['field']) | ||
| 383 | - assert_equal 'SIGNUP_FIELD', optional_field(profile, 'field', 'SIGNUP_FIELD') | 382 | + profile.stubs(:active_fields).returns(['field']) |
| 383 | + | ||
| 384 | + expects(:profile_field_privacy_selector).with(profile, 'field').returns('') | ||
| 385 | + assert_tag_in_string optional_field(profile, 'field', 'EDIT_FIELD'), :tag => 'div', :content => 'EDIT_FIELD', :attributes => {:class => 'field-with-privacy-selector'} | ||
| 384 | end | 386 | end |
| 385 | 387 | ||
| 386 | should 'not display active fields' do | 388 | should 'not display active fields' do |
| @@ -394,7 +396,7 @@ class ApplicationHelperTest < ActiveSupport::TestCase | @@ -394,7 +396,7 @@ class ApplicationHelperTest < ActiveSupport::TestCase | ||
| 394 | 396 | ||
| 395 | profile = Person.new | 397 | profile = Person.new |
| 396 | profile.expects(:active_fields).returns([]) | 398 | profile.expects(:active_fields).returns([]) |
| 397 | - assert_equal '', optional_field(profile, 'field', 'SIGNUP_FIELD') | 399 | + assert_equal '', optional_field(profile, 'field', 'EDIT_FIELD') |
| 398 | end | 400 | end |
| 399 | 401 | ||
| 400 | should 'display required fields' do | 402 | should 'display required fields' do |
| @@ -406,11 +408,13 @@ class ApplicationHelperTest < ActiveSupport::TestCase | @@ -406,11 +408,13 @@ class ApplicationHelperTest < ActiveSupport::TestCase | ||
| 406 | controller.stubs(:controller_name).returns('') | 408 | controller.stubs(:controller_name).returns('') |
| 407 | controller.stubs(:action_name).returns('edit') | 409 | controller.stubs(:action_name).returns('edit') |
| 408 | 410 | ||
| 409 | - stubs(:required).with('SIGNUP_FIELD').returns('<span>SIGNUP_FIELD</span>') | ||
| 410 | profile = Person.new | 411 | profile = Person.new |
| 411 | - profile.expects(:active_fields).returns(['field']) | 412 | + profile.stubs(:active_fields).returns(['field']) |
| 412 | profile.expects(:required_fields).returns(['field']) | 413 | profile.expects(:required_fields).returns(['field']) |
| 413 | - assert_equal '<span>SIGNUP_FIELD</span>', optional_field(profile, 'field', 'SIGNUP_FIELD') | 414 | + |
| 415 | + stubs(:required).with(anything).returns('<span>EDIT_FIELD</span>') | ||
| 416 | + expects(:profile_field_privacy_selector).with(profile, 'field').returns('') | ||
| 417 | + assert_equal '<span>EDIT_FIELD</span>', optional_field(profile, 'field', 'EDIT_FIELD') | ||
| 414 | end | 418 | end |
| 415 | 419 | ||
| 416 | should 'base theme uses default icon theme' do | 420 | should 'base theme uses default icon theme' do |
test/unit/person_test.rb
| @@ -441,6 +441,14 @@ class PersonTest < ActiveSupport::TestCase | @@ -441,6 +441,14 @@ class PersonTest < ActiveSupport::TestCase | ||
| 441 | assert_equal e.active_person_fields, person.active_fields | 441 | assert_equal e.active_person_fields, person.active_fields |
| 442 | end | 442 | end |
| 443 | 443 | ||
| 444 | + should 'return email as active_person_fields' do | ||
| 445 | + e = Environment.default | ||
| 446 | + e.expects(:active_person_fields).returns(['nickname']).at_least_once | ||
| 447 | + person = Person.new(:environment => e) | ||
| 448 | + | ||
| 449 | + assert_equal ['nickname', 'email'], person.active_fields | ||
| 450 | + end | ||
| 451 | + | ||
| 444 | should 'return required_person_fields' do | 452 | should 'return required_person_fields' do |
| 445 | e = Environment.default | 453 | e = Environment.default |
| 446 | e.expects(:required_person_fields).returns(['cell_phone', 'comercial_phone']).at_least_once | 454 | e.expects(:required_person_fields).returns(['cell_phone', 'comercial_phone']).at_least_once |
test/unit/profile_helper_test.rb
| @@ -2,6 +2,10 @@ require File.dirname(__FILE__) + '/../test_helper' | @@ -2,6 +2,10 @@ require File.dirname(__FILE__) + '/../test_helper' | ||
| 2 | 2 | ||
| 3 | class ProfileHelperTest < ActiveSupport::TestCase | 3 | class ProfileHelperTest < ActiveSupport::TestCase |
| 4 | 4 | ||
| 5 | + include ProfileHelper | ||
| 6 | + include ApplicationHelper | ||
| 7 | + include ActionView::Helpers::TagHelper | ||
| 8 | + | ||
| 5 | def setup | 9 | def setup |
| 6 | @profile = mock | 10 | @profile = mock |
| 7 | @helper = mock | 11 | @helper = mock |
| @@ -9,8 +13,85 @@ class ProfileHelperTest < ActiveSupport::TestCase | @@ -9,8 +13,85 @@ class ProfileHelperTest < ActiveSupport::TestCase | ||
| 9 | end | 13 | end |
| 10 | attr_reader :profile, :helper | 14 | attr_reader :profile, :helper |
| 11 | 15 | ||
| 12 | - def test_true | ||
| 13 | - assert true | 16 | + should 'not display field if field is not active and not forced' do |
| 17 | + profile.expects(:active_fields).returns([]) | ||
| 18 | + assert_equal '', display_field('Title', profile, 'field') | ||
| 19 | + end | ||
| 20 | + | ||
| 21 | + should 'display field if field is not active but is forced' do | ||
| 22 | + profile.expects(:active_fields).returns([]) | ||
| 23 | + profile.expects(:field).returns('value') | ||
| 24 | + assert_match /Title.*value/, display_field('Title', profile, 'field', true) | ||
| 25 | + end | ||
| 26 | + | ||
| 27 | + should 'not display field if field is active but not public and not logged in' do | ||
| 28 | + profile.stubs(:active_fields).returns(['field']) | ||
| 29 | + profile.expects(:public_fields).returns([]) | ||
| 30 | + @controller = mock | ||
| 31 | + @controller.stubs(:user).returns(nil) | ||
| 32 | + assert_equal '', display_field('Title', profile, 'field') | ||
| 33 | + end | ||
| 34 | + | ||
| 35 | + should 'not display field if field is active but not public and user is not friend' do | ||
| 36 | + profile.stubs(:active_fields).returns(['field']) | ||
| 37 | + profile.expects(:public_fields).returns([]) | ||
| 38 | + user = mock | ||
| 39 | + user.expects(:is_a_friend?).with(profile).returns(false) | ||
| 40 | + @controller = mock | ||
| 41 | + @controller.stubs(:user).returns(user) | ||
| 42 | + assert_equal '', display_field('Title', profile, 'field') | ||
| 43 | + end | ||
| 44 | + | ||
| 45 | + should 'display field if field is active and not public but user is profile owner' do | ||
| 46 | + profile.stubs(:active_fields).returns(['field']) | ||
| 47 | + profile.expects(:public_fields).returns([]) | ||
| 48 | + profile.expects(:field).returns('value') | ||
| 49 | + @controller = mock | ||
| 50 | + @controller.stubs(:user).returns(profile) | ||
| 51 | + assert_match /Title.*value/, display_field('Title', profile, 'field', true) | ||
| 52 | + end | ||
| 53 | + | ||
| 54 | + should 'display field if field is active and not public but user is a friend' do | ||
| 55 | + profile.stubs(:active_fields).returns(['field']) | ||
| 56 | + profile.expects(:public_fields).returns([]) | ||
| 57 | + profile.expects(:field).returns('value') | ||
| 58 | + user = mock | ||
| 59 | + user.expects(:is_a_friend?).with(profile).returns(true) | ||
| 60 | + @controller = mock | ||
| 61 | + @controller.stubs(:user).returns(user) | ||
| 62 | + assert_match /Title.*value/, display_field('Title', profile, 'field', true) | ||
| 63 | + end | ||
| 64 | + | ||
| 65 | + should 'not display work info if field is active but not public and user is not friend' do | ||
| 66 | + profile.stubs(:active_fields).returns(['organization', 'organization_website']) | ||
| 67 | + profile.expects(:public_fields).returns([]).times(2) | ||
| 68 | + user = mock | ||
| 69 | + user.expects(:is_a_friend?).with(profile).returns(false).times(2) | ||
| 70 | + @controller = mock | ||
| 71 | + @controller.stubs(:user).returns(user) | ||
| 72 | + assert_equal '', display_work_info(profile) | ||
| 73 | + end | ||
| 74 | + | ||
| 75 | + should 'display work info if field is active and not public but user is profile owner' do | ||
| 76 | + profile.stubs(:active_fields).returns(['organization', 'organization_website']) | ||
| 77 | + profile.expects(:public_fields).returns([]).times(2) | ||
| 78 | + profile.expects(:organization).returns('Organization Name') | ||
| 79 | + profile.expects(:organization_website).returns('') | ||
| 80 | + @controller = mock | ||
| 81 | + @controller.stubs(:user).returns(profile) | ||
| 82 | + assert_match /Work.*Organization Name/, display_work_info(profile) | ||
| 83 | + end | ||
| 84 | + | ||
| 85 | + should 'display work info if field is active and not public but user is a friend' do | ||
| 86 | + profile.stubs(:active_fields).returns(['organization', 'organization_website']) | ||
| 87 | + profile.expects(:public_fields).returns([]).times(2) | ||
| 88 | + profile.expects(:organization).returns('Organization Name') | ||
| 89 | + profile.expects(:organization_website).returns('') | ||
| 90 | + user = mock | ||
| 91 | + user.expects(:is_a_friend?).with(profile).returns(true).times(2) | ||
| 92 | + @controller = mock | ||
| 93 | + @controller.stubs(:user).returns(user) | ||
| 94 | + assert_match /Work.*Organization Name/, display_work_info(profile) | ||
| 14 | end | 95 | end |
| 15 | 96 | ||
| 16 | end | 97 | end |