From f23966ef876f923b244bf0fe40bd3e1c0ba058b2 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 20 Jan 2018 02:52:27 +0100 Subject: [PATCH 1/8] Make mobile bookmarklet work the same way as with desktop UI This fixes that it fails when you call it without notes parameter. closes #7698 --- Changelog.md | 1 + app/assets/javascripts/mobile/bookmarklet.js | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index 1dd215e90..1b5f7cce3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ ## Bug fixes * Fix invite link on the contacts page when the user has no contacts [#7690](https://github.com/diaspora/diaspora/pull/7690) +* Fixed the mobile bookmarklet when called without parameters [#7698](https://github.com/diaspora/diaspora/pull/7698) ## Features * Check if redis is running in script/server [#7685](https://github.com/diaspora/diaspora/pull/7685) diff --git a/app/assets/javascripts/mobile/bookmarklet.js b/app/assets/javascripts/mobile/bookmarklet.js index 94822c2ef..f8a3ba12d 100644 --- a/app/assets/javascripts/mobile/bookmarklet.js +++ b/app/assets/javascripts/mobile/bookmarklet.js @@ -6,16 +6,21 @@ $(document).ready(function() { return params.content; } - var content = params.title + " - " + params.url; - if (params.notes.length > 0) { - content += " - " + params.notes; + var separator = "\n\n"; + var contents = "### " + params.title + separator; + if (params.notes) { + var notes = params.notes.toString().replace(/(?:\r\n|\r|\n)/g, "\n> "); + contents += "> " + notes + separator; } - return content; + contents += params.url; + return contents; } var content = publisherContent(gon.preloads.bookmarklet); if (content.length > 0) { - $("#status_message_text").val(content); + var textarea = $("#status_message_text"); + textarea.val(content); + autosize.update(textarea); } }); // @license-end From 746ff52256591a2ca81dc26d2aae3724301dd008 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 25 Jan 2018 03:02:04 +0100 Subject: [PATCH 2/8] Fix mention in #newhere message when invited by another person fixes #7701 closes #7702 --- Changelog.md | 1 + .../javascripts/app/views/publisher_view.js | 3 +++ app/controllers/streams_controller.rb | 7 ++++++- app/helpers/interim_stream_hackiness_helper.rb | 2 +- lib/publisher.rb | 8 -------- lib/stream/multi.rb | 2 +- spec/controllers/streams_controller_spec.rb | 14 ++++++++++++++ .../interim_stream_hackiness_helper_spec.rb | 15 +++++++++++---- spec/lib/publisher_spec.rb | 7 ------- spec/lib/stream/multi_spec.rb | 2 +- 10 files changed, 38 insertions(+), 23 deletions(-) diff --git a/Changelog.md b/Changelog.md index 1b5f7cce3..9b41bf961 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ ## Bug fixes * Fix invite link on the contacts page when the user has no contacts [#7690](https://github.com/diaspora/diaspora/pull/7690) * Fixed the mobile bookmarklet when called without parameters [#7698](https://github.com/diaspora/diaspora/pull/7698) +* Properly build the #newhere message for people who got invited [#7702](https://github.com/diaspora/diaspora/pull/7702) ## Features * Check if redis is running in script/server [#7685](https://github.com/diaspora/diaspora/pull/7685) diff --git a/app/assets/javascripts/app/views/publisher_view.js b/app/assets/javascripts/app/views/publisher_view.js index 19eb92296..84f957395 100644 --- a/app/assets/javascripts/app/views/publisher_view.js +++ b/app/assets/javascripts/app/views/publisher_view.js @@ -178,6 +178,9 @@ app.views.Publisher = Backbone.View.extend({ if (gon.preloads.getting_started) { this.open(); this.viewGettingStarted.show(); + if (gon.preloads.mentioned_person) { + this.mention.addPersonToMentions(gon.preloads.mentioned_person); + } } }, diff --git a/app/controllers/streams_controller.rb b/app/controllers/streams_controller.rb index 5e638e80d..89190168c 100644 --- a/app/controllers/streams_controller.rb +++ b/app/controllers/streams_controller.rb @@ -30,7 +30,12 @@ class StreamsController < ApplicationController end def multi - gon.preloads[:getting_started] = current_user.getting_started + if current_user.getting_started + gon.preloads[:getting_started] = true + inviter = current_user.invited_by.try(:person) + gon.preloads[:mentioned_person] = {name: inviter.name, handle: inviter.diaspora_handle} if inviter + end + stream_responder(Stream::Multi) end diff --git a/app/helpers/interim_stream_hackiness_helper.rb b/app/helpers/interim_stream_hackiness_helper.rb index 888155b00..b2c7056c7 100644 --- a/app/helpers/interim_stream_hackiness_helper.rb +++ b/app/helpers/interim_stream_hackiness_helper.rb @@ -17,7 +17,7 @@ module InterimStreamHackinessHelper if params[:prefill].present? params[:prefill] elsif defined?(@stream) - @stream.publisher.text + @stream.publisher.prefill else nil end diff --git a/lib/publisher.rb b/lib/publisher.rb index a6c6ab069..fd2808a80 100644 --- a/lib/publisher.rb +++ b/lib/publisher.rb @@ -9,12 +9,4 @@ class Publisher self.prefill = opts[:prefill] self.public = opts[:public] end - - def text - return unless prefill.present? - Diaspora::MessageRenderer.new( - prefill, - mentioned_people: Diaspora::Mentionable.people_from_string(prefill) - ).plain_text - end end diff --git a/lib/stream/multi.rb b/lib/stream/multi.rb index 3ca27a262..0304e0f1f 100644 --- a/lib/stream/multi.rb +++ b/lib/stream/multi.rb @@ -46,7 +46,7 @@ class Stream::Multi < Stream::Base if inviter = self.user.invited_by.try(:person) prefill << I18n.t("shared.publisher.new_user_prefill.invited_by") - prefill << "@{#{inviter.name} ; #{inviter.diaspora_handle}}!" + prefill << "@{#{inviter.diaspora_handle}}!" end prefill diff --git a/spec/controllers/streams_controller_spec.rb b/spec/controllers/streams_controller_spec.rb index 40f9fdf46..9eb5e56ee 100644 --- a/spec/controllers/streams_controller_spec.rb +++ b/spec/controllers/streams_controller_spec.rb @@ -5,6 +5,8 @@ # the COPYRIGHT file. describe StreamsController, :type => :controller do + include_context :gon + before do sign_in alice end @@ -26,6 +28,18 @@ describe StreamsController, :type => :controller do get :multi, :format => :mobile expect(response).to be_success end + + context "getting started" do + it "add the inviter to gon" do + user = FactoryGirl.create(:user, getting_started: true, invited_by: alice) + sign_in user + + get :multi + + expect(gon["preloads"][:mentioned_person][:name]).to eq(alice.person.name) + expect(gon["preloads"][:mentioned_person][:handle]).to eq(alice.person.diaspora_handle) + end + end end streams = { diff --git a/spec/helpers/interim_stream_hackiness_helper_spec.rb b/spec/helpers/interim_stream_hackiness_helper_spec.rb index 890e6e842..69bb04509 100644 --- a/spec/helpers/interim_stream_hackiness_helper_spec.rb +++ b/spec/helpers/interim_stream_hackiness_helper_spec.rb @@ -6,14 +6,14 @@ describe InterimStreamHackinessHelper, type: :helper do before do sign_in alice - def user_signed_in? + def user_signed_in? true end end it 'returns true if no user is signed in' do - def user_signed_in? - false + def user_signed_in? + false end expect(commenting_disabled?(double)).to eq(true) end @@ -22,7 +22,7 @@ describe InterimStreamHackinessHelper, type: :helper do @commenting_disabled = true expect(commenting_disabled?(double)).to eq(true) @commenting_disabled = false - expect(commenting_disabled?(double)).to eq(false) + expect(commenting_disabled?(double)).to eq(false) end it 'returns @stream.can_comment? if @stream is set' do @@ -35,4 +35,11 @@ describe InterimStreamHackinessHelper, type: :helper do expect(commenting_disabled?(post)).to eq(true) end end + + describe "#publisher_formatted_text" do + it "returns the prefill text from the stream" do + @stream = double(publisher: Publisher.new(alice, prefill: "hello world")) + expect(publisher_formatted_text).to eq("hello world") + end + end end diff --git a/spec/lib/publisher_spec.rb b/spec/lib/publisher_spec.rb index 55d9fdea8..22bd7360d 100644 --- a/spec/lib/publisher_spec.rb +++ b/spec/lib/publisher_spec.rb @@ -15,13 +15,6 @@ describe Publisher do end end - describe '#text' do - it 'is a formatted version of the prefill' do - p = Publisher.new(alice, prefill: "@{alice; #{alice.diaspora_handle}}") - expect(p.text).to eq("@alice") - end - end - %w(open public).each do |property| describe "##{property}" do it 'defaults to closed' do diff --git a/spec/lib/stream/multi_spec.rb b/spec/lib/stream/multi_spec.rb index 04dbd153d..3ef7e604a 100644 --- a/spec/lib/stream/multi_spec.rb +++ b/spec/lib/stream/multi_spec.rb @@ -62,7 +62,7 @@ describe Stream::Multi do end it 'includes a mention of the inviter' do - mention = "@{#{@inviter.name} ; #{@inviter.diaspora_handle}}" + mention = "@{#{@inviter.diaspora_handle}}" expect(@stream.send(:publisher_prefill)).to include(mention) end end From 80bfc3fcfdb16f7d425c82a86e497c14587209fd Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 27 Jan 2018 02:30:36 +0100 Subject: [PATCH 3/8] Bump diaspora_federation --- Gemfile | 6 +++--- Gemfile.lock | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Gemfile b/Gemfile index 8234f47be..2fa296632 100644 --- a/Gemfile +++ b/Gemfile @@ -15,8 +15,8 @@ gem "unicorn-worker-killer", "0.4.4" # Federation -gem "diaspora_federation-json_schema", "0.2.2" -gem "diaspora_federation-rails", "0.2.2" +gem "diaspora_federation-json_schema", "0.2.3" +gem "diaspora_federation-rails", "0.2.3" # API and JSON @@ -292,7 +292,7 @@ group :test do gem "timecop", "0.9.1" gem "webmock", "3.0.1", require: false - gem "diaspora_federation-test", "0.2.2" + gem "diaspora_federation-test", "0.2.3" # Coverage gem "coveralls", "0.8.21", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 625eff57f..5e8481ab8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -166,18 +166,18 @@ GEM devise rails (>= 3.0.4) diaspora-prosody-config (0.0.7) - diaspora_federation (0.2.2) - faraday (>= 0.9.0, < 0.14.0) + diaspora_federation (0.2.3) + faraday (>= 0.9.0, < 0.15.0) faraday_middleware (>= 0.10.0, < 0.13.0) nokogiri (~> 1.6, >= 1.6.8) typhoeus (~> 1.0) valid (~> 1.0) - diaspora_federation-json_schema (0.2.2) - diaspora_federation-rails (0.2.2) + diaspora_federation-json_schema (0.2.3) + diaspora_federation-rails (0.2.3) actionpack (>= 4.2, < 6) - diaspora_federation (= 0.2.2) - diaspora_federation-test (0.2.2) - diaspora_federation (= 0.2.2) + diaspora_federation (= 0.2.3) + diaspora_federation-test (0.2.3) + diaspora_federation (= 0.2.3) fabrication (~> 2.16) uuid (~> 2.3, >= 2.3.8) diff-lcs (1.3) @@ -194,7 +194,7 @@ GEM rake et-orbi (1.0.5) tzinfo - ethon (0.10.1) + ethon (0.11.0) ffi (>= 1.3.0) excon (0.59.0) execjs (2.7.0) @@ -204,7 +204,7 @@ GEM sigar (~> 0.7.3) state_machines thor - fabrication (2.16.3) + fabrication (2.20.1) factory_girl (4.8.0) activesupport (>= 3.0.0) factory_girl_rails (4.8.0) @@ -783,9 +783,9 @@ DEPENDENCIES devise (= 4.3.0) devise_lastseenable (= 0.0.6) diaspora-prosody-config (= 0.0.7) - diaspora_federation-json_schema (= 0.2.2) - diaspora_federation-rails (= 0.2.2) - diaspora_federation-test (= 0.2.2) + diaspora_federation-json_schema (= 0.2.3) + diaspora_federation-rails (= 0.2.3) + diaspora_federation-test (= 0.2.3) entypo-rails (= 3.0.0) eye (= 0.9.2) factory_girl_rails (= 4.8.0) @@ -904,4 +904,4 @@ DEPENDENCIES will_paginate (= 3.1.6) BUNDLED WITH - 1.15.4 + 1.16.1 From 815cf121abf45980d243ac52740dee8dc3ac6944 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 27 Jan 2018 02:34:25 +0100 Subject: [PATCH 4/8] Remove participants limit for conversations The limit was added in 2012 to prevent spam, but since the participants need to be a mutual contact with the author nowadays, I don't think it's a spam problem anymore. --- app/models/conversation.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 5197ebd71..bb7cc1f3b 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -9,13 +9,8 @@ class Conversation < ApplicationRecord has_many :participants, class_name: "Person", through: :conversation_visibilities, source: :person has_many :messages, -> { order("created_at ASC") }, inverse_of: :conversation - validate :max_participants validate :local_recipients - def max_participants - errors.add(:max_participants, "too many participants") if participants.count > 20 - end - def local_recipients recipients.each do |recipient| if recipient.local? From b9787cc632b803e776efcaf8ddf517d98334f3ee Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 27 Jan 2018 03:47:50 +0100 Subject: [PATCH 5/8] Start sending the blocking flag --- app/controllers/blocks_controller.rb | 16 +++++-- app/models/block.rb | 5 +++ lib/diaspora/federated/contact_retraction.rb | 4 +- lib/diaspora/federation/entities.rb | 13 +++++- lib/diaspora/federation/mappings.rb | 1 + spec/controllers/blocks_controller_spec.rb | 34 +++++++++++---- spec/lib/diaspora/federation/entities_spec.rb | 42 +++++++++++++++++++ spec/models/block_spec.rb | 9 +++- 8 files changed, 108 insertions(+), 16 deletions(-) diff --git a/app/controllers/blocks_controller.rb b/app/controllers/blocks_controller.rb index a9bcae865..7256a38c8 100644 --- a/app/controllers/blocks_controller.rb +++ b/app/controllers/blocks_controller.rb @@ -6,7 +6,7 @@ class BlocksController < ApplicationController def create block = current_user.blocks.new(block_params) - disconnect_if_contact(block.person) if block.save + send_message(block) if block.save respond_to do |format| format.json { head :no_content } @@ -14,7 +14,9 @@ class BlocksController < ApplicationController end def destroy - notice = if current_user.blocks.find_by(id: params[:id])&.delete + block = current_user.blocks.find_by(id: params[:id]) + notice = if block&.delete + ContactRetraction.for(block).defer_dispatch(current_user) {notice: t("blocks.destroy.success")} else {error: t("blocks.destroy.failure")} @@ -28,8 +30,14 @@ class BlocksController < ApplicationController private - def disconnect_if_contact(person) - current_user.contact_for(person).try {|contact| current_user.disconnect(contact) } + def send_message(block) + contact = current_user.contact_for(block.person) + + if contact + current_user.disconnect(contact) + elsif block.person.remote? + Diaspora::Federation::Dispatcher.defer_dispatch(current_user, block) + end end def block_params diff --git a/app/models/block.rb b/app/models/block.rb index f0e79b22b..147361cac 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -15,4 +15,9 @@ class Block < ApplicationRecord errors[:person_id] << "stop blocking yourself!" end end + + # @return [Array] The recipient of the block + def subscribers + [person] + end end diff --git a/lib/diaspora/federated/contact_retraction.rb b/lib/diaspora/federated/contact_retraction.rb index 57efd2f45..69ec85f7f 100644 --- a/lib/diaspora/federated/contact_retraction.rb +++ b/lib/diaspora/federated/contact_retraction.rb @@ -6,11 +6,11 @@ class ContactRetraction < Retraction end def self.retraction_data_for(target) - Diaspora::Federation::Entities.contact(target).to_h + Diaspora::Federation::Entities.build(target).to_h end def self.for(target) - target.receiving = false + target.receiving = false if target.is_a?(Contact) super end diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index e6dbff2a0..f8d8aa1a5 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -31,6 +31,16 @@ module Diaspora ) end + def self.block(block) + DiasporaFederation::Entities::Contact.new( + author: block.user.diaspora_handle, + recipient: block.person.diaspora_handle, + sharing: false, + following: false, + blocking: Block.exists?(user: block.user, person: block.person) + ) + end + def self.comment(comment) DiasporaFederation::Entities::Comment.new( { @@ -52,7 +62,8 @@ module Diaspora author: contact.user.diaspora_handle, recipient: contact.person.diaspora_handle, sharing: contact.receiving, - following: contact.receiving + following: contact.receiving, + blocking: Block.exists?(user: contact.user, person: contact.person) ) end diff --git a/lib/diaspora/federation/mappings.rb b/lib/diaspora/federation/mappings.rb index 91ccefee6..ac10929c5 100644 --- a/lib/diaspora/federation/mappings.rb +++ b/lib/diaspora/federation/mappings.rb @@ -29,6 +29,7 @@ module Diaspora case diaspora_entity when AccountMigration then :account_migration when AccountDeletion then :account_deletion + when Block then :block when Comment then :comment when Contact then :contact when Conversation then :conversation diff --git a/spec/controllers/blocks_controller_spec.rb b/spec/controllers/blocks_controller_spec.rb index b3d7b1567..11acfdccb 100644 --- a/spec/controllers/blocks_controller_spec.rb +++ b/spec/controllers/blocks_controller_spec.rb @@ -17,8 +17,8 @@ describe BlocksController, :type => :controller do expect(response.status).to eq(204) end - it "calls #disconnect_if_contact" do - expect(@controller).to receive(:disconnect_if_contact).with(bob.person) + it "calls #send_message" do + expect(@controller).to receive(:send_message).with(an_instance_of(Block)) post :create, params: {block: {person_id: bob.person.id}}, format: :json end end @@ -38,6 +38,13 @@ describe BlocksController, :type => :controller do expect(flash[:notice]).to eq(I18n.t("blocks.destroy.success")) end + it "sends a message" do + retraction = double + expect(ContactRetraction).to receive(:for).with(@block).and_return(retraction) + expect(retraction).to receive(:defer_dispatch).with(alice) + delete :destroy, params: {id: @block.id} + end + it "responds with 204 with json" do delete :destroy, params: {id: @block.id}, format: :json expect(response.status).to eq(204) @@ -60,21 +67,32 @@ describe BlocksController, :type => :controller do end end - describe "#disconnect_if_contact" do + describe "#send_message" do before do allow(@controller).to receive(:current_user).and_return(alice) end - it "calls disconnect with the force option if there is a contact for a given user" do + it "calls disconnect if there is a contact for a given user" do + block = alice.blocks.create(person: bob.person) contact = alice.contact_for(bob.person) - allow(alice).to receive(:contact_for).and_return(contact) + expect(alice).to receive(:contact_for).and_return(contact) expect(alice).to receive(:disconnect).with(contact) - @controller.send(:disconnect_if_contact, bob.person) + expect(Diaspora::Federation::Dispatcher).not_to receive(:defer_dispatch) + @controller.send(:send_message, block) end - it "doesn't call disconnect if there is a contact for a given user" do + it "queues a message with the block if the person is remote and there is no contact for a given user" do + block = alice.blocks.create(person: remote_raphael) expect(alice).not_to receive(:disconnect) - @controller.send(:disconnect_if_contact, eve.person) + expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(alice, block) + @controller.send(:send_message, block) + end + + it "does nothing if the person is local and there is no contact for a given user" do + block = alice.blocks.create(person: eve.person) + expect(alice).not_to receive(:disconnect) + expect(Diaspora::Federation::Dispatcher).not_to receive(:defer_dispatch) + @controller.send(:send_message, block) end end end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index 0b80c1cc8..c0ab1aff8 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -56,6 +56,19 @@ describe Diaspora::Federation::Entities do expect(federation_entity.recipient).to eq(diaspora_entity.person.diaspora_handle) expect(federation_entity.sharing).to be_truthy expect(federation_entity.following).to be_truthy + expect(federation_entity.blocking).to be_falsey + end + + it "builds a contact for a block" do + diaspora_entity = FactoryGirl.create(:block) + federation_entity = described_class.build(diaspora_entity) + + expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Contact) + expect(federation_entity.author).to eq(diaspora_entity.user.diaspora_handle) + expect(federation_entity.recipient).to eq(diaspora_entity.person.diaspora_handle) + expect(federation_entity.sharing).to be_falsey + expect(federation_entity.following).to be_falsey + expect(federation_entity.blocking).to be_truthy end context "Conversation" do @@ -237,6 +250,35 @@ describe Diaspora::Federation::Entities do expect(federation_entity.recipient).to eq(target.person.diaspora_handle) expect(federation_entity.sharing).to be_falsey expect(federation_entity.following).to be_falsey + expect(federation_entity.blocking).to be_falsey + end + + it "builds a Contact for a Contact retraction with block" do + target = FactoryGirl.create(:contact, receiving: false) + FactoryGirl.create(:block, user: target.user, person: target.person) + retraction = ContactRetraction.for(target) + federation_entity = described_class.build(retraction) + + expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Contact) + expect(federation_entity.author).to eq(target.user.diaspora_handle) + expect(federation_entity.recipient).to eq(target.person.diaspora_handle) + expect(federation_entity.sharing).to be_falsey + expect(federation_entity.following).to be_falsey + expect(federation_entity.blocking).to be_truthy + end + + it "builds a Contact for a Block retraction" do + target = FactoryGirl.create(:block) + target.delete + retraction = ContactRetraction.for(target) + federation_entity = described_class.build(retraction) + + expect(federation_entity).to be_instance_of(DiasporaFederation::Entities::Contact) + expect(federation_entity.author).to eq(target.user.diaspora_handle) + expect(federation_entity.recipient).to eq(target.person.diaspora_handle) + expect(federation_entity.sharing).to be_falsey + expect(federation_entity.following).to be_falsey + expect(federation_entity.blocking).to be_falsey end end diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index e3ee01c42..27f12848e 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -1,10 +1,17 @@ # frozen_string_literal: true -describe Block, :type => :model do +describe Block, type: :model do describe "validations" do it "doesnt allow you to block yourself" do block = alice.blocks.create(person: alice.person) expect(block.errors[:person_id].size).to eq(1) end end + + describe "#subscribers" do + it "returns an array with recipient of the block" do + block = alice.blocks.create(person: eve.person) + expect(block.subscribers).to match_array([eve.person]) + end + end end From a32cac06ab5bbaa124246d0c5c85e544713f94b3 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 27 Jan 2018 04:16:45 +0100 Subject: [PATCH 6/8] Retry Contact messages 20 time (about two weeks) closes #7705 --- Changelog.md | 2 ++ app/workers/send_base.rb | 2 +- spec/workers/send_base_spec.rb | 14 +++++++------- spec/workers/send_private_spec.rb | 17 +++++++++++++++++ 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Changelog.md b/Changelog.md index 9b41bf961..d38b1809b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,8 @@ * Work on the data downloads: Fixed general layout of buttons, added a timestamp and implemented auto-deletion of old exports [#7684](https://github.com/diaspora/diaspora/pull/7684) * Increase Twitter character limit to 280 [#7694](https://github.com/diaspora/diaspora/pull/7694) * Improve password autocomplete with password managers [#7642](https://github.com/diaspora/diaspora/pull/7642) +* Removed the limit of participants in private conversations [#7705](https://github.com/diaspora/diaspora/pull/7705) +* Send blocks to the blocked persons pod for better UX [#7705](https://github.com/diaspora/diaspora/pull/7705) ## Bug fixes * Fix invite link on the contacts page when the user has no contacts [#7690](https://github.com/diaspora/diaspora/pull/7690) diff --git a/app/workers/send_base.rb b/app/workers/send_base.rb index 4e292de9b..eed2ac19c 100644 --- a/app/workers/send_base.rb +++ b/app/workers/send_base.rb @@ -9,7 +9,7 @@ module Workers protected def schedule_retry(retry_count, sender_id, obj_str, failed_urls) - if retry_count < MAX_RETRIES + if retry_count < (obj_str.start_with?("Contact") ? MAX_RETRIES + 10 : MAX_RETRIES) yield(seconds_to_delay(retry_count), retry_count) else logger.warn "status=abandon sender=#{sender_id} obj=#{obj_str} failed_urls='[#{failed_urls.join(', ')}]'" diff --git a/spec/workers/send_base_spec.rb b/spec/workers/send_base_spec.rb index c7961451b..7a3b77a98 100644 --- a/spec/workers/send_base_spec.rb +++ b/spec/workers/send_base_spec.rb @@ -8,13 +8,13 @@ describe Workers::SendBase do end it "increases the interval for each retry" do - expect(Workers::SendBase.new.send(:seconds_to_delay, 2)).to be >= 625 - expect(Workers::SendBase.new.send(:seconds_to_delay, 3)).to be >= 1_296 - expect(Workers::SendBase.new.send(:seconds_to_delay, 4)).to be >= 2_401 - expect(Workers::SendBase.new.send(:seconds_to_delay, 5)).to be >= 4_096 - expect(Workers::SendBase.new.send(:seconds_to_delay, 6)).to be >= 6_561 - expect(Workers::SendBase.new.send(:seconds_to_delay, 7)).to be >= 10_000 - expect(Workers::SendBase.new.send(:seconds_to_delay, 8)).to be >= 14_641 + (2..19).each do |count| + expect(Workers::SendBase.new.send(:seconds_to_delay, count)).to be >= ((count + 3)**4) + end + + # lets make some tests with explicit numbers to make sure the formula above works correctly + # and increases the delay with the expected result expect(Workers::SendBase.new.send(:seconds_to_delay, 9)).to be >= 20_736 + expect(Workers::SendBase.new.send(:seconds_to_delay, 19)).to be >= 234_256 end end diff --git a/spec/workers/send_private_spec.rb b/spec/workers/send_private_spec.rb index 622c1e93e..1817c5983 100644 --- a/spec/workers/send_private_spec.rb +++ b/spec/workers/send_private_spec.rb @@ -41,4 +41,21 @@ describe Workers::SendPrivate do Workers::SendPrivate.new.perform(sender_id, obj_str, targets, 9) }.to raise_error Workers::SendBase::MaxRetriesReached end + + it "retries contact entities 20 times" do + contact = Fabricate(:contact_entity, author: sender_id, recipient: alice.diaspora_handle) + obj_str = contact.to_s + targets = {"https://example.org/receive/user/guid" => "post"} + expect(DiasporaFederation::Federation::Sender).to receive(:private).with( + sender_id, obj_str, targets + ).and_return(targets).twice + + expect(Workers::SendPrivate).to receive(:perform_in).with(a_kind_of(Numeric), sender_id, obj_str, targets, 19) + Workers::SendPrivate.new.perform(sender_id, obj_str, targets, 18) + + expect(Workers::SendPrivate).not_to receive(:perform_in) + expect { + Workers::SendPrivate.new.perform(sender_id, obj_str, targets, 19) + }.to raise_error Workers::SendBase::MaxRetriesReached + end end From 6c5b8b73afa76b545dcbe125743bdf06624969f2 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 27 Jan 2018 19:00:41 +0100 Subject: [PATCH 7/8] Fix post_message for posts without text fixes #7700 closes #7706 --- Changelog.md | 5 +++-- app/helpers/notifier_helper.rb | 2 +- spec/helpers/notifier_helper_spec.rb | 7 +++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Changelog.md b/Changelog.md index d38b1809b..7655de013 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,13 +4,14 @@ * Work on the data downloads: Fixed general layout of buttons, added a timestamp and implemented auto-deletion of old exports [#7684](https://github.com/diaspora/diaspora/pull/7684) * Increase Twitter character limit to 280 [#7694](https://github.com/diaspora/diaspora/pull/7694) * Improve password autocomplete with password managers [#7642](https://github.com/diaspora/diaspora/pull/7642) -* Removed the limit of participants in private conversations [#7705](https://github.com/diaspora/diaspora/pull/7705) +* Remove the limit of participants in private conversations [#7705](https://github.com/diaspora/diaspora/pull/7705) * Send blocks to the blocked persons pod for better UX [#7705](https://github.com/diaspora/diaspora/pull/7705) ## Bug fixes * Fix invite link on the contacts page when the user has no contacts [#7690](https://github.com/diaspora/diaspora/pull/7690) -* Fixed the mobile bookmarklet when called without parameters [#7698](https://github.com/diaspora/diaspora/pull/7698) +* Fix the mobile bookmarklet when called without parameters [#7698](https://github.com/diaspora/diaspora/pull/7698) * Properly build the #newhere message for people who got invited [#7702](https://github.com/diaspora/diaspora/pull/7702) +* Fix the admin report view for posts without text [#7706](https://github.com/diaspora/diaspora/pull/7706) ## Features * Check if redis is running in script/server [#7685](https://github.com/diaspora/diaspora/pull/7685) diff --git a/app/helpers/notifier_helper.rb b/app/helpers/notifier_helper.rb index eaf1ff597..70e780709 100644 --- a/app/helpers/notifier_helper.rb +++ b/app/helpers/notifier_helper.rb @@ -7,7 +7,7 @@ module NotifierHelper # @return [String] The formatted post. def post_message(post, opts={}) if post.respond_to? :message - post.message.try(:plain_text_without_markdown) || post_page_title(post) + post.message.try(:plain_text_without_markdown).presence || post_page_title(post) else I18n.translate 'notifier.a_post_you_shared' end diff --git a/spec/helpers/notifier_helper_spec.rb b/spec/helpers/notifier_helper_spec.rb index 5e2499001..946753aa8 100644 --- a/spec/helpers/notifier_helper_spec.rb +++ b/spec/helpers/notifier_helper_spec.rb @@ -17,6 +17,13 @@ describe NotifierHelper, :type => :helper do expect(post_message(@markdown_post)).to eq(@striped_markdown_post) end + it "falls back to the title if the post has no text" do + photo = FactoryGirl.build(:photo, public: true) + photo_post = FactoryGirl.build(:status_message, author: photo.author, text: "", photos: [photo], public: true) + expect(helper.post_message(photo_post)) + .to eq(I18n.t("posts.show.photos_by", count: 1, author: photo_post.author_name)) + end + it "falls back to the title, if the root post was deleted" do reshare = FactoryGirl.create(:reshare) reshare.root.destroy From 5e157dc9c3c73b47caf32ef85029ffdc7c82c6db Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 29 Jan 2018 21:38:30 +0100 Subject: [PATCH 8/8] Send participation after receiving a public post This is to let the author of the post know, that this pod is interested in updates about this post. The sending user is only used to verify that the participation was sent from this pod, but lets use an admin/podmin account if available. closes #7708 --- Changelog.md | 1 + lib/diaspora/federation/receive.rb | 22 +++++- .../receive_federation_messages_spec.rb | 5 ++ .../federation/shared_receive_stream_items.rb | 9 +++ spec/lib/diaspora/federation/receive_spec.rb | 16 ++++ spec/shared_behaviors/receiving.rb | 73 +++++++++++++++++++ 6 files changed, 125 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 7655de013..6ff634234 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ * Improve password autocomplete with password managers [#7642](https://github.com/diaspora/diaspora/pull/7642) * Remove the limit of participants in private conversations [#7705](https://github.com/diaspora/diaspora/pull/7705) * Send blocks to the blocked persons pod for better UX [#7705](https://github.com/diaspora/diaspora/pull/7705) +* Send a dummy participation on all incoming public posts to increase interaction consistency [#7708](https://github.com/diaspora/diaspora/pull/7708) ## Bug fixes * Fix invite link on the contacts page when the user has no contacts [#7690](https://github.com/diaspora/diaspora/pull/7690) diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index d4dcea34d..51954db01 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -153,7 +153,7 @@ module Diaspora guid: entity.guid, created_at: entity.created_at, root_guid: entity.root_guid - ) + ).tap {|reshare| send_participation_for(reshare) } end end @@ -193,6 +193,8 @@ module Diaspora status_message.photos = save_or_load_photos(entity.photos) status_message.save! + + send_participation_for(status_message) end end end @@ -328,6 +330,24 @@ module Diaspora end end end + + private_class_method def self.send_participation_for(post) + return unless post.public? + user = user_for_participation + participation = Participation.new(target: post, author: user.person) + Diaspora::Federation::Dispatcher.build(user, participation, subscribers: [post.author]).dispatch + rescue => e # rubocop:disable Lint/RescueWithoutErrorClass + logger.warn "failed to send participation for post #{post.guid}: #{e.class}: #{e.message}" + end + + # Use configured admin account if available, + # or use first user with admin role if available, + # or use first user who isn't closed + private_class_method def self.user_for_participation + User.find_by(username: AppConfig.admins.account.to_s) || + Role.admins.first&.person&.owner || + User.where(locked_at: nil).first + end end end end diff --git a/spec/integration/federation/receive_federation_messages_spec.rb b/spec/integration/federation/receive_federation_messages_spec.rb index c9a67aeb4..b53bc97c1 100644 --- a/spec/integration/federation/receive_federation_messages_spec.rb +++ b/spec/integration/federation/receive_federation_messages_spec.rb @@ -116,6 +116,11 @@ describe "Receive federation messages feature" do alice, instance_of(Reshare) ).and_return(double(create!: true)) + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |_user, participation, _opts| + expect(participation.target.guid).to eq(reshare.guid) + instance_double(:dispatch) + end + post_message(generate_payload(reshare, sender)) expect(Reshare.exists?(root_guid: post.guid)).to be_truthy diff --git a/spec/integration/federation/shared_receive_stream_items.rb b/spec/integration/federation/shared_receive_stream_items.rb index 347848e35..1431b9e1a 100644 --- a/spec/integration/federation/shared_receive_stream_items.rb +++ b/spec/integration/federation/shared_receive_stream_items.rb @@ -8,6 +8,15 @@ shared_examples_for "messages which are indifferent about sharing fact" do it "treats status message receive correctly" do entity = Fabricate(:status_message_entity, author: sender_id, public: public) + if public + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |_user, participation, _opts| + expect(participation.target.guid).to eq(entity.guid) + instance_double(:dispatch) + end + else + expect(Diaspora::Federation::Dispatcher).not_to receive(:build) + end + post_message(generate_payload(entity, sender, recipient), recipient) expect(StatusMessage.exists?(guid: entity.guid)).to be_truthy diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 2c2e7e110..767e67796 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -541,6 +541,10 @@ describe Diaspora::Federation::Receive do it_behaves_like "it ignores existing object received twice", Reshare do let(:entity) { reshare_entity } end + + it_behaves_like "it sends a participation to the author" do + let(:entity) { reshare_entity } + end end describe ".retraction" do @@ -766,6 +770,18 @@ describe Diaspora::Federation::Receive do expect(status_message.photos.map(&:guid)).to include(photo1.guid, photo2.guid) expect(status_message.photos.map(&:text)).to include(received_photo.text, photo2.text) end + + it_behaves_like "it sends a participation to the author" do + let(:entity) { status_message_entity } + end + + it "doesn't send participations for a private post" do + status_message_entity = Fabricate(:status_message_entity, author: sender.diaspora_handle, public: false) + + expect(Diaspora::Federation::Dispatcher).not_to receive(:build) + + Diaspora::Federation::Receive.perform(status_message_entity) + end end end end diff --git a/spec/shared_behaviors/receiving.rb b/spec/shared_behaviors/receiving.rb index 6a563e237..8c2c8da65 100644 --- a/spec/shared_behaviors/receiving.rb +++ b/spec/shared_behaviors/receiving.rb @@ -73,3 +73,76 @@ shared_examples_for "it relays relayables" do |klass| Diaspora::Federation::Receive.perform(entity) end end + +shared_examples_for "it sends a participation to the author" do + it "sends a participation for the post to the author" do + dispatcher = double + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |user, participation, opts| + expect(user).to be_a(User) + expect(participation.target.guid).to eq(entity.guid) + + subscribers = opts[:subscribers] + expect(subscribers.size).to eq(1) + expect(subscribers.first.diaspora_handle).to eq(entity.author) + + dispatcher + end + expect(dispatcher).to receive(:dispatch) + + Diaspora::Federation::Receive.perform(entity) + end + + it "doesn't save the participation in the database" do + participation_guid = nil + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |_user, participation, _opts| + participation_guid = participation.guid + instance_double(:dispatch) + end + + Diaspora::Federation::Receive.perform(entity) + + expect(Participation.where(guid: participation_guid)).not_to exist + end + + it "uses the configured admin as sender for the participation" do + AppConfig.admins.account = bob.username + + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |user, _participation, _opts| + expect(user.username).to eq(bob.username) + instance_double(:dispatch) + end + + Diaspora::Federation::Receive.perform(entity) + end + + it "uses the first user with an admin role if no admin is configured in the config" do + AppConfig.admins.account = nil + admin_role = FactoryGirl.create(:role, name: "admin") + + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |user, _participation, _opts| + expect(user.username).to eq(admin_role.person.owner.username) + instance_double(:dispatch) + end + + Diaspora::Federation::Receive.perform(entity) + end + + it "uses the first open account if no admin is available" do + AppConfig.admins.account = nil + expect(Role.admins).to be_empty + User.first.close_account! + + expect(Diaspora::Federation::Dispatcher).to receive(:build) do |user, _participation, _opts| + expect(user.username).to eq(User.second.username) + instance_double(:dispatch) + end + + Diaspora::Federation::Receive.perform(entity) + end + + it "still receives the entity successfully if there is an error while sending the participation" do + expect(Diaspora::Federation::Dispatcher).to receive(:build).and_raise "FooBar" + + Diaspora::Federation::Receive.perform(entity) + end +end