From 4fa92c1823a0268dabf58c83c6e8438672e7a93c Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 5 Apr 2018 03:38:08 +0200 Subject: [PATCH] Remove flag for contacts visible to each other This feature only worked on the same pod and was more confusing than useful. --- app/assets/javascripts/app/pages/contacts.js | 21 ------- .../app/views/aspect_create_view.js | 7 +-- .../templates/aspect_create_modal_tpl.jst.hbs | 10 ---- app/assets/templates/help_tpl.jst.hbs | 2 +- app/controllers/aspects_controller.rb | 14 +---- app/models/contact.rb | 11 ---- app/serializers/export/aspect_serializer.rb | 4 +- app/views/contacts/_header.html.haml | 6 -- config/locales/diaspora/en.yml | 2 - config/locales/javascript/javascript.en.yml | 3 - config/routes.rb | 1 - ...21_remove_contacts_visible_from_aspects.rb | 8 +++ features/desktop/contacts.feature | 25 --------- lib/schemas/archive-format.json | 1 - spec/controllers/aspects_controller_spec.rb | 18 ------ spec/integration/exporter_spec.rb | 2 - spec/javascripts/app/pages/contacts_spec.js | 29 ---------- .../app/views/aspect_create_view_spec.js | 32 ----------- spec/models/aspect_spec.rb | 4 -- spec/models/contact_spec.rb | 56 ------------------- .../export/aspect_serializer_spec.rb | 5 +- spec/support/data_generator.rb | 2 +- 22 files changed, 15 insertions(+), 248 deletions(-) create mode 100644 db/migrate/20180406235521_remove_contacts_visible_from_aspects.rb diff --git a/app/assets/javascripts/app/pages/contacts.js b/app/assets/javascripts/app/pages/contacts.js index 091601516..0035951c0 100644 --- a/app/assets/javascripts/app/pages/contacts.js +++ b/app/assets/javascripts/app/pages/contacts.js @@ -5,7 +5,6 @@ app.pages.Contacts = Backbone.View.extend({ el: "#contacts_container", events: { - "click #contacts_visibility_toggle" : "toggleContactVisibility", "click #chat_privilege_toggle" : "toggleChatPrivilege", "click #change_aspect_name" : "showAspectNameForm", "click .conversation_button": "showMessageModal", @@ -13,7 +12,6 @@ app.pages.Contacts = Backbone.View.extend({ }, initialize: function(opts) { - this.visibilityToggle = $("#contacts_visibility_toggle i"); this.chatToggle = $("#chat_privilege_toggle i"); this.stream = opts.stream; this.stream.render(); @@ -45,25 +43,6 @@ app.pages.Contacts = Backbone.View.extend({ } }, - toggleContactVisibility: function() { - if (this.visibilityToggle.hasClass("entypo-lock-open")) { - this.visibilityToggle.removeClass("entypo-lock-open") - .addClass("entypo-lock") - .tooltip("destroy") - .removeAttr("data-original-title") - .attr("title", Diaspora.I18n.t("contacts.aspect_list_is_not_visible")) - .tooltip({"placement": "bottom"}); - } - else { - this.visibilityToggle.removeClass("entypo-lock") - .addClass("entypo-lock-open") - .tooltip("destroy") - .removeAttr("data-original-title") - .attr("title", Diaspora.I18n.t("contacts.aspect_list_is_visible")) - .tooltip({"placement": "bottom"}); - } - }, - showAspectNameForm: function() { $(".header > h3").hide(); var aspectName = $.trim($(".header h3 #aspect_name").text()); diff --git a/app/assets/javascripts/app/views/aspect_create_view.js b/app/assets/javascripts/app/views/aspect_create_view.js index 8c7d030f9..c6c1e592b 100644 --- a/app/assets/javascripts/app/views/aspect_create_view.js +++ b/app/assets/javascripts/app/views/aspect_create_view.js @@ -21,10 +21,6 @@ app.views.AspectCreate = app.views.Base.extend({ }); }, - _contactsVisible: function() { - return this.$("#aspect_contacts_visible").is(":checked"); - }, - _name: function() { return this.$("#aspect_name").val(); }, @@ -71,8 +67,7 @@ app.views.AspectCreate = app.views.Base.extend({ app.aspects.create({ "person_id": this._personId || null, - "name": this._name(), - "contacts_visible": this._contactsVisible() + "name": this._name() }); }, diff --git a/app/assets/templates/aspect_create_modal_tpl.jst.hbs b/app/assets/templates/aspect_create_modal_tpl.jst.hbs index 00cd08d22..721f6f9f2 100644 --- a/app/assets/templates/aspect_create_modal_tpl.jst.hbs +++ b/app/assets/templates/aspect_create_modal_tpl.jst.hbs @@ -22,16 +22,6 @@ - -
-
-
- -
-
-
diff --git a/app/assets/templates/help_tpl.jst.hbs b/app/assets/templates/help_tpl.jst.hbs index b1cd4408e..16dc93075 100644 --- a/app/assets/templates/help_tpl.jst.hbs +++ b/app/assets/templates/help_tpl.jst.hbs @@ -17,7 +17,7 @@ {{ title_account_and_data_management }}
  • - {{ title_aspects }} + {{ title_aspects }} {{ title_aspects }}
  • diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index 4e0344cc3..b970f7596 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -84,18 +84,6 @@ class AspectsController < ApplicationController head :no_content end - def toggle_contact_visibility - @aspect = current_user.aspects.where(:id => params[:aspect_id]).first - - if @aspect.contacts_visible? - @aspect.contacts_visible = false - else - @aspect.contacts_visible = true - end - @aspect.save - head :no_content - end - private def connect_person_to_aspect(aspecting_person_id) @@ -109,6 +97,6 @@ class AspectsController < ApplicationController end def aspect_params - params.require(:aspect).permit(:name, :contacts_visible, :chat_enabled, :order_id) + params.require(:aspect).permit(:name, :chat_enabled, :order_id) end end diff --git a/app/models/contact.rb b/app/models/contact.rb index 2f3dd4e73..6a7a5ce30 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -47,17 +47,6 @@ class Contact < ApplicationRecord ).destroy_all end - def contacts - people = Person.arel_table - incoming_aspects = Aspect.where( - :user_id => self.person.owner_id, - :contacts_visible => true).joins(:contacts).where( - :contacts => {:person_id => self.user.person_id}).select('aspects.id') - incoming_aspect_ids = incoming_aspects.map{|a| a.id} - similar_contacts = Person.joins(:contacts => :aspect_memberships).where( - :aspect_memberships => {:aspect_id => incoming_aspect_ids}).where(people[:id].not_eq(self.user.person.id)).select('DISTINCT people.*') - end - def mutual? sharing && receiving end diff --git a/app/serializers/export/aspect_serializer.rb b/app/serializers/export/aspect_serializer.rb index ceeef754c..938939ad9 100644 --- a/app/serializers/export/aspect_serializer.rb +++ b/app/serializers/export/aspect_serializer.rb @@ -2,8 +2,6 @@ module Export class AspectSerializer < ActiveModel::Serializer - attributes :name, - :contacts_visible, - :chat_enabled + attributes :name, :chat_enabled end end diff --git a/app/views/contacts/_header.html.haml b/app/views/contacts/_header.html.haml index d8d323fb1..4348e0dea 100644 --- a/app/views/contacts/_header.html.haml +++ b/app/views/contacts/_header.html.haml @@ -4,12 +4,6 @@ - if @aspect.contacts.size > 0 && @aspect.contacts.size < 20 = start_a_conversation_link(@aspect, @aspect.contacts.size) - = link_to aspect_toggle_contact_visibility_path(@aspect), id: "contacts_visibility_toggle", class: "contacts_button", method: :put, remote: true do - -if @aspect.contacts_visible? - %i.entypo-lock-open.contacts-header-icon{title: t("aspects.edit.aspect_list_is_visible")} - -else - %i.entypo-lock.contacts-header-icon{title: t("aspects.edit.aspect_list_is_not_visible")} - -if AppConfig.chat.enabled? = link_to aspect_toggle_chat_privilege_path(@aspect), id: "chat_privilege_toggle", class: "contacts_button", method: :put, remote: true do -if @aspect.chat_enabled? diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index d9e887a91..f28b8f551 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -369,8 +369,6 @@ en: contacts_know_aspect_a: "No. They cannot see the name of the aspect under any circumstances." person_multiple_aspects_q: "Can I add a person to multiple aspects?" person_multiple_aspects_a: "Yes. Go to your contacts page and click on “My contacts”. For each contact you can use the menu on the right to add them to (or remove them from) as many aspects as you want. Or you can add them to a new aspect (or remove them from an aspect) by clicking the aspect selector button on their profile page. Or you can even just move the pointer over their name where you see it in the stream, and a “hovercard” will appear. You can change the aspects they are in right there." - contacts_visible_q: "What does “make contacts in this aspect visible to each other” mean?" - contacts_visible_a: "If you check this option then contacts from that aspect will be able to see who else is in it, in the “Contacts” tab on your profile page. (At the moment, only your contacts who are on the same pod as you will be able to see the “Contacts” tab on your profile.) It’s best to select this option only if the contacts in that aspect all know each other, for example if the aspect is for a club or society you belong to. They still won’t be able to see what the aspect is called." remove_notification_q: "If I remove someone from an aspect, or all of my aspects, are they notified of this?" remove_notification_a: "No. They are also not notified if you add them to more aspects, when you are already sharing with them." change_aspect_of_post_q: "Once I have posted something, can I change the aspect(s) that can see it?" diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index e89cf6109..63e8dee68 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -81,7 +81,6 @@ en: other: "The connection test returned an error for <%= count %> pods." aspects: - make_aspect_list_visible: "Make contacts in this aspect visible to each other?" name: "Name" create: add_a_new_aspect: "Add a new aspect" @@ -124,8 +123,6 @@ en: add_contact: "Add contact" aspect_chat_is_enabled: "Contacts in this aspect are able to chat with you." aspect_chat_is_not_enabled: "Contacts in this aspect are not able to chat with you." - aspect_list_is_visible: "Contacts in this aspect are able to see each other." - aspect_list_is_not_visible: "Contacts in this aspect are not able to see each other." remove_contact: "Remove contact" error_add: "Couldn’t add <%= name %> to the aspect :(" error_remove: "Couldn’t remove <%= name %> from the aspect :(" diff --git a/config/routes.rb b/config/routes.rb index 28ca010f6..ab8b75ffe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -59,7 +59,6 @@ Rails.application.routes.draw do get "aspects" => "streams#aspects", :as => "aspects_stream" resources :aspects, except: %i(index new edit) do - put :toggle_contact_visibility put :toggle_chat_privilege collection do put "order" => :update_order diff --git a/db/migrate/20180406235521_remove_contacts_visible_from_aspects.rb b/db/migrate/20180406235521_remove_contacts_visible_from_aspects.rb new file mode 100644 index 000000000..5b201ccdf --- /dev/null +++ b/db/migrate/20180406235521_remove_contacts_visible_from_aspects.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class RemoveContactsVisibleFromAspects < ActiveRecord::Migration[5.1] + def change + remove_index :aspects, column: %i[user_id contacts_visible], name: :index_aspects_on_user_id_and_contacts_visible + remove_column :aspects, :contacts_visible, :boolean, default: true, null: false + end +end diff --git a/features/desktop/contacts.feature b/features/desktop/contacts.feature index 1beecbc86..89ef019a3 100644 --- a/features/desktop/contacts.feature +++ b/features/desktop/contacts.feature @@ -15,28 +15,3 @@ Feature: show contacts When I am on "robert@grimm.grimm"'s page And I press the first "#contacts_link" Then I should be on the contacts page - - Scenario: see contacts of a visible aspect list - When I am on "bob@bob.bob"'s page - And I add the person to my "Unicorns" aspect - And I sign out - And I sign in as "alice@alice.alice" - And I am on "robert@grimm.grimm"'s page - Then I should see "Contacts" within "#profile-horizontal-bar" - - When I press the first "#contacts_link" - Then I should see "Bob Jones" within "#people-stream .media-body" - When I add the person to my "Besties" aspect within "#people-stream" - Then I should see a flash message containing "You have started sharing with Bob Jones!" - - Scenario: don't see contacts of an invisible aspect list - When I am on "bob@bob.bob"'s page - And I add the person to my "Unicorns" aspect - And I am on the contacts page - And I follow "Unicorns" - And I press the first "a#contacts_visibility_toggle" - And I sign out - - And I sign in as "alice@alice.alice" - And I am on "robert@grimm.grimm"'s page - Then I should not see "Contacts" within "#profile-horizontal-bar" diff --git a/lib/schemas/archive-format.json b/lib/schemas/archive-format.json index 314331506..cc4c99ffe 100644 --- a/lib/schemas/archive-format.json +++ b/lib/schemas/archive-format.json @@ -31,7 +31,6 @@ "type": "object", "properties": { "name": { "type": "string" }, - "contacts_visible": { "type": "boolean" }, "chat_enabled": { "type": "boolean" } }, "required": [ diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index d763f4a83..a243cfaf8 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -156,22 +156,4 @@ describe AspectsController, :type => :controller do end end end - - describe "#toggle_contact_visibility" do - it 'sets contacts visible' do - @alices_aspect_1.contacts_visible = false - @alices_aspect_1.save - - get :toggle_contact_visibility, xhr: true, params: {aspect_id: @alices_aspect_1.id} - expect(@alices_aspect_1.reload.contacts_visible).to be true - end - - it 'sets contacts hidden' do - @alices_aspect_1.contacts_visible = true - @alices_aspect_1.save - - get :toggle_contact_visibility, xhr: true, params: {aspect_id: @alices_aspect_1.id} - expect(@alices_aspect_1.reload.contacts_visible).to be false - end - end end diff --git a/spec/integration/exporter_spec.rb b/spec/integration/exporter_spec.rb index 908a92532..ac9bdbfd9 100644 --- a/spec/integration/exporter_spec.rb +++ b/spec/integration/exporter_spec.rb @@ -47,12 +47,10 @@ describe Diaspora::Exporter do "contact_groups": [ { "name": "generic", - "contacts_visible": true, "chat_enabled": false }, { "name": "Work", - "contacts_visible": false, "chat_enabled": false } ] diff --git a/spec/javascripts/app/pages/contacts_spec.js b/spec/javascripts/app/pages/contacts_spec.js index 485b17dc1..c9d972ec6 100644 --- a/spec/javascripts/app/pages/contacts_spec.js +++ b/spec/javascripts/app/pages/contacts_spec.js @@ -34,35 +34,6 @@ describe("app.pages.Contacts", function(){ }); }); - context('toggle contacts visibility', function() { - beforeEach(function() { - this.visibilityToggle = $("#contacts_visibility_toggle"); - this.lockIcon = $("#contacts_visibility_toggle i"); - }); - - it("updates the title for the tooltip", function() { - expect(this.lockIcon.attr("data-original-title")).toBe( - Diaspora.I18n.t("contacts.aspect_list_is_visible") - ); - - this.visibilityToggle.trigger("click"); - - expect(this.lockIcon.attr("data-original-title")).toBe( - Diaspora.I18n.t("contacts.aspect_list_is_not_visible") - ); - }); - - it("toggles the lock icon", function() { - expect(this.lockIcon.hasClass("entypo-lock-open")).toBeTruthy(); - expect(this.lockIcon.hasClass("entypo-lock")).toBeFalsy(); - - this.visibilityToggle.trigger("click"); - - expect(this.lockIcon.hasClass("entypo-lock")).toBeTruthy(); - expect(this.lockIcon.hasClass("entypo-lock-open")).toBeFalsy(); - }); - }); - context('show aspect name form', function() { beforeEach(function() { this.button = $('#change_aspect_name'); diff --git a/spec/javascripts/app/views/aspect_create_view_spec.js b/spec/javascripts/app/views/aspect_create_view_spec.js index bd1a38c4b..bac0870ed 100644 --- a/spec/javascripts/app/views/aspect_create_view_spec.js +++ b/spec/javascripts/app/views/aspect_create_view_spec.js @@ -17,7 +17,6 @@ describe("app.views.AspectCreate", function() { expect(this.view.$("#newAspectModal.modal").length).toBe(1); expect(this.view.$("#newAspectModal form").length).toBe(1); expect(this.view.$("#newAspectModal input#aspect_name").length).toBe(1); - expect(this.view.$("#newAspectModal input#aspect_contacts_visible").length).toBe(1); expect(this.view.$("#newAspectModal .btn-primary").length).toBe(1); }); @@ -61,21 +60,6 @@ describe("app.views.AspectCreate", function() { expect(obj.name).toBe(name); }); - it("should send the correct contacts_visible to the server", function() { - this.view.createAspect(); - var obj = JSON.parse(jasmine.Ajax.requests.mostRecent().params); - /* jshint camelcase: false */ - expect(obj.contacts_visible).toBeFalsy(); - /* jshint camelcase: true */ - - this.view.$("input#aspect_contacts_visible").prop("checked", true); - this.view.createAspect(); - obj = JSON.parse(jasmine.Ajax.requests.mostRecent().params); - /* jshint camelcase: false */ - expect(obj.contacts_visible).toBeTruthy(); - /* jshint camelcase: true */ - }); - it("should send person_id = null to the server", function() { this.view.createAspect(); var obj = JSON.parse(jasmine.Ajax.requests.mostRecent().params); @@ -150,7 +134,6 @@ describe("app.views.AspectCreate", function() { expect(this.view.$("#newAspectModal.modal").length).toBe(1); expect(this.view.$("#newAspectModal form").length).toBe(1); expect(this.view.$("#newAspectModal input#aspect_name").length).toBe(1); - expect(this.view.$("#newAspectModal input#aspect_contacts_visible").length).toBe(1); expect(this.view.$("#newAspectModal .btn-primary").length).toBe(1); }); @@ -174,21 +157,6 @@ describe("app.views.AspectCreate", function() { expect(obj.name).toBe(name); }); - it("should send the correct contacts_visible to the server", function() { - this.view.createAspect(); - var obj = JSON.parse(jasmine.Ajax.requests.mostRecent().params); - /* jshint camelcase: false */ - expect(obj.contacts_visible).toBeFalsy(); - /* jshint camelcase: true */ - - this.view.$("input#aspect_contacts_visible").prop("checked", true); - this.view.createAspect(); - obj = JSON.parse(jasmine.Ajax.requests.mostRecent().params); - /* jshint camelcase: false */ - expect(obj.contacts_visible).toBeTruthy(); - /* jshint camelcase: true */ - }); - it("should send the correct person_id to the server", function() { this.view.createAspect(); var obj = JSON.parse(jasmine.Ajax.requests.mostRecent().params); diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index c5426bf7c..b65816ab2 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -30,10 +30,6 @@ describe Aspect, :type => :model do expect(aspect.contacts.size).to eq(1) end - it "has a contacts_visible? method" do - expect(alice.aspects.first.contacts_visible?).to be true - end - it "sets an order_id" do aspect_2 = alice.aspects.create(name: "People") expect(aspect_2.order_id).to eq(2) diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index dd5f5b27c..dc5be39b8 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -160,62 +160,6 @@ describe Contact, type: :model do end end - describe "#contacts" do - before do - bob.aspects.create(name: "next") - - @original_aspect = bob.aspects.where(name: "generic").first - @new_aspect = bob.aspects.where(name: "next").first - - @people1 = [] - @people2 = [] - - 1.upto(5) do - person = FactoryGirl.build(:person) - bob.contacts.create(person: person, aspects: [@original_aspect]) - @people1 << person - end - 1.upto(5) do - person = FactoryGirl.build(:person) - bob.contacts.create(person: person, aspects: [@new_aspect]) - @people2 << person - end - # eve <-> bob <-> alice - end - - context "on a contact for a local user" do - before do - alice.reload - alice.aspects.reload - @contact = alice.contact_for(bob.person) - end - - it "returns the target local user's contacts that are in the same aspect" do - expect(@contact.contacts.map(&:id)).to match_array([eve.person].concat(@people1).map(&:id)) - end - - it "returns nothing if contacts_visible is false in that aspect" do - @original_aspect.contacts_visible = false - @original_aspect.save - expect(@contact.contacts).to eq([]) - end - - it "returns no duplicate contacts" do - [alice, eve].each {|c| bob.add_contact_to_aspect(bob.contact_for(c.person), bob.aspects.last) } - contact_ids = @contact.contacts.map(&:id) - expect(contact_ids.uniq).to eq(contact_ids) - end - end - - context "on a contact for a remote user" do - let(:contact) { bob.contact_for @people1.first } - - it "returns an empty array" do - expect(contact.contacts).to eq([]) - end - end - end - describe "#receive" do it "shares back if auto_following is enabled" do alice.auto_follow_back = true diff --git a/spec/serializers/export/aspect_serializer_spec.rb b/spec/serializers/export/aspect_serializer_spec.rb index bd13b7b2b..eba297f19 100644 --- a/spec/serializers/export/aspect_serializer_spec.rb +++ b/spec/serializers/export/aspect_serializer_spec.rb @@ -6,9 +6,8 @@ describe Export::AspectSerializer do it "has aspect attributes" do expect(serializer.attributes).to eq( - name: aspect.name, - contacts_visible: aspect.contacts_visible, - chat_enabled: aspect.chat_enabled + name: aspect.name, + chat_enabled: aspect.chat_enabled ) end end diff --git a/spec/support/data_generator.rb b/spec/support/data_generator.rb index a2849a6f9..16f7d877b 100644 --- a/spec/support/data_generator.rb +++ b/spec/support/data_generator.rb @@ -170,7 +170,7 @@ class DataGenerator end def work_aspect - user.aspects.create(name: "Work", contacts_visible: false) + user.aspects.create(name: "Work") end def status_messages_flavours