From 7a5a0a909aac4dc8a0ab661e3069dfba6bf3b767 Mon Sep 17 00:00:00 2001 From: cmrd Senya Date: Tue, 30 Jun 2015 22:14:10 +0300 Subject: [PATCH] Allow extended profile fields (previously private profile) to be set public (#5684). This adds a new boolean field "public_details" to person model. By default it is false and represents old behaviour. When it is set to true, extended profile (bio,location,gender,birthday) get available to people who didn't log into diaspora and to people you don't share with (i.e. it is made public). In UI, a bootstrap-switch added on the profile-edit page in order to change the setting. This also changes wording from public/private profile to basic/extended. The latter could be public and limited. --- Gemfile | 1 + Gemfile.lock | 2 + app/assets/javascripts/app/pages/settings.js | 1 + app/assets/javascripts/app/router.js | 1 + .../app/views/profile_sidebar_view.js | 12 +---- app/assets/javascripts/main.js | 1 + app/assets/stylesheets/_application.scss | 2 + app/assets/stylesheets/profile.scss | 6 +++ app/controllers/people_controller.rb | 4 +- app/controllers/photos_controller.rb | 2 +- app/controllers/profiles_controller.rb | 5 +- app/models/person.rb | 2 +- app/presenters/contact_presenter.rb | 2 +- app/presenters/person_presenter.rb | 49 +++++++------------ app/views/profiles/_edit_private.haml | 14 +++++- app/views/profiles/_edit_public.haml | 6 ++- config/locales/diaspora/en.yml | 10 +++- .../20150630221004_add_public_to_profiles.rb | 5 ++ db/schema.rb | 3 +- features/desktop/edits_profile.feature | 3 ++ features/step_definitions/web_steps.rb | 9 ++++ .../app/views/profile_sidebar_view_spec.js | 1 + spec/models/profile_spec.rb | 25 +++++----- spec/presenters/person_presenter_spec.rb | 34 +++++++++++-- 24 files changed, 131 insertions(+), 69 deletions(-) create mode 100644 db/migrate/20150630221004_add_public_to_profiles.rb diff --git a/Gemfile b/Gemfile index afd17cfc2..e97ae9f89 100644 --- a/Gemfile +++ b/Gemfile @@ -57,6 +57,7 @@ gem "bootstrap-sass", "3.3.5" gem "compass-rails", "2.0.4" gem "sass-rails", "5.0.1" gem "autoprefixer-rails", "5.2.1" +gem "bootstrap-switch-rails", "3.3.3" # Database diff --git a/Gemfile.lock b/Gemfile.lock index 3a5211391..535d0ccbe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -73,6 +73,7 @@ GEM bootstrap-sass (3.3.5) autoprefixer-rails (>= 5.0.0.1) sass (>= 3.2.19) + bootstrap-switch-rails (3.3.3) buftok (0.2.0) builder (3.2.2) byebug (4.0.5) @@ -751,6 +752,7 @@ DEPENDENCIES autoprefixer-rails (= 5.2.1) backbone-on-rails (= 1.1.2.1) bootstrap-sass (= 3.3.5) + bootstrap-switch-rails (= 3.3.3) capybara (= 2.4.4) carrierwave (= 0.10.0) compass-rails (= 2.0.4) diff --git a/app/assets/javascripts/app/pages/settings.js b/app/assets/javascripts/app/pages/settings.js index eab65b738..d8a2437bb 100644 --- a/app/assets/javascripts/app/pages/settings.js +++ b/app/assets/javascripts/app/pages/settings.js @@ -2,6 +2,7 @@ app.pages.Settings = Backbone.View.extend({ initialize: function() { $(".settings_visibility").tooltip({placement: "top"}); + $("[name='profile[public_details]']").bootstrapSwitch(); } }); // @license-end diff --git a/app/assets/javascripts/app/router.js b/app/assets/javascripts/app/router.js index 0a9958bcf..a63d5d7ff 100644 --- a/app/assets/javascripts/app/router.js +++ b/app/assets/javascripts/app/router.js @@ -9,6 +9,7 @@ app.Router = Backbone.Router.extend({ "conversations": "conversations", "user/edit": "settings", "users/sign_up": "registration", + "profile/edit": "settings", //new hotness "posts/:id": "singlePost", diff --git a/app/assets/javascripts/app/views/profile_sidebar_view.js b/app/assets/javascripts/app/views/profile_sidebar_view.js index a8b0d4074..f19f7970a 100644 --- a/app/assets/javascripts/app/views/profile_sidebar_view.js +++ b/app/assets/javascripts/app/views/profile_sidebar_view.js @@ -1,17 +1,7 @@ // @license magnet:?xt=urn:btih:0b31508aeb0634b347b8270c7bee4d411b5d4109&dn=agpl-3.0.txt AGPL-v3-or-Later app.views.ProfileSidebar = app.views.Base.extend({ - templateName: 'profile_sidebar', - - presenter: function() { - return _.extend({}, this.defaultPresenter(), { - show_profile_info: this._shouldShowProfileInfo(), - }); - }, - - _shouldShowProfileInfo: function() { - return (this.model.isSharing() || this.model.get('is_own_profile')); - } + templateName: "profile_sidebar" }); // @license-end diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 48a07f568..6828a7180 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -46,3 +46,4 @@ //= require mentions //= require bootstrap //= require osmlocator +//= require bootstrap-switch diff --git a/app/assets/stylesheets/_application.scss b/app/assets/stylesheets/_application.scss index 4bc1621b4..13f2ddf54 100644 --- a/app/assets/stylesheets/_application.scss +++ b/app/assets/stylesheets/_application.scss @@ -92,3 +92,5 @@ /* statistics */ @import 'new_styles/statistics'; + +@import "bootstrap3-switch"; diff --git a/app/assets/stylesheets/profile.scss b/app/assets/stylesheets/profile.scss index 7afff703c..364349763 100644 --- a/app/assets/stylesheets/profile.scss +++ b/app/assets/stylesheets/profile.scss @@ -148,3 +148,9 @@ &:last-of-type{ float: right; } } } + +#update_profile_form { + .visibility-hint-icon { + cursor: pointer; + } +} diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index a4693a90a..a79a1df1e 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -75,7 +75,7 @@ class PeopleController < ApplicationController def show mark_corresponding_notifications_read if user_signed_in? - @person_json = PersonPresenter.new(@person, current_user).full_hash_with_profile + @person_json = PersonPresenter.new(@person, current_user).as_json respond_to do |format| format.all do @@ -144,7 +144,7 @@ class PeopleController < ApplicationController if @person @contact = current_user.contact_for(@person) @contacts_of_contact = Contact.contact_contacts_for(current_user, @person) - gon.preloads[:person] = PersonPresenter.new(@person, current_user).full_hash_with_profile + gon.preloads[:person] = PersonPresenter.new(@person, current_user).as_json gon.preloads[:photos] = { count: photos_from(@person, :all).count(:all) } diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 7c543c140..95fa714c1 100644 --- a/app/controllers/photos_controller.rb +++ b/app/controllers/photos_controller.rb @@ -25,7 +25,7 @@ class PhotosController < ApplicationController @posts = current_user.photos_from(@person, max_time: max_time).order('created_at desc') respond_to do |format| format.all do - gon.preloads[:person] = PersonPresenter.new(@person, current_user).full_hash_with_profile + gon.preloads[:person] = PersonPresenter.new(@person, current_user).as_json gon.preloads[:photos] = { count: current_user.photos_from(@person, limit: :all).count(:all) } diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 2c2bdf2c2..409dcaee0 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -40,6 +40,7 @@ class ProfilesController < ApplicationController #checkbox tags wtf @profile_attrs[:searchable] ||= false @profile_attrs[:nsfw] ||= false + @profile_attrs[:public_details] ||= false if params[:photo_id] @profile_attrs[:photo] = Photo.where(:author_id => current_user.person_id, :id => params[:photo_id]).first @@ -79,6 +80,8 @@ class ProfilesController < ApplicationController end def profile_params - params.require(:profile).permit(:first_name, :last_name, :gender, :bio, :location, :searchable, :tag_string, :nsfw, :date => [:year, :month, :day]) || {} + params.require(:profile).permit(:first_name, :last_name, :gender, :bio, + :location, :searchable, :tag_string, :nsfw, + :public_details, date: %i(year month day)) || {} end end diff --git a/app/models/person.rb b/app/models/person.rb index fc966c4eb..adacfe02d 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -31,7 +31,7 @@ class Person < ActiveRecord::Base has_one :profile, :dependent => :destroy delegate :last_name, :image_url, :tag_string, :bio, :location, :gender, :birthday, :formatted_birthday, :tags, :searchable, - to: :profile + :public_details?, to: :profile accepts_nested_attributes_for :profile before_validation :downcase_diaspora_handle diff --git a/app/presenters/contact_presenter.rb b/app/presenters/contact_presenter.rb index 6c3463a9d..91a6252b4 100644 --- a/app/presenters/contact_presenter.rb +++ b/app/presenters/contact_presenter.rb @@ -12,6 +12,6 @@ class ContactPresenter < BasePresenter end def full_hash_with_person - full_hash.merge(person: PersonPresenter.new(person, current_user).full_hash_with_profile) + full_hash.merge(person: PersonPresenter.new(person, current_user).as_json) end end diff --git a/app/presenters/person_presenter.rb b/app/presenters/person_presenter.rb index f53087eee..fc8100e52 100644 --- a/app/presenters/person_presenter.rb +++ b/app/presenters/person_presenter.rb @@ -10,41 +10,16 @@ class PersonPresenter < BasePresenter def full_hash base_hash.merge( - relationship: relationship, - block: is_blocked? ? BlockPresenter.new(current_user_person_block).base_hash : false, - contact: (!own_profile? && has_contact?) ? {id: current_user_person_contact.id} : false, - is_own_profile: own_profile? + relationship: relationship, + block: is_blocked? ? BlockPresenter.new(current_user_person_block).base_hash : false, + contact: (!own_profile? && has_contact?) ? {id: current_user_person_contact.id} : false, + is_own_profile: own_profile?, + show_profile_info: public_details? || own_profile? || person_is_following_current_user ) end - def full_hash_with_avatar - full_hash.merge(avatar: AvatarPresenter.new(profile).base_hash) - end - - def full_hash_with_profile - attrs = full_hash - - if own_profile? || person_is_following_current_user - attrs.merge!(profile: ProfilePresenter.new(profile).private_hash) - else - attrs.merge!(profile: ProfilePresenter.new(profile).public_hash) - end - - attrs - end - def as_json(_options={}) - attrs = full_hash_with_avatar - - if own_profile? || person_is_following_current_user - attrs.merge!( - location: @presentable.location, - birthday: @presentable.formatted_birthday, - bio: @presentable.bio - ) - end - - attrs + full_hash_with_profile end protected @@ -69,6 +44,18 @@ class PersonPresenter < BasePresenter @presentable.shares_with(current_user) end + def full_hash_with_profile + attrs = full_hash + + if attrs[:show_profile_info] + attrs.merge!(profile: ProfilePresenter.new(profile).private_hash) + else + attrs.merge!(profile: ProfilePresenter.new(profile).public_hash) + end + + attrs + end + private def current_user_person_block diff --git a/app/views/profiles/_edit_private.haml b/app/views/profiles/_edit_private.haml index 18cd45505..44667cb04 100644 --- a/app/views/profiles/_edit_private.haml +++ b/app/views/profiles/_edit_private.haml @@ -3,7 +3,13 @@ -# the COPYRIGHT file. %hr -%h3= t('profiles.edit.your_private_profile') +%h3.inline + = t("profiles.edit.extended") += t("profiles.edit.extended_visibility_text") += check_box_tag "profile[public_details]", true, profile.public_details, {"data-size" => "mini", "data-on-text" => t("profiles.edit.public"), "data-off-text" => t("profiles.edit.limited")} +%span{ :title => t("profiles.edit.extended_hint") } + %i.entypo.circled-help.visibility-hint-icon +.small-horizontal-spacer %h4= t('profiles.edit.your_bio') @@ -30,6 +36,12 @@ .small-horizontal-spacer +%hr +%h3.inline + = t('profiles.edit.settings') + +.small-horizontal-spacer + %h4= t('search') .well.checkbox diff --git a/app/views/profiles/_edit_public.haml b/app/views/profiles/_edit_public.haml index 85cf916b4..ac45aff8c 100644 --- a/app/views/profiles/_edit_public.haml +++ b/app/views/profiles/_edit_public.haml @@ -38,7 +38,11 @@ .stream %p{:class => "conversation_#{name}"}= msg -%h3= t('profiles.edit.your_public_profile') +%h3.inline + = t("profiles.edit.basic") + %span{ :title => t("profiles.edit.basic_hint") } + %i.entypo.circled-help.visibility-hint-icon +.small-horizontal-spacer = error_messages_for profile diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 0a814cb26..8596cb3bb 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -1008,8 +1008,14 @@ en: profiles: edit: - your_public_profile: "Your public profile" - your_private_profile: "Your private profile" + basic: "My basic profile" + extended: "My extended profile" + settings: "Profile settings" + extended_visibility_text: "Visibility of your extended profile:" + public: "Public" + limited: "Limited" + basic_hint: "Every item in your profile is optional. Your basic profile will always be publicly visible." + extended_hint: "Click the switch to set your extended profile data visibility. Public means it is visible to the internet, limited means only people who you share with will see this information." your_name: "Your name" first_name: "First name" last_name: "Last name" diff --git a/db/migrate/20150630221004_add_public_to_profiles.rb b/db/migrate/20150630221004_add_public_to_profiles.rb new file mode 100644 index 000000000..06973e027 --- /dev/null +++ b/db/migrate/20150630221004_add_public_to_profiles.rb @@ -0,0 +1,5 @@ +class AddPublicToProfiles < ActiveRecord::Migration + def change + add_column :profiles, :public_details, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index fd77691b3..2a4e92591 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150607143809) do +ActiveRecord::Schema.define(version: 20150630221004) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 @@ -409,6 +409,7 @@ ActiveRecord::Schema.define(version: 20150607143809) do t.string "location", limit: 255 t.string "full_name", limit: 70 t.boolean "nsfw", default: false + t.boolean "public_details", default: false end add_index "profiles", ["full_name", "searchable"], name: "index_profiles_on_full_name_and_searchable", using: :btree diff --git a/features/desktop/edits_profile.feature b/features/desktop/edits_profile.feature index 3db0978e9..6bb6e8213 100644 --- a/features/desktop/edits_profile.feature +++ b/features/desktop/edits_profile.feature @@ -31,13 +31,16 @@ Feature: editing your profile And the "profile_date_day" field should be filled with "30" And the "profile_location" field should be filled with "Kamino" And I should see "#starwars" within "ul#as-selections-tags" + And the "#profile_public_details" bootstrap-switch should be off When I fill in "profile[tag_string]" with "#kamino" And I press the first ".as-result-item" within ".as-results" + And I toggle the "#profile_public_details" bootstrap-switch And I press "update_profile" Then I should see "#kamino" within "ul#as-selections-tags" And I should see "#starwars" within "ul#as-selections-tags" + And the "#profile_public_details" bootstrap-switch should be on When I attach the file "spec/fixtures/bad_urls.txt" to "file" within "#file-upload" And I confirm the alert diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb index a9ed77abf..e7e0ee0e0 100644 --- a/features/step_definitions/web_steps.rb +++ b/features/step_definitions/web_steps.rb @@ -168,6 +168,15 @@ Then /^the "([^"]*)" checkbox(?: within "([^"]*)")? should not be checked$/ do | end end +Then /^the "([^"]*)" bootstrap-switch should be (on|off)$/ do |label, state| + result = execute_script("return $('#{label}').bootstrapSwitch('state')") + result.should state == "on" ? be_truthy : be_falsey +end + +Then /^I toggle the "([^"]*)" bootstrap-switch$/ do |label| + execute_script("return $('#{label}').bootstrapSwitch('toggleState')") +end + Then /^(?:|I )should be on (.+)$/ do |page_name| confirm_on_page(page_name) end diff --git a/spec/javascripts/app/views/profile_sidebar_view_spec.js b/spec/javascripts/app/views/profile_sidebar_view_spec.js index f648bb118..e54ac9062 100644 --- a/spec/javascripts/app/views/profile_sidebar_view_spec.js +++ b/spec/javascripts/app/views/profile_sidebar_view_spec.js @@ -5,6 +5,7 @@ describe("app.views.ProfileSidebar", function() { diaspora_id: "alice@umbrella.corp", name: "Project Alice", relationship: 'mutual', + show_profile_info: true, profile: { bio: "confidential", location: "underground", diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index d1087a6e8..bce20ce64 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -356,18 +356,19 @@ describe Profile, :type => :model do profile = FactoryGirl.build :profile expect(profile.send(:clearable_fields).sort).to eq( ["diaspora_handle", - "first_name", - "last_name", - "image_url", - "image_url_small", - "image_url_medium", - "birthday", - "gender", - "bio", - "searchable", - "nsfw", - "location", - "full_name"].sort + "first_name", + "last_name", + "image_url", + "image_url_small", + "image_url_medium", + "birthday", + "gender", + "bio", + "searchable", + "nsfw", + "location", + "public_details", + "full_name"].sort ) end end diff --git a/spec/presenters/person_presenter_spec.rb b/spec/presenters/person_presenter_spec.rb index 54424e2ee..1873d80cb 100644 --- a/spec/presenters/person_presenter_spec.rb +++ b/spec/presenters/person_presenter_spec.rb @@ -6,26 +6,52 @@ describe PersonPresenter do describe "#as_json" do context "with no current_user" do - it "returns the user's public information if a user is not logged in" do + it "returns the user's basic profile" do expect(PersonPresenter.new(person, nil).as_json).to include(person.as_api_response(:backbone).except(:avatar)) end + + it "returns the user's additional profile if the user has set additional profile public" do + person.profile.public_details = true + expect(PersonPresenter.new(person, nil).as_json[:profile]).to include(*%i(location bio gender birthday)) + end + + it "doesn't return user's additional profile if the user hasn't set additional profile public" do + person.profile.public_details = false + expect(PersonPresenter.new(person, nil).as_json[:profile]).not_to include(*%i(location bio gender birthday)) + end end context "with a current_user" do let(:current_user) { FactoryGirl.create(:user)} let(:presenter){ PersonPresenter.new(person, current_user) } + # here private information == addtional user profile, because additional profile by default is private it "doesn't share private information when the users aren't connected" do - expect(presenter.full_hash_with_profile[:profile]).not_to have_key(:location) + expect(person.profile.public_details).to be_falsey + expect(presenter.as_json[:show_profile_info]).to be_falsey + expect(presenter.as_json[:profile]).not_to have_key(:location) + end + + it "shares private information when the users aren't connected, but profile is public" do + person.profile.public_details = true + expect(presenter.as_json[:show_profile_info]).to be_truthy + expect(presenter.as_json[:relationship]).to be(:not_sharing) + expect(presenter.as_json[:profile]).to have_key(:location) end it "has private information when the person is sharing with the current user" do expect(person).to receive(:shares_with).with(current_user).and_return(true) - expect(presenter.full_hash_with_profile[:profile]).to have_key(:location) + expect(person.profile.public_details).to be_falsey + pr_json = presenter.as_json + expect(pr_json[:show_profile_info]).to be_truthy + expect(pr_json[:profile]).to have_key(:location) end it "returns the user's private information if a user is logged in as herself" do - expect(PersonPresenter.new(current_user.person, current_user).as_json).to have_key(:location) + current_person_presenter = PersonPresenter.new(current_user.person, current_user) + expect(current_user.person.profile.public_details).to be_falsey + expect(current_person_presenter.as_json[:show_profile_info]).to be_truthy + expect(current_person_presenter.as_json[:profile]).to have_key(:location) end end end