diff --git a/Changelog.md b/Changelog.md index 2dfddc7f2..62269bca2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -89,6 +89,7 @@ Contributions are very welcome, the hard work is done! * Stream faces are gone [#6686](https://github.com/diaspora/diaspora/pull/6686) * Refactor mobile javascript and add tests [#6394](https://github.com/diaspora/diaspora/pull/6394) * Dropped `parent_author_signature` from relayables [#6586](https://github.com/diaspora/diaspora/pull/6586) +* Attached ShareVisibilities to the User, not the Contact [#6723](https://github.com/diaspora/diaspora/pull/6723) ## Bug fixes * Destroy Participation when removing interactions with a post [#5852](https://github.com/diaspora/diaspora/pull/5852) diff --git a/app/models/contact.rb b/app/models/contact.rb index 861ec0771..a2ffedae4 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -14,9 +14,6 @@ class Contact < ActiveRecord::Base has_many :aspect_memberships, :dependent => :destroy has_many :aspects, :through => :aspect_memberships - has_many :share_visibilities, :source => :shareable, :source_type => 'Post' - has_many :posts, :through => :share_visibilities, :source => :shareable, :source_type => 'Post' - validate :not_contact_for_self, :not_blocked_user, :not_contact_with_closed_account @@ -60,10 +57,6 @@ class Contact < ActiveRecord::Base :into => aspects.first) end - def receive_shareable(shareable) - ShareVisibility.create!(:shareable_id => shareable.id, :shareable_type => shareable.class.base_class.to_s, :contact_id => self.id) - end - def contacts people = Person.arel_table incoming_aspects = Aspect.where( diff --git a/app/models/share_visibility.rb b/app/models/share_visibility.rb index 060f65cd2..0eda3b0d8 100644 --- a/app/models/share_visibility.rb +++ b/app/models/share_visibility.rb @@ -3,40 +3,36 @@ # the COPYRIGHT file. class ShareVisibility < ActiveRecord::Base - belongs_to :contact + belongs_to :user belongs_to :shareable, :polymorphic => :true - scope :for_a_users_contacts, ->(user) { - where(:contact_id => user.contacts.map {|c| c.id}) - } - - scope :for_contacts_of_a_person, ->(person) { - where(:contact_id => person.contacts.map {|c| c.id}) + scope :for_a_user, ->(user) { + where(user_id: user.id) } validate :not_public - # Perform a batch import, given a set of contacts and a shareable + # Perform a batch import, given a set of users and a shareable # @note performs a bulk insert in mySQL; performs linear insertions in postgres - # @param contacts [Array] Recipients + # @param user_ids [Array] Recipients # @param share [Shareable] # @return [void] - def self.batch_import(contact_ids, share) + def self.batch_import(user_ids, share) return false unless ShareVisibility.new(:shareable_id => share.id, :shareable_type => share.class.to_s).valid? if AppConfig.postgres? - contact_ids.each do |contact_id| + user_ids.each do |user_id| ShareVisibility.find_or_create_by( - contact_id: contact_id, - shareable_id: share.id, + user_id: user_id, + shareable_id: share.id, shareable_type: share.class.base_class.to_s ) end else - new_share_visibilities_data = contact_ids.map do |contact_id| - [contact_id, share.id, share.class.base_class.to_s] + new_share_visibilities_data = user_ids.map do |user_id| + [user_id, share.id, share.class.base_class.to_s] end - ShareVisibility.import([:contact_id, :shareable_id, :shareable_type], new_share_visibilities_data) + ShareVisibility.import(%i(user_id shareable_id shareable_type), new_share_visibilities_data) end end diff --git a/app/models/user.rb b/app/models/user.rb index ec009aa47..598ec8cb6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -80,6 +80,8 @@ class User < ActiveRecord::Base has_many :authorizations, class_name: "Api::OpenidConnect::Authorization" has_many :o_auth_applications, through: :authorizations, class_name: "Api::OpenidConnect::OAuthApplication" + has_many :share_visibilities + before_save :guard_unconfirmed_email, :save_person! @@ -114,6 +116,10 @@ class User < ActiveRecord::Base InvitationCode.find_or_create_by(user_id: self.id) end + def receive_shareable(shareable) + ShareVisibility.create!(shareable_id: shareable.id, shareable_type: shareable.class.base_class.to_s, user_id: id) + end + def hidden_shareables self[:hidden_shareables] ||= {} end diff --git a/app/models/user/connecting.rb b/app/models/user/connecting.rb index 333e6bd5e..a1f425206 100644 --- a/app/models/user/connecting.rb +++ b/app/models/user/connecting.rb @@ -24,23 +24,9 @@ module User::Connecting end deliver_profile_update - register_share_visibilities(contact) contact end - # This puts the last 100 public posts by the passed in contact into the user's stream. - # @param [Contact] contact - # @return [void] - def register_share_visibilities(contact) - #should have select here, but proven hard to test - posts = Post.where(:author_id => contact.person_id, :public => true).limit(100) - p = posts.map do |post| - ShareVisibility.new(:contact_id => contact.id, :shareable_id => post.id, :shareable_type => 'Post') - end - ShareVisibility.import(p) unless posts.empty? - nil - end - def remove_contact(contact, opts={:force => false, :retracted => false}) if !contact.mutual? || opts[:force] contact.destroy diff --git a/app/models/user/querying.rb b/app/models/user/querying.rb index 20d52df45..531c8f5fa 100644 --- a/app/models/user/querying.rb +++ b/app/models/user/querying.rb @@ -13,90 +13,12 @@ module User::Querying def visible_shareables(klass, opts={}) opts = prep_opts(klass, opts) shareable_ids = visible_shareable_ids(klass, opts) - klass.where(:id => shareable_ids).select('DISTINCT '+klass.to_s.tableize+'.*').limit(opts[:limit]).order(opts[:order_with_table]).order(klass.table_name+".id DESC") + klass.where(id: shareable_ids).select("DISTINCT #{klass.table_name}.*") + .limit(opts[:limit]).order(opts[:order_with_table]) end def visible_shareable_ids(klass, opts={}) - opts = prep_opts(klass, opts) - visible_ids_from_sql(klass, opts) - end - - # @return [Array] - def visible_ids_from_sql(klass, opts={}) - opts = prep_opts(klass, opts) - opts[:klass] = klass - opts[:by_members_of] ||= self.aspect_ids - - post_ids = klass.connection.select_values(visible_shareable_sql(klass, opts)).map(&:to_i) - post_ids += klass.connection.select_values("#{construct_public_followings_sql(opts).to_sql} LIMIT #{opts[:limit]}").map {|id| id.to_i } - end - - def visible_shareable_sql(klass, opts={}) - table = klass.table_name - opts = prep_opts(klass, opts) - opts[:klass] = klass - - shareable_from_others = construct_shareable_from_others_query(opts) - shareable_from_self = construct_shareable_from_self_query(opts) - - "(#{shareable_from_others.to_sql} LIMIT #{opts[:limit]}) UNION ALL (#{shareable_from_self.to_sql} LIMIT #{opts[:limit]}) ORDER BY #{opts[:order]} LIMIT #{opts[:limit]}" - end - - def ugly_select_clause(query, opts) - klass = opts[:klass] - select_clause ='DISTINCT %s.id, %s.updated_at AS updated_at, %s.created_at AS created_at' % [klass.table_name, klass.table_name, klass.table_name] - query.select(select_clause).order(opts[:order_with_table]).where(klass.arel_table[opts[:order_field]].lt(opts[:max_time])) - end - - def construct_shareable_from_others_query(opts) - conditions = { - :pending => false, - :share_visibilities => {:hidden => opts[:hidden]}, - :contacts => {:user_id => self.id, :receiving => true} - } - - conditions[:type] = opts[:type] if opts.has_key?(:type) - - query = opts[:klass].joins(:contacts).where(conditions) - - if opts[:by_members_of] - query = query.joins(:contacts => :aspect_memberships).where( - :aspect_memberships => {:aspect_id => opts[:by_members_of]}) - end - - ugly_select_clause(query, opts) - end - - def construct_public_followings_sql(opts) - logger.debug "[EVIL-QUERY] user.construct_public_followings_sql" - - # For PostgreSQL and MySQL/MariaDB we use a different query - # see issue: https://github.com/diaspora/diaspora/issues/5014 - if AppConfig.postgres? - query = opts[:klass].where(:author_id => Person.in_aspects(opts[:by_members_of]).select("people.id"), :public => true, :pending => false) - else - aspects = Aspect.where(:id => opts[:by_members_of]) - person_ids = Person.connection.select_values(people_in_aspects(aspects).select("people.id").to_sql) - query = opts[:klass].where(:author_id => person_ids, :public => true, :pending => false) - end - - unless(opts[:klass] == Photo) - query = query.where(:type => opts[:type]) - end - - ugly_select_clause(query, opts) - end - - def construct_shareable_from_self_query(opts) - conditions = {:pending => false, :author_id => self.person_id } - conditions[:type] = opts[:type] if opts.has_key?(:type) - query = opts[:klass].where(conditions) - - if opts[:by_members_of] - query = query.joins(:aspect_visibilities).where(:aspect_visibilities => {:aspect_id => opts[:by_members_of]}) - end - - ugly_select_clause(query, opts) + visible_ids_from_sql(klass, prep_opts(klass, opts)) end def contact_for(person) @@ -144,18 +66,88 @@ module User::Querying end def posts_from(person) - ::EvilQuery::ShareablesFromPerson.new(self, Post, person).make_relation! + Post.from_person_visible_by_user(self, person).order("posts.created_at desc") end def photos_from(person, opts={}) opts = prep_opts(Photo, opts) - ::EvilQuery::ShareablesFromPerson.new(self, Photo, person).make_relation! + Photo.from_person_visible_by_user(self, person) .by_max_time(opts[:max_time]) .limit(opts[:limit]) end protected + # @return [Array] + def visible_ids_from_sql(klass, opts) + opts[:klass] = klass + opts[:by_members_of] ||= aspect_ids + + klass.connection.select_values(visible_shareable_sql(opts)).map(&:to_i) + end + + def visible_shareable_sql(opts) + shareable_from_others = construct_shareable_from_others_query(opts) + shareable_from_self = construct_shareable_from_self_query(opts) + + "(#{shareable_from_others.to_sql} LIMIT #{opts[:limit]}) " \ + "UNION ALL (#{shareable_from_self.to_sql} LIMIT #{opts[:limit]}) " \ + "ORDER BY #{opts[:order]} LIMIT #{opts[:limit]}" + end + + def construct_shareable_from_others_query(opts) + logger.debug "[EVIL-QUERY] user.construct_shareable_from_others_query" + + query = visible_shareables_query(posts_from_aspects_query(opts), opts) + + query = query.where(type: opts[:type]) unless opts[:klass] == Photo + + ugly_select_clause(query, opts) + end + + # For PostgreSQL and MySQL/MariaDB we use a different query + # see issue: https://github.com/diaspora/diaspora/issues/5014 + def posts_from_aspects_query(opts) + if AppConfig.postgres? + opts[:klass].where(author_id: Person.in_aspects(opts[:by_members_of]).select("people.id")) + else + person_ids = Person.connection.select_values(Person.in_aspects(opts[:by_members_of]).select("people.id").to_sql) + opts[:klass].where(author_id: person_ids) + end + end + + def visible_shareables_query(query, opts) + query.with_visibility.where(pending: false).where( + visible_private_shareables(opts).or(opts[:klass].arel_table[:public].eq(true)) + ) + end + + def visible_private_shareables(opts) + ShareVisibility.arel_table[:user_id].eq(id) + .and(ShareVisibility.arel_table[:shareable_type].eq(opts[:klass].to_s)) + .and(ShareVisibility.arel_table[:hidden].eq(opts[:hidden])) + end + + def construct_shareable_from_self_query(opts) + conditions = {pending: false, author_id: person_id} + conditions[:type] = opts[:type] if opts.has_key?(:type) + query = opts[:klass].where(conditions) + + unless opts[:all_aspects?] + query = query.joins(:aspect_visibilities).where(aspect_visibilities: {aspect_id: opts[:by_members_of]}) + end + + ugly_select_clause(query, opts) + end + + def ugly_select_clause(query, opts) + klass = opts[:klass] + table = klass.table_name + select_clause = "DISTINCT %s.id, %s.updated_at AS updated_at, %s.created_at AS created_at" % [table, table, table] + query.select(select_clause).order(opts[:order_with_table]) + .where(klass.arel_table[opts[:order_field]].lt(opts[:max_time])) + end + # @return [Hash] def prep_opts(klass, opts) defaults = { diff --git a/db/migrate/20160225232049_link_share_visibilities_with_user.rb b/db/migrate/20160225232049_link_share_visibilities_with_user.rb new file mode 100644 index 000000000..f7db77306 --- /dev/null +++ b/db/migrate/20160225232049_link_share_visibilities_with_user.rb @@ -0,0 +1,79 @@ +class LinkShareVisibilitiesWithUser < ActiveRecord::Migration + class ShareVisibility < ActiveRecord::Base + end + + def up + cleanup_deleted_share_visibilities + + remove_columns :share_visibilities, :created_at, :updated_at + add_column :share_visibilities, :user_id, :integer + + # update_all from AR doesn't work with postgres, see: https://github.com/rails/rails/issues/13496 + if AppConfig.postgres? + execute "UPDATE share_visibilities SET user_id = contacts.user_id " \ + "FROM contacts WHERE contacts.id = share_visibilities.contact_id" + else + ShareVisibility.joins("INNER JOIN contacts ON share_visibilities.contact_id = contacts.id") + .update_all("share_visibilities.user_id = contacts.user_id") + end + + remove_foreign_key :share_visibilities, name: :post_visibilities_contact_id_fk + + remove_index :share_visibilities, name: :index_post_visibilities_on_contact_id + remove_index :share_visibilities, name: :shareable_and_contact_id + remove_index :share_visibilities, name: :shareable_and_hidden_and_contact_id + + remove_column :share_visibilities, :contact_id + change_column :share_visibilities, :user_id, :integer, null: false + + ShareVisibility.joins("LEFT OUTER JOIN users ON users.id = share_visibilities.user_id") + .delete_all("users.id is NULL") + + add_index :share_visibilities, :user_id + add_index :share_visibilities, %i(shareable_id shareable_type user_id), name: :shareable_and_user_id + add_index :share_visibilities, %i(shareable_id shareable_type hidden user_id), + name: :shareable_and_hidden_and_user_id + + add_foreign_key :share_visibilities, :users, name: :share_visibilities_user_id_fk, on_delete: :cascade + end + + def down + add_column :share_visibilities, :contact_id, :integer + + if AppConfig.postgres? + execute "UPDATE share_visibilities SET contact_id = contacts.id " \ + "FROM contacts WHERE contacts.user_id = share_visibilities.user_id" + else + ShareVisibility.joins("INNER JOIN contacts ON share_visibilities.user_id = contacts.user_id") + .update_all("share_visibilities.contact_id = contacts.id") + end + + remove_foreign_key :share_visibilities, name: :share_visibilities_user_id_fk + + remove_index :share_visibilities, :user_id + remove_index :share_visibilities, name: :shareable_and_user_id + remove_index :share_visibilities, name: :shareable_and_hidden_and_user_id + + remove_column :share_visibilities, :user_id + change_column :share_visibilities, :contact_id, :integer, null: false + + add_index :share_visibilities, :contact_id, name: :index_post_visibilities_on_contact_id + add_index :share_visibilities, %i(shareable_id shareable_type contact_id), name: :shareable_and_contact_id + add_index :share_visibilities, %i(shareable_id shareable_type hidden contact_id), + name: :shareable_and_hidden_and_contact_id + + add_foreign_key :share_visibilities, :contacts, name: :post_visibilities_contact_id_fk, on_delete: :cascade + + add_column :share_visibilities, :created_at, :datetime + add_column :share_visibilities, :updated_at, :datetime + end + + private + + def cleanup_deleted_share_visibilities + ShareVisibility.joins("LEFT OUTER JOIN posts ON posts.id = share_visibilities.shareable_id") + .where(shareable_type: "Post").delete_all("posts.id is NULL") + ShareVisibility.joins("LEFT OUTER JOIN photos ON photos.id = share_visibilities.shareable_id") + .where(shareable_type: "Photo").delete_all("photos.id is NULL") + end +end diff --git a/db/schema.rb b/db/schema.rb index 5214e27e4..04230193d 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: 20151210213023) do +ActiveRecord::Schema.define(version: 20160225232049) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 @@ -103,15 +103,15 @@ ActiveRecord::Schema.define(version: 20151210213023) do end create_table "comments", force: :cascade do |t| - t.text "text", limit: 65535, null: false - t.integer "commentable_id", limit: 4, null: false - t.integer "author_id", limit: 4, null: false - t.string "guid", limit: 255, null: false - t.text "author_signature", limit: 65535 - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.integer "likes_count", limit: 4, default: 0, null: false - t.string "commentable_type", limit: 60, default: "Post", null: false + t.text "text", limit: 65535, null: false + t.integer "commentable_id", limit: 4, null: false + t.integer "author_id", limit: 4, null: false + t.string "guid", limit: 255, null: false + t.text "author_signature", limit: 65535 + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "likes_count", limit: 4, default: 0, null: false + t.string "commentable_type", limit: 60, default: "Post", null: false end add_index "comments", ["author_id"], name: "index_comments_on_person_id", using: :btree @@ -188,14 +188,14 @@ ActiveRecord::Schema.define(version: 20151210213023) do add_index "invitations", ["sender_id"], name: "index_invitations_on_sender_id", using: :btree create_table "likes", force: :cascade do |t| - t.boolean "positive", default: true - t.integer "target_id", limit: 4 - t.integer "author_id", limit: 4 - t.string "guid", limit: 255 - t.text "author_signature", limit: 65535 - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "target_type", limit: 60, null: false + t.boolean "positive", default: true + t.integer "target_id", limit: 4 + t.integer "author_id", limit: 4 + t.string "guid", limit: 255 + t.text "author_signature", limit: 65535 + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "target_type", limit: 60, null: false end add_index "likes", ["author_id"], name: "likes_author_id_fk", using: :btree @@ -222,13 +222,13 @@ ActiveRecord::Schema.define(version: 20151210213023) do add_index "mentions", ["post_id"], name: "index_mentions_on_post_id", using: :btree create_table "messages", force: :cascade do |t| - t.integer "conversation_id", limit: 4, null: false - t.integer "author_id", limit: 4, null: false - t.string "guid", limit: 255, null: false - t.text "text", limit: 65535, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.text "author_signature", limit: 65535 + t.integer "conversation_id", limit: 4, null: false + t.integer "author_id", limit: 4, null: false + t.string "guid", limit: 255, null: false + t.text "text", limit: 65535, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.text "author_signature", limit: 65535 end add_index "messages", ["author_id"], name: "index_messages_on_author_id", using: :btree @@ -312,14 +312,14 @@ ActiveRecord::Schema.define(version: 20151210213023) do end create_table "participations", force: :cascade do |t| - t.string "guid", limit: 255 - t.integer "target_id", limit: 4 - t.string "target_type", limit: 60, null: false - t.integer "author_id", limit: 4 - t.text "author_signature", limit: 65535 - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.integer "count", limit: 4, default: 1, null: false + t.string "guid", limit: 255 + t.integer "target_id", limit: 4 + t.string "target_type", limit: 60, null: false + t.integer "author_id", limit: 4 + t.text "author_signature", limit: 65535 + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "count", limit: 4, default: 1, null: false end add_index "participations", ["guid"], name: "index_participations_on_guid", length: {"guid"=>191}, using: :btree @@ -392,11 +392,11 @@ ActiveRecord::Schema.define(version: 20151210213023) do add_index "poll_answers", ["poll_id"], name: "index_poll_answers_on_poll_id", using: :btree create_table "poll_participations", force: :cascade do |t| - t.integer "poll_answer_id", limit: 4, null: false - t.integer "author_id", limit: 4, null: false - t.integer "poll_id", limit: 4, null: false - t.string "guid", limit: 255 - t.text "author_signature", limit: 65535 + t.integer "poll_answer_id", limit: 4, null: false + t.integer "author_id", limit: 4, null: false + t.integer "poll_id", limit: 4, null: false + t.string "guid", limit: 255 + t.text "author_signature", limit: 65535 t.datetime "created_at" t.datetime "updated_at" end @@ -542,18 +542,16 @@ ActiveRecord::Schema.define(version: 20151210213023) do add_index "services", ["user_id"], name: "index_services_on_user_id", using: :btree create_table "share_visibilities", force: :cascade do |t| - t.integer "shareable_id", limit: 4, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.boolean "hidden", default: false, null: false - t.integer "contact_id", limit: 4, null: false - t.string "shareable_type", limit: 60, default: "Post", null: false + t.integer "shareable_id", limit: 4, null: false + t.boolean "hidden", default: false, null: false + t.string "shareable_type", limit: 60, default: "Post", null: false + t.integer "user_id", limit: 4, null: false end - add_index "share_visibilities", ["contact_id"], name: "index_post_visibilities_on_contact_id", using: :btree - add_index "share_visibilities", ["shareable_id", "shareable_type", "contact_id"], name: "shareable_and_contact_id", using: :btree - add_index "share_visibilities", ["shareable_id", "shareable_type", "hidden", "contact_id"], name: "shareable_and_hidden_and_contact_id", using: :btree + add_index "share_visibilities", ["shareable_id", "shareable_type", "hidden", "user_id"], name: "shareable_and_hidden_and_user_id", using: :btree + add_index "share_visibilities", ["shareable_id", "shareable_type", "user_id"], name: "shareable_and_user_id", using: :btree add_index "share_visibilities", ["shareable_id"], name: "index_post_visibilities_on_post_id", using: :btree + add_index "share_visibilities", ["user_id"], name: "index_share_visibilities_on_user_id", using: :btree create_table "simple_captcha_data", force: :cascade do |t| t.string "key", limit: 40 @@ -679,5 +677,5 @@ ActiveRecord::Schema.define(version: 20151210213023) do add_foreign_key "ppid", "users" add_foreign_key "profiles", "people", name: "profiles_person_id_fk", on_delete: :cascade add_foreign_key "services", "users", name: "services_user_id_fk", on_delete: :cascade - add_foreign_key "share_visibilities", "contacts", name: "post_visibilities_contact_id_fk", on_delete: :cascade + add_foreign_key "share_visibilities", "users", name: "share_visibilities_user_id_fk", on_delete: :cascade end diff --git a/lib/account_deleter.rb b/lib/account_deleter.rb index 4d0335100..7e8fee278 100644 --- a/lib/account_deleter.rb +++ b/lib/account_deleter.rb @@ -27,7 +27,6 @@ class AccountDeleter #person delete_standard_person_associations remove_conversation_visibilities - remove_share_visibilities_on_persons_posts delete_contacts_of_me tombstone_person_and_profile @@ -56,7 +55,7 @@ class AccountDeleter def ignored_ar_user_associations %i(followed_tags invited_by contact_people aspect_memberships - ignored_people conversation_visibilities conversations reports) + ignored_people share_visibilities conversation_visibilities conversations reports) end def delete_standard_user_associations @@ -83,12 +82,8 @@ class AccountDeleter # Currently this would get deleted due to the db foreign key constrainsts, # but we'll keep this method here for completeness - def remove_share_visibilities_on_persons_posts - ShareVisibility.for_contacts_of_a_person(person).destroy_all - end - def remove_share_visibilities_on_contacts_posts - ShareVisibility.for_a_users_contacts(user).destroy_all + ShareVisibility.for_a_user(user).destroy_all end def remove_conversation_visibilities diff --git a/lib/diaspora/federated/shareable.rb b/lib/diaspora/federated/shareable.rb index 0b29da8d8..b2ffee6b9 100644 --- a/lib/diaspora/federated/shareable.rb +++ b/lib/diaspora/federated/shareable.rb @@ -33,14 +33,14 @@ module Diaspora end # @param [User] user The user that is receiving this shareable. - # @param [Person] person The person who dispatched this shareable to the + # @param [Person] _person The sender of the shareable # @return [void] - def receive(user, person) + def receive(user, _person) local_shareable = persisted_shareable if local_shareable - receive_persisted(user, person, local_shareable) if verify_persisted_shareable(local_shareable) + receive_persisted(user, local_shareable) if verify_persisted_shareable(local_shareable) else - receive_non_persisted(user, person) + receive_non_persisted(user) end end @@ -81,12 +81,12 @@ module Diaspora false end - def receive_persisted(user, person, shareable) + def receive_persisted(user, shareable) known_shareable = user.find_visible_shareable_by_id(self.class.base_class, guid, key: :guid) if known_shareable update_existing_sharable(known_shareable) else - receive_shareable_visibility(user, person, shareable) + receive_shareable_visibility(user, shareable) end end @@ -101,18 +101,18 @@ module Diaspora end end - def receive_shareable_visibility(user, person, shareable) - user.contact_for(person).receive_shareable(shareable) + def receive_shareable_visibility(user, shareable) + user.receive_shareable(shareable) user.notify_if_mentioned(shareable) logger.info "event=receive payload_type=#{self.class} status=complete " \ - "sender=#{diaspora_handle} receiver=#{person.diaspora_handle} guid=#{shareable.guid}" + "sender=#{diaspora_handle} receiver=#{user.diaspora_handle} guid=#{shareable.guid}" end - def receive_non_persisted(user, person) + def receive_non_persisted(user) if save logger.info "event=receive payload_type=#{self.class} status=complete sender=#{diaspora_handle} " \ "guid=#{guid}" - receive_shareable_visibility(user, person, self) + receive_shareable_visibility(user, self) else logger.warn "event=receive payload_type=#{self.class} status=abort sender=#{diaspora_handle} " \ "reason=#{errors.full_messages} guid=#{guid}" @@ -122,7 +122,7 @@ module Diaspora logger.info "event=receive payload_type=#{self.class} status=retry sender=#{diaspora_handle} guid=#{guid}" local_shareable = persisted_shareable raise e unless local_shareable - receive_shareable_visibility(user, person, local_shareable) if verify_persisted_shareable(local_shareable) + receive_shareable_visibility(user, local_shareable) if verify_persisted_shareable(local_shareable) end end end diff --git a/lib/diaspora/shareable.rb b/lib/diaspora/shareable.rb index 5c34115f8..f0a5b3121 100644 --- a/lib/diaspora/shareable.rb +++ b/lib/diaspora/shareable.rb @@ -1,55 +1,67 @@ # Copyright (c) 2010, Diaspora Inc. This file is # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -#the pont of this object is to centralize the simmilarities of Photo and post, + +# the point of this object is to centralize the simmilarities of Photo and Post, # as they used to be the same class module Diaspora module Shareable def self.included(model) model.instance_eval do + has_many :aspect_visibilities, as: :shareable, validate: false + has_many :aspects, through: :aspect_visibilities - has_many :aspect_visibilities, :as => :shareable, :validate => false - has_many :aspects, :through => :aspect_visibilities + has_many :share_visibilities, as: :shareable, dependent: :delete_all - has_many :share_visibilities, :as => :shareable - has_many :contacts, :through => :share_visibilities - - belongs_to :author, :class_name => 'Person' + belongs_to :author, class_name: "Person" delegate :id, :name, :first_name, to: :author, prefix: true - #scopes - scope :all_public, -> { where(:public => true, :pending => false) } + # scopes + scope :all_public, -> { where(public: true, pending: false) } + + scope :with_visibility, -> { + joins("LEFT OUTER JOIN share_visibilities ON share_visibilities.shareable_id = #{table_name}.id") + } def self.owned_or_visible_by_user(user) - self.joins("LEFT OUTER JOIN share_visibilities ON share_visibilities.shareable_id = posts.id AND share_visibilities.shareable_type = 'Post'"). - joins("LEFT OUTER JOIN contacts ON contacts.id = share_visibilities.contact_id"). - where( - Contact.arel_table[:user_id].eq(user.id).or( - self.arel_table[:public].eq(true).or( - self.arel_table[:author_id].eq(user.person_id) - ) - ) - ). - select("DISTINCT #{self.table_name}.*") + with_visibility.where( + visible_by_user(user).or(arel_table[:public].eq(true) + .or(arel_table[:author_id].eq(user.person_id))) + ).select("DISTINCT #{table_name}.*") end - def self.for_visible_shareable_sql(max_time, order, limit = 15, types = Stream::Base::TYPES_OF_POST_IN_STREAM) - by_max_time(max_time, order). - where(:type => types). - limit(limit) + def self.from_person_visible_by_user(user, person) + return owned_by_user(user) if person == user.person + + with_visibility.where(author_id: person.id).where( + visible_by_user(user).or(arel_table[:public].eq(true)) + ).select("DISTINCT #{table_name}.*") end - def self.by_max_time(max_time, order='created_at') - where("#{self.table_name}.#{order} < ?", max_time).order("#{self.table_name}.#{order} desc") + def self.for_visible_shareable_sql(max_time, order, limit=15, types=Stream::Base::TYPES_OF_POST_IN_STREAM) + by_max_time(max_time, order).order(table_name + ".id DESC").where(type: types).limit(limit) end + + def self.by_max_time(max_time, order="created_at") + where("#{table_name}.#{order} < ?", max_time).order("#{table_name}.#{order} DESC") + end + + def self.owned_by_user(user) + user.person.send(table_name).where(pending: false) + end + + def self.visible_by_user(user) + ShareVisibility.arel_table[:user_id].eq(user.id) + .and(ShareVisibility.arel_table[:shareable_type].eq(base_class.to_s)) + end + private_class_method :visible_by_user end end # @return [Integer] def update_reshares_counter - self.class.where(:id => self.id). - update_all(:reshares_count => self.reshares.count) + self.class.where(id: id).update_all(reshares_count: reshares.count) end end end diff --git a/lib/evil_query.rb b/lib/evil_query.rb index 20e0e66d6..9f96480bd 100644 --- a/lib/evil_query.rb +++ b/lib/evil_query.rb @@ -61,7 +61,7 @@ module EvilQuery def aspects_post_ids! logger.debug("[EVIL-QUERY] aspect_post_ids!") - @user.visible_shareable_ids(Post, :limit => 15, :order => "#{@order} DESC", :max_time => @max_time, :all_aspects? => true, :by_members_of => @user.aspect_ids) + @user.visible_shareable_ids(Post, limit: 15, order: "#{@order} DESC", max_time: @max_time, all_aspects?: true) end def followed_tags_posts! @@ -94,13 +94,16 @@ module EvilQuery def post! #small optimization - is this optimal order?? - querent_is_contact.first || querent_is_author.first || public_post.first + querent_has_visibility.first || querent_is_author.first || public_post.first end protected - def querent_is_contact - @class.where(@key => @id).joins(:contacts).where(:contacts => {:user_id => @querent.id}).where(@conditions).select(@class.table_name+".*") + def querent_has_visibility + @class.where(@key => @id).joins(:share_visibilities) + .where(share_visibilities: {user_id: @querent.id}) + .where(@conditions) + .select(@class.table_name + ".*") end def querent_is_author @@ -111,49 +114,4 @@ module EvilQuery @class.where(@key => @id, :public => true).where(@conditions) end end - - class ShareablesFromPerson < Base - def initialize(querent, klass, person) - @querent = querent - @class = klass - @person = person - end - - def make_relation! - return querents_posts if @person == @querent.person - - # persons_private_visibilities and persons_public_posts have no limit which is making shareable_ids gigantic. - # perhaps they should the arrays should be merged and sorted - # then the query at the bottom of this method can be paginated or something? - - shareable_ids = contact.present? ? fetch_ids!(persons_private_visibilities, "share_visibilities.shareable_id") : [] - shareable_ids += fetch_ids!(persons_public_posts, table_name + ".id") - - @class.where(:id => shareable_ids, :pending => false). - select('DISTINCT '+table_name+'.*'). - order(table_name+".created_at DESC") - end - - protected - - def table_name - @class.table_name - end - - def contact - @contact ||= @querent.contact_for(@person) - end - - def querents_posts - @querent.person.send(table_name).where(:pending => false).order("#{table_name}.created_at DESC") - end - - def persons_private_visibilities - contact.share_visibilities.where(:hidden => false, :shareable_type => @class.to_s) - end - - def persons_public_posts - @person.send(table_name).where(:public => true).select(table_name+'.id') - end - end end diff --git a/lib/postzord/receiver/local_batch.rb b/lib/postzord/receiver/local_batch.rb index 01a6ff47f..177e557b7 100644 --- a/lib/postzord/receiver/local_batch.rb +++ b/lib/postzord/receiver/local_batch.rb @@ -41,8 +41,7 @@ class Postzord::Receiver::LocalBatch < Postzord::Receiver # @note performs a bulk insert into mySQL # @return [void] def create_share_visibilities - contacts_ids = Contact.connection.select_values(Contact.where(:user_id => @recipient_user_ids, :person_id => @object.author_id).select("id").to_sql) - ShareVisibility.batch_import(contacts_ids, object) + ShareVisibility.batch_import(@recipient_user_ids, object) end # Notify any mentioned users within the @object's text diff --git a/lib/stream/tag.rb b/lib/stream/tag.rb index afb6175a2..d8f3ba06b 100644 --- a/lib/stream/tag.rb +++ b/lib/stream/tag.rb @@ -44,12 +44,11 @@ class Stream::Tag < Stream::Base end def construct_post_query - posts = StatusMessage - if user.present? - posts = posts.owned_or_visible_by_user(user) - else - posts = posts.all_public - end + posts = if user.present? + StatusMessage.owned_or_visible_by_user(user) + else + StatusMessage.all_public + end posts.tagged_with(tag_name, :any => true) end end diff --git a/spec/integration/account_deletion_spec.rb b/spec/integration/account_deletion_spec.rb index 71e52786e..abe3c4db2 100644 --- a/spec/integration/account_deletion_spec.rb +++ b/spec/integration/account_deletion_spec.rb @@ -1,134 +1,130 @@ -require 'spec_helper' +require "spec_helper" -describe 'deleteing your account', :type => :request do +describe "deleteing your account", type: :request do context "user" do before do - @bob2 = bob - @person = @bob2.person - @alices_post = alice.post(:status_message, :text => "@{@bob2 Grimn; #{@bob2.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) + @person = bob.person + @alices_post = alice.post(:status_message, + text: "@{bob Grimn; #{bob.person.diaspora_handle}} you are silly", + to: alice.aspects.find_by_name("generic")) - @bobs_contact_ids = @bob2.contacts.map {|c| c.id} + # bob's own content + bob.post(:status_message, text: "asldkfjs", to: bob.aspects.first) + FactoryGirl.create(:photo, author: bob.person) - #@bob2's own content - @bob2.post(:status_message, :text => 'asldkfjs', :to => @bob2.aspects.first) - f = FactoryGirl.create(:photo, :author => @bob2.person) + @aspect_vis = AspectVisibility.where(aspect_id: bob.aspects.map(&:id)) - @aspect_vis = AspectVisibility.where(:aspect_id => @bob2.aspects.map(&:id)) + # objects on post + bob.like!(@alices_post) + bob.comment!(@alices_post, "here are some thoughts on your post") - #objects on post - @bob2.like!(@alices_post) - @bob2.comment!(@alices_post, "here are some thoughts on your post") + # conversations + create_conversation_with_message(alice, bob.person, "Subject", "Hey bob") - #conversations - create_conversation_with_message(alice, @bob2.person, "Subject", "Hey @bob2") + # join tables + @users_sv = ShareVisibility.where(user_id: bob.id).load + @persons_sv = ShareVisibility.where(shareable_id: bob.posts.map(&:id), shareable_type: "Post").load - #join tables - @users_sv = ShareVisibility.where(:contact_id => @bobs_contact_ids).load - @persons_sv = ShareVisibility.where(:contact_id => bob.person.contacts.map(&:id)).load - - #user associated objects + # user associated objects @prefs = [] - %w{mentioned liked reshared}.each do |pref| - @prefs << @bob2.user_preferences.create!(:email_type => pref) + %w(mentioned liked reshared).each do |pref| + @prefs << bob.user_preferences.create!(email_type: pref) end # notifications @notifications = [] - 3.times do |n| - @notifications << FactoryGirl.create(:notification, :recipient => @bob2) + 3.times do + @notifications << FactoryGirl.create(:notification, recipient: bob) end # services @services = [] - 3.times do |n| - @services << FactoryGirl.create(:service, :user => @bob2) + 3.times do + @services << FactoryGirl.create(:service, user: bob) end # block - @block = @bob2.blocks.create!(:person => eve.person) + @block = bob.blocks.create!(person: eve.person) - #authorization - - AccountDeleter.new(@bob2.person.diaspora_handle).perform! - @bob2.reload + AccountDeleter.new(bob.person.diaspora_handle).perform! + bob.reload end it "deletes all of the user's preferences" do - expect(UserPreference.where(:id => @prefs.map{|pref| pref.id})).to be_empty + expect(UserPreference.where(id: @prefs.map(&:id))).to be_empty end it "deletes all of the user's notifications" do - expect(Notification.where(:id => @notifications.map{|n| n.id})).to be_empty + expect(Notification.where(id: @notifications.map(&:id))).to be_empty end it "deletes all of the users's blocked users" do - expect(Block.where(:id => @block.id)).to be_empty + expect(Block.where(id: @block.id)).to be_empty end it "deletes all of the user's services" do - expect(Service.where(:id => @services.map{|s| s.id})).to be_empty + expect(Service.where(id: @services.map(&:id))).to be_empty end - it 'deletes all of @bob2s share visiblites' do - expect(ShareVisibility.where(:id => @users_sv.map{|sv| sv.id})).to be_empty - expect(ShareVisibility.where(:id => @persons_sv.map{|sv| sv.id})).to be_empty + it "deletes all of bobs share visiblites" do + expect(ShareVisibility.where(id: @users_sv.map(&:id))).to be_empty + expect(ShareVisibility.where(id: @persons_sv.map(&:id))).to be_empty end - it 'deletes all of @bob2s aspect visiblites' do - expect(AspectVisibility.where(:id => @aspect_vis.map(&:id))).to be_empty + it "deletes all of bobs aspect visiblites" do + expect(AspectVisibility.where(id: @aspect_vis.map(&:id))).to be_empty end - it 'deletes all aspects' do - expect(@bob2.aspects).to be_empty + it "deletes all aspects" do + expect(bob.aspects).to be_empty end - it 'deletes all user contacts' do - expect(@bob2.contacts).to be_empty + it "deletes all user contacts" do + expect(bob.contacts).to be_empty end - it "clears the account fields" do - @bob2.send(:clearable_fields).each do |field| - expect(@bob2.reload[field]).to be_blank + bob.send(:clearable_fields).each do |field| + expect(bob.reload[field]).to be_blank end end - it_should_behave_like 'it removes the person associations' + it_should_behave_like "it removes the person associations" end - context 'remote person' do + context "remote person" do before do @person = remote_raphael - #contacts + # contacts @contacts = @person.contacts - #posts + # posts @posts = (1..3).map do - FactoryGirl.create(:status_message, :author => @person) + FactoryGirl.create(:status_message, author: @person) end @persons_sv = @posts.each do |post| @contacts.each do |contact| - ShareVisibility.create!(:contact_id => contact.id, :shareable => post) + ShareVisibility.create!(user_id: contact.user.id, shareable: post) end end - #photos - @photo = FactoryGirl.create(:photo, :author => @person) + # photos + @photo = FactoryGirl.create(:photo, author: @person) - #mentions + # mentions @mentions = 3.times do - FactoryGirl.create(:mention, :person => @person) + FactoryGirl.create(:mention, person: @person) end - #conversations - create_conversation_with_message(alice, @person, "Subject", "Hey @bob2") + # conversations + create_conversation_with_message(alice, @person, "Subject", "Hey bob") AccountDeleter.new(@person.diaspora_handle).perform! @person.reload end - it_should_behave_like 'it removes the person associations' + it_should_behave_like "it removes the person associations" end end diff --git a/spec/integration/receiving_spec.rb b/spec/integration/receiving_spec.rb index c62241cf7..e106047cb 100644 --- a/spec/integration/receiving_spec.rb +++ b/spec/integration/receiving_spec.rb @@ -15,8 +15,6 @@ describe 'a user receives a post', :type => :request do @alices_aspect = alice.aspects.where(:name => "generic").first @bobs_aspect = bob.aspects.where(:name => "generic").first @eves_aspect = eve.aspects.where(:name => "generic").first - - @contact = alice.contact_for(bob.person) end it 'should be able to parse and store a status message from xml' do @@ -134,39 +132,17 @@ describe 'a user receives a post', :type => :request do describe 'post refs' do before do @status_message = bob.post(:status_message, :text => "hi", :to => @bobs_aspect.id) - alice.reload - @alices_aspect.reload - @contact = alice.contact_for(bob.person) end - it "adds a received post to the the contact" do + it "adds a received post to the the user" do expect(alice.visible_shareables(Post)).to include(@status_message) - expect(@contact.posts).to include(@status_message) + expect(ShareVisibility.find_by(user_id: alice.id, shareable_id: @status_message.id)).not_to be_nil end - it 'removes posts upon forceful removal' do - alice.remove_contact(@contact, :force => true) + it "does not remove visibility on disconnect" do + alice.remove_contact(alice.contact_for(bob.person), force: true) alice.reload - expect(alice.visible_shareables(Post)).not_to include @status_message - end - - context 'dependent delete' do - it 'deletes share_visibilities on disconnected by' do - @person = FactoryGirl.create(:person) - alice.contacts.create(:person => @person, :aspects => [@alices_aspect]) - - @post = FactoryGirl.create(:status_message, :author => @person) - expect(@post.share_visibilities).to be_empty - receive_with_zord(alice, @person, @post.to_diaspora_xml) - @contact = alice.contact_for(@person) - @contact.share_visibilities.reset - expect(@contact.posts(true)).to include(@post) - @post.share_visibilities.reset - - expect { - alice.disconnected_by(@person) - }.to change{@post.share_visibilities(true).count}.by(-1) - end + expect(ShareVisibility.find_by(user_id: alice.id, shareable_id: @status_message.id)).not_to be_nil end end diff --git a/spec/lib/account_deleter_spec.rb b/spec/lib/account_deleter_spec.rb index a8d525eb8..673791c89 100644 --- a/spec/lib/account_deleter_spec.rb +++ b/spec/lib/account_deleter_spec.rb @@ -27,7 +27,6 @@ describe AccountDeleter do person_removal_methods = [:delete_contacts_of_me, :delete_standard_person_associations, :tombstone_person_and_profile, - :remove_share_visibilities_on_persons_posts, :remove_conversation_visibilities] context "user deletion" do @@ -158,21 +157,11 @@ describe AccountDeleter do end end - describe "#remove_person_share_visibilities" do - it 'removes the share visibilities for a person ' do - @s_vis = double - expect(ShareVisibility).to receive(:for_contacts_of_a_person).with(bob.person).and_return(@s_vis) - expect(@s_vis).to receive(:destroy_all) - - @account_deletion.remove_share_visibilities_on_persons_posts - end - end - describe "#remove_share_visibilities_by_contacts_of_user" do - it 'removes the share visibilities for a user' do - @s_vis = double - expect(ShareVisibility).to receive(:for_a_users_contacts).with(bob).and_return(@s_vis) - expect(@s_vis).to receive(:destroy_all) + it "removes the share visibilities for a user" do + s_vis = double + expect(ShareVisibility).to receive(:for_a_user).with(bob).and_return(s_vis) + expect(s_vis).to receive(:destroy_all) @account_deletion.remove_share_visibilities_on_contacts_posts end diff --git a/spec/lib/postzord/receiver/local_batch_spec.rb b/spec/lib/postzord/receiver/local_batch_spec.rb index 1a6c8e2fe..5ba25c75c 100644 --- a/spec/lib/postzord/receiver/local_batch_spec.rb +++ b/spec/lib/postzord/receiver/local_batch_spec.rb @@ -35,7 +35,7 @@ describe Postzord::Receiver::LocalBatch do describe '#create_share_visibilities' do it 'calls sharevisibility.batch_import with hashes' do - expect(ShareVisibility).to receive(:batch_import).with(instance_of(Array), @object) + expect(ShareVisibility).to receive(:batch_import).with(@ids, @object) receiver.create_share_visibilities end end diff --git a/spec/migrations/remove_duplicate_and_empty_pods_spec.rb b/spec/migrations/remove_duplicate_and_empty_pods_spec.rb deleted file mode 100644 index fc26b9637..000000000 --- a/spec/migrations/remove_duplicate_and_empty_pods_spec.rb +++ /dev/null @@ -1,64 +0,0 @@ - -require "spec_helper" -require Rails.root.join("db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb") - -describe RemoveDuplicateAndEmptyPods do - self.use_transactional_fixtures = false - - before do - @previous_migration = 20_150_731_123_114 - @my_migration = 20_150_828_132_451 - Pod.delete_all - end - - after :all do - migrate_to(nil) # up - Pod.delete_all # cleanup manually, transactions are disabled - end - - def migrate_to(version) - ActiveRecord::Migrator.migrate(Rails.root.join("db", "migrate"), version) - end - - describe "#up" do - before do - migrate_to(@previous_migration) - - FactoryGirl.create(:pod, host: nil) - FactoryGirl.create(:pod, host: "") - 4.times { FactoryGirl.create(:pod, host: "aaa.aa") } - end - - it "removes duplicates" do - expect(Pod.where(host: "aaa.aa").count).to eql(4) - migrate_to(@my_migration) - expect(Pod.where(host: "aaa.aa").count).to eql(1) - end - - it "removes empty hostnames" do - expect(Pod.where(host: [nil, ""]).count).to eql(2) - migrate_to(@my_migration) - expect(Pod.where(host: [nil, ""]).count).to eql(0) - end - - it "adds an index on the host column" do - expect { - migrate_to(@my_migration) - Pod.reset_column_information - }.to change { Pod.connection.indexes(Pod.table_name).count }.by(1) - end - end - - describe "#down" do - before do - migrate_to(@my_migration) - end - - it "removes the index on the host column" do - expect { - migrate_to(@previous_migration) - Pod.reset_column_information - }.to change { Pod.connection.indexes(Pod.table_name).count }.by(-1) - end - end -end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index d35d4d19f..3a988f2a2 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -148,6 +148,22 @@ describe Post, :type => :model do Post.for_visible_shareable_sql(Time.now + 1, "created_at") end + context "with two posts with the same timestamp" do + before do + aspect_id = alice.aspects.where(name: "generic").first.id + Timecop.freeze Time.now do + alice.post(:status_message, text: "first", to: aspect_id) + alice.post(:status_message, text: "second", to: aspect_id) + end + end + + it "returns them in reverse creation order" do + posts = Post.for_visible_shareable_sql(Time.now + 1, "created_at") + expect(posts.first.text).to eq("second") + expect(posts.at(1).text).to eq("first") + expect(posts.last.text).to eq("alice - 5") + end + end end end end @@ -255,7 +271,7 @@ describe Post, :type => :model do before do @post = FactoryGirl.create(:status_message, author: bob.person) @known_post = Post.new - allow(bob).to receive(:contact_for).with(eve.person).and_return(double(receive_shareable: true)) + allow(bob).to receive(:receive_shareable).with(@known_post).and_return(true) end context "user knows about the post" do @@ -266,13 +282,13 @@ describe Post, :type => :model do it "updates attributes only if mutable" do allow(@known_post).to receive(:mutable?).and_return(true) expect(@known_post).to receive(:update_attributes) - expect(@post.send(:receive_persisted, bob, eve.person, @known_post)).to eq(true) + expect(@post.send(:receive_persisted, bob, @known_post)).to eq(true) end it "does not update attributes if trying to update a non-mutable object" do allow(@known_post).to receive(:mutable?).and_return(false) expect(@known_post).not_to receive(:update_attributes) - @post.send(:receive_persisted, bob, eve.person, @known_post) + @post.send(:receive_persisted, bob, @known_post) end end @@ -283,14 +299,14 @@ describe Post, :type => :model do end it "receives the post from the contact of the author" do - expect(@post.send(:receive_persisted, bob, eve.person, @known_post)).to eq(true) + expect(@post.send(:receive_persisted, bob, @known_post)).to eq(true) end it "notifies the user if they are mentioned" do allow(bob).to receive(:contact_for).with(eve.person).and_return(double(receive_shareable: true)) expect(bob).to receive(:notify_if_mentioned).and_return(true) - expect(@post.send(:receive_persisted, bob, eve.person, @known_post)).to eq(true) + expect(@post.send(:receive_persisted, bob, @known_post)).to eq(true) end end end @@ -304,34 +320,34 @@ describe Post, :type => :model do end it "it receives the post from the contact of the author" do - expect(bob).to receive(:contact_for).with(eve.person).and_return(double(receive_shareable: true)) - expect(@post.send(:receive_non_persisted, bob, eve.person)).to eq(true) + expect(bob).to receive(:receive_shareable).with(@post).and_return(true) + expect(@post.send(:receive_non_persisted, bob)).to eq(true) end it "notifies the user if they are mentioned" do - allow(bob).to receive(:contact_for).with(eve.person).and_return(double(receive_shareable: true)) + allow(bob).to receive(:receive_shareable).with(@post).and_return(true) expect(bob).to receive(:notify_if_mentioned).and_return(true) - expect(@post.send(:receive_non_persisted, bob, eve.person)).to eq(true) + expect(@post.send(:receive_non_persisted, bob)).to eq(true) end it "does not create shareable visibility if the post does not save" do allow(@post).to receive(:save).and_return(false) expect(@post).not_to receive(:receive_shareable_visibility) - @post.send(:receive_non_persisted, bob, eve.person) + @post.send(:receive_non_persisted, bob) end it "retries if saving fails with RecordNotUnique error" do allow(@post).to receive(:save).and_raise(ActiveRecord::RecordNotUnique.new("Duplicate entry ...")) - expect(bob).to receive(:contact_for).with(eve.person).and_return(double(receive_shareable: true)) - expect(@post.send(:receive_non_persisted, bob, eve.person)).to eq(true) + expect(bob).to receive(:receive_shareable).with(@post).and_return(true) + expect(@post.send(:receive_non_persisted, bob)).to eq(true) end it "retries if saving fails with RecordNotUnique error and raise again if no persisted shareable found" do allow(@post).to receive(:save).and_raise(ActiveRecord::RecordNotUnique.new("Duplicate entry ...")) allow(@post).to receive(:persisted_shareable).and_return(nil) - expect(bob).not_to receive(:contact_for).with(eve.person) - expect { @post.send(:receive_non_persisted, bob, eve.person) }.to raise_error(ActiveRecord::RecordNotUnique) + expect(bob).not_to receive(:receive_shareable) + expect { @post.send(:receive_non_persisted, bob) }.to raise_error(ActiveRecord::RecordNotUnique) end end end diff --git a/spec/models/share_visibility_spec.rb b/spec/models/share_visibility_spec.rb index fe2674fe3..39698feab 100644 --- a/spec/models/share_visibility_spec.rb +++ b/spec/models/share_visibility_spec.rb @@ -2,55 +2,45 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe ShareVisibility, :type => :model do - describe '.batch_import' do - before do - @post = FactoryGirl.create(:status_message, :author => alice.person) - @contact = bob.contact_for(alice.person) + describe ".batch_import" do + let(:post) { FactoryGirl.create(:status_message, author: alice.person) } + + it "returns false if share is public" do + post.public = true + post.save + expect(ShareVisibility.batch_import([bob.id], post)).to be false end - it 'returns false if share is public' do - @post.public = true - @post.save - expect(ShareVisibility.batch_import([@contact.id], @post)).to be false - end - - it 'creates a visibility for each user' do + it "creates a visibility for each user" do expect { - ShareVisibility.batch_import([@contact.id], @post) + ShareVisibility.batch_import([bob.id], post) }.to change { - ShareVisibility.exists?(:contact_id => @contact.id, :shareable_id => @post.id, :shareable_type => 'Post') + ShareVisibility.exists?(user_id: bob.id, shareable_id: post.id, shareable_type: "Post") }.from(false).to(true) end - it 'does not raise if a visibility already exists' do - ShareVisibility.create!(:contact_id => @contact.id, :shareable_id => @post.id, :shareable_type => 'Post') + it "does not raise if a visibility already exists" do + ShareVisibility.create!(user_id: bob.id, shareable_id: post.id, shareable_type: "Post") expect { - ShareVisibility.batch_import([@contact.id], @post) + ShareVisibility.batch_import([bob.id], post) }.not_to raise_error end context "scopes" do - describe '.for_a_users_contacts' do - before do - alice.post(:status_message, :text => "Hey", :to => alice.aspects.first) - end + before do + alice.post(:status_message, text: "Hey", to: alice.aspects.first) - it 'searches for share visibilies for all users contacts' do - contact_ids = alice.contacts.map(&:id) - expect(ShareVisibility.for_a_users_contacts(alice)).to eq(ShareVisibility.where(:contact_id => contact_ids).to_a) - end + photo_path = File.join(File.dirname(__FILE__), "..", "fixtures", "button.png") + alice.post(:photo, user_file: File.open(photo_path), text: "Photo", to: alice.aspects.first) end - describe '.for_contacts_of_a_person' do - it 'searches for share visibilties generated by a person' do - - contact_ids = alice.person.contacts.map(&:id) - - ShareVisibility.for_contacts_of_a_person(alice.person) == ShareVisibility.where(:contact_id => contact_ids).to_a - + describe ".for_a_user" do + it "searches for share visibilies for a user" do + expect(ShareVisibility.for_a_user(bob).count).to eq(2) + expect(ShareVisibility.for_a_user(bob)).to eq(ShareVisibility.where(user_id: bob.id).to_a) end end end diff --git a/spec/models/user/connecting_spec.rb b/spec/models/user/connecting_spec.rb index 155dff64b..f33fcd233 100644 --- a/spec/models/user/connecting_spec.rb +++ b/spec/models/user/connecting_spec.rb @@ -82,16 +82,6 @@ describe User::Connecting, :type => :model do end end - describe '#register_share_visibilities' do - it 'creates post visibilites for up to 100 posts' do - allow(Post).to receive_message_chain(:where, :limit).and_return([FactoryGirl.create(:status_message)]) - c = Contact.create!(:user_id => alice.id, :person_id => eve.person.id) - expect{ - alice.register_share_visibilities(c) - }.to change(ShareVisibility, :count).by(1) - end - end - describe '#share_with' do it 'finds or creates a contact' do expect { @@ -120,11 +110,6 @@ describe User::Connecting, :type => :model do }.to change(contact.aspects, :count).by(1) end - it 'calls #register_share_visibilities with a contact' do - expect(eve).to receive(:register_share_visibilities) - eve.share_with(alice.person, eve.aspects.first) - end - context 'dispatching' do it 'dispatches a request on initial request' do contact = alice.contacts.new(:person => eve.person) diff --git a/spec/models/user/querying_spec.rb b/spec/models/user/querying_spec.rb index 9d8a0ff14..6adebe002 100644 --- a/spec/models/user/querying_spec.rb +++ b/spec/models/user/querying_spec.rb @@ -85,8 +85,7 @@ describe User::Querying, :type => :model do end it "does not pull back hidden posts" do - visibility = @status.share_visibilities(Post).where(:contact_id => alice.contact_for(bob.person).id).first - visibility.update_attributes(:hidden => true) + @status.share_visibilities(Post).where(user_id: alice.id).first.update_attributes(hidden: true) expect(alice.visible_shareable_ids(Post).include?(@status.id)).to be false end end @@ -114,21 +113,6 @@ describe User::Querying, :type => :model do expect(bob.visible_shareables(Post).count(:all)).to eq(0) end - context 'with two posts with the same timestamp' do - before do - aspect_id = alice.aspects.where(:name => "generic").first.id - Timecop.freeze Time.now do - alice.post :status_message, :text => "first", :to => aspect_id - alice.post :status_message, :text => "second", :to => aspect_id - end - end - - it "returns them in reverse creation order" do - expect(bob.visible_shareables(Post).first.text).to eq("second") - expect(bob.visible_shareables(Post).last.text).to eq("first") - end - end - context 'with many posts' do before do time_interval = 1000 diff --git a/spec/shared_behaviors/account_deletion.rb b/spec/shared_behaviors/account_deletion.rb index 027442b14..1f4b655c1 100644 --- a/spec/shared_behaviors/account_deletion.rb +++ b/spec/shared_behaviors/account_deletion.rb @@ -32,8 +32,4 @@ shared_examples_for 'it removes the person associations' do expect(ConversationVisibility.where(:person_id => alice.person.id)).not_to be_empty expect(ConversationVisibility.where(:person_id => @person.id)).to be_empty end - - it "deletes the share visibilities on the person's posts" do - expect(ShareVisibility.for_contacts_of_a_person(@person)).to be_empty - end end