From 6cf1cd5d7657e58eb93e18df8576978f403ae8a2 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 11 Aug 2016 01:09:58 +0200 Subject: [PATCH 1/6] migration to remove old unused invitation columns from users table --- ...4_cleanup_invitation_columns_from_users.rb | 74 +++++++++++++++++++ db/schema.rb | 33 +-------- 2 files changed, 77 insertions(+), 30 deletions(-) create mode 100644 db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb diff --git a/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb b/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb new file mode 100644 index 000000000..b1bbf103a --- /dev/null +++ b/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb @@ -0,0 +1,74 @@ +class CleanupInvitationColumnsFromUsers < ActiveRecord::Migration + class User < ActiveRecord::Base + end + + def change + remove_index :users, column: %i(invitation_service invitation_identifier), + name: :index_users_on_invitation_service_and_invitation_identifier, + unique: true, length: {invitation_service: 64} + remove_index :users, column: :invitation_token, name: :index_users_on_invitation_token + remove_index :users, column: :email, name: :index_users_on_email, length: 191 + + username_not_null + + remove_column :users, :invitation_token, :string, limit: 60 + remove_column :users, :invitation_sent_at, :datetime + remove_column :users, :invitation_service, :string, limit: 127 + remove_column :users, :invitation_identifier, :string, limit: 127 + remove_column :users, :invitation_limit, :integer + remove_column :users, :invited_by_type, :string + + add_index :users, :email, name: :index_users_on_email, unique: true, length: 191 + + drop_invitations_table + end + + def username_not_null + reversible do |dir| + dir.up do + User.delete_all(username: nil) + change_column :users, :username, :string, null: false + end + + dir.down do + change_column :users, :username, :string, null: true + end + end + end + + def drop_invitations_table + reversible do |dir| + dir.up do + drop_table :invitations + end + + dir.down do + create_invitations_table + end + end + end + + def create_invitations_table + # rubocop:disable Style/ExtraSpacing + create_table :invitations, force: :cascade do |t| + t.text :message, limit: 65_535 + t.integer :sender_id, limit: 4 + t.integer :recipient_id, limit: 4 + t.integer :aspect_id, limit: 4 + t.datetime :created_at, null: false + t.datetime :updated_at, null: false + t.string :service, limit: 255 + t.string :identifier, limit: 255 + t.boolean :admin, default: false + t.string :language, limit: 255, default: "en" + end + # rubocop:enable Style/ExtraSpacing + + add_index :invitations, :aspect_id, name: :index_invitations_on_aspect_id, using: :btree + add_index :invitations, :recipient_id, name: :index_invitations_on_recipient_id, using: :btree + add_index :invitations, :sender_id, name: :index_invitations_on_sender_id, using: :btree + + add_foreign_key :invitations, :users, column: :recipient_id, name: :invitations_recipient_id_fk, on_delete: :cascade + add_foreign_key :invitations, :users, column: :sender_id, name: :invitations_sender_id_fk, on_delete: :cascade + end +end diff --git a/db/schema.rb b/db/schema.rb index 90513270a..b1333a8cd 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: 20160807212443) do +ActiveRecord::Schema.define(version: 20160810230114) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 @@ -178,23 +178,6 @@ ActiveRecord::Schema.define(version: 20160807212443) do t.datetime "updated_at", null: false end - create_table "invitations", force: :cascade do |t| - t.text "message", limit: 65535 - t.integer "sender_id", limit: 4 - t.integer "recipient_id", limit: 4 - t.integer "aspect_id", limit: 4 - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "service", limit: 255 - t.string "identifier", limit: 255 - t.boolean "admin", default: false - t.string "language", limit: 255, default: "en" - end - - add_index "invitations", ["aspect_id"], name: "index_invitations_on_aspect_id", using: :btree - add_index "invitations", ["recipient_id"], name: "index_invitations_on_recipient_id", using: :btree - add_index "invitations", ["sender_id"], name: "index_invitations_on_sender_id", using: :btree - create_table "like_signatures", id: false, force: :cascade do |t| t.integer "like_id", limit: 4, null: false t.text "author_signature", limit: 65535, null: false @@ -624,15 +607,13 @@ ActiveRecord::Schema.define(version: 20160807212443) do end create_table "users", force: :cascade do |t| - t.string "username", limit: 255 + t.string "username", limit: 255, null: false t.text "serialized_private_key", limit: 65535 t.boolean "getting_started", default: true, null: false t.boolean "disable_mail", default: false, null: false t.string "language", limit: 255 t.string "email", limit: 255, default: "", null: false t.string "encrypted_password", limit: 255, default: "", null: false - t.string "invitation_token", limit: 60 - t.datetime "invitation_sent_at" t.string "reset_password_token", limit: 255 t.datetime "remember_created_at" t.integer "sign_in_count", limit: 4, default: 0 @@ -642,11 +623,7 @@ ActiveRecord::Schema.define(version: 20160807212443) do t.string "last_sign_in_ip", limit: 255 t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "invitation_service", limit: 127 - t.string "invitation_identifier", limit: 127 - t.integer "invitation_limit", limit: 4 t.integer "invited_by_id", limit: 4 - t.string "invited_by_type", limit: 255 t.string "authentication_token", limit: 30 t.string "unconfirmed_email", limit: 255 t.string "confirm_email_token", limit: 30 @@ -669,9 +646,7 @@ ActiveRecord::Schema.define(version: 20160807212443) do end add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree - add_index "users", ["email"], name: "index_users_on_email", length: {"email"=>191}, using: :btree - add_index "users", ["invitation_service", "invitation_identifier"], name: "index_users_on_invitation_service_and_invitation_identifier", unique: true, length: {"invitation_service"=>64, "invitation_identifier"=>nil}, using: :btree - add_index "users", ["invitation_token"], name: "index_users_on_invitation_token", using: :btree + add_index "users", ["email"], name: "index_users_on_email", unique: true, length: {"email"=>191}, using: :btree add_index "users", ["username"], name: "index_users_on_username", unique: true, length: {"username"=>191}, using: :btree add_foreign_key "aspect_memberships", "aspects", name: "aspect_memberships_aspect_id_fk", on_delete: :cascade @@ -687,8 +662,6 @@ ActiveRecord::Schema.define(version: 20160807212443) do add_foreign_key "conversation_visibilities", "people", name: "conversation_visibilities_person_id_fk", on_delete: :cascade add_foreign_key "conversations", "people", column: "author_id", name: "conversations_author_id_fk", on_delete: :cascade add_foreign_key "id_tokens", "authorizations" - add_foreign_key "invitations", "users", column: "recipient_id", name: "invitations_recipient_id_fk", on_delete: :cascade - add_foreign_key "invitations", "users", column: "sender_id", name: "invitations_sender_id_fk", on_delete: :cascade add_foreign_key "like_signatures", "likes", name: "like_signatures_like_id_fk", on_delete: :cascade add_foreign_key "like_signatures", "signature_orders", name: "like_signatures_signature_orders_id_fk" add_foreign_key "likes", "people", column: "author_id", name: "likes_author_id_fk", on_delete: :cascade From 66b7b7e27a6c7e00c3de536fa9f3a113910b6baf Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 11 Aug 2016 22:01:34 +0200 Subject: [PATCH 2/6] Cleanup legacy invitations from code Fixes #5116 --- app/controllers/invitations_controller.rb | 41 +---- app/controllers/registrations_controller.rb | 6 +- app/models/invitation.rb | 151 ------------------ app/models/user.rb | 15 +- app/workers/resend_invitation.rb | 15 -- config/locales/diaspora/en.yml | 3 - config/routes.rb | 7 +- lib/account_deleter.rb | 11 +- lib/email_inviter.rb | 6 +- .../invitations_controller_spec.rb | 31 ---- spec/lib/account_deleter_spec.rb | 31 ++-- spec/models/invitation_spec.rb | 74 --------- spec/models/user_spec.rb | 27 +--- spec/workers/resend_invitation_spec.rb | 17 -- 14 files changed, 29 insertions(+), 406 deletions(-) delete mode 100644 app/models/invitation.rb delete mode 100644 app/workers/resend_invitation.rb delete mode 100644 spec/models/invitation_spec.rb delete mode 100644 spec/workers/resend_invitation_spec.rb diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index 6acf3602d..ebfff4a7c 100644 --- a/app/controllers/invitations_controller.rb +++ b/app/controllers/invitations_controller.rb @@ -4,7 +4,7 @@ class InvitationsController < ApplicationController - before_action :authenticate_user!, :only => [:new, :create] + before_action :authenticate_user! def new @invite_code = current_user.invitation_code @@ -19,36 +19,6 @@ class InvitationsController < ApplicationController end end - # this is for legacy invites. We try to look the person who sent them the - # invite, and use their new invite code - # owe will be removing this eventually - # @depreciated - def edit - user = User.find_by_invitation_token(params[:invitation_token]) - invitation_code = user.ugly_accept_invitation_code - redirect_to invite_code_path(invitation_code) - end - - def email - @invitation_code = - if params[:invitation_token] - # this is for legacy invites. - user = User.find_by_invitation_token(params[:invitation_token]) - - user.ugly_accept_invitation_code if user - else - params[:invitation_code] - end - @inviter = user || InvitationCode.where(id: params[:invitation_code]).first.try(:user) - if @invitation_code.present? - render 'notifier/invite', :layout => false - else - flash[:error] = t('invitations.check_token.not_found') - - redirect_to root_url - end - end - def create emails = inviter_params[:emails].split(',').map(&:strip).uniq @@ -78,15 +48,8 @@ class InvitationsController < ApplicationController redirect_to :back end - def check_if_invites_open - unless AppConfig.settings.invitations.open? - flash[:error] = I18n.t 'invitations.create.no_more' - - redirect_to :back - end - end - private + def valid_email?(email) User.email_regexp.match(email).present? end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 8b816e390..5614fc112 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -56,6 +56,10 @@ class RegistrationsController < Devise::RegistrationsController helper_method :invite def user_params - params.require(:user).permit(:username, :email, :getting_started, :password, :password_confirmation, :language, :disable_mail, :invitation_service, :invitation_identifier, :show_community_spotlight_in_stream, :auto_follow_back, :auto_follow_back_aspect_id, :remember_me, :captcha, :captcha_key) + params.require(:user).permit( + :username, :email, :getting_started, :password, :password_confirmation, :language, :disable_mail, + :show_community_spotlight_in_stream, :auto_follow_back, :auto_follow_back_aspect_id, + :remember_me, :captcha, :captcha_key + ) end end diff --git a/app/models/invitation.rb b/app/models/invitation.rb deleted file mode 100644 index 40e5037c9..000000000 --- a/app/models/invitation.rb +++ /dev/null @@ -1,151 +0,0 @@ -# Copyright (c) 2010-2011, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - -#TODO: kill me -class Invitation < ActiveRecord::Base - - belongs_to :sender, :class_name => 'User' - belongs_to :recipient, :class_name => 'User' - belongs_to :aspect - - before_validation :set_email_as_default_service - - # before_create :share_with_exsisting_user, :if => :recipient_id? - validates :identifier, :presence => true - validates :service, :presence => true - validate :valid_identifier? - validate :recipient_not_on_pod? - validates_presence_of :sender, :aspect, :unless => :admin? - validate :ensure_not_inviting_self, :on => :create, :unless => :admin? - validate :sender_owns_aspect?, :unless => :admin? - validates_uniqueness_of :sender_id, :scope => [:identifier, :service], :unless => :admin? - - - # @note options hash is passed through to [Invitation.new] - # @see [Invitation.new] - # - # @param [Array] emails - # @option opts [User] :sender - # @option opts [Aspect] :aspect - # @option opts [String] :service - # @return [Array] An array of [Invitation] models - # the valid optsnes are saved, and the invalid ones are not. - def self.batch_invite(emails, opts) - - users_on_pod = User.where(:email => emails, :invitation_token => nil) - - #share with anyone whose email you entered who is on the pod - users_on_pod.each{|u| opts[:sender].share_with(u.person, opts[:aspect])} - - emails.map! do |e| - user = users_on_pod.find{|u| u.email == e} - Invitation.create(opts.merge(:identifier => e, :recipient => user)) - end - emails - end - - - # Downcases the incoming service identifier and assigns it - # - # @param ident [String] Service identifier - # @see super - def identifier=(ident) - ident.downcase! if ident - super - end - - # Determine if we want to skip emailing the recipient. - # - # @return [Boolean] - # @return [void] - def skip_email? - !email_like_identifer - end - - # Find or create user, and send that resultant User an - # invitation. - # - # @return [Invitation] self - def send! - if email_like_identifer - EmailInviter.new(self.identifier, sender).send! - else - puts "broken facebook invitation_token" - end - self - end - - - # converts a personal invitation to an admin invite - # used in account deletion - # @return [Invitation] self - def convert_to_admin! - self.admin = true - self.sender = nil - self.aspect = nil - self.save - self - end - # @return [Invitation] self - def resend - self.send! - end - - # @return [String] - def recipient_identifier - case self.service - when 'email' - self.identifier - when'facebook' - I18n.t('invitations.a_facebook_user') - end - end - - # @return [String] - def email_like_identifer - case self.service - when 'email' - self.identifier - when 'facebook' - false - end - end - - # @note before_save - def set_email_as_default_service - self.service ||= 'email' - end - - # @note Validation - def ensure_not_inviting_self - if self.identifier == self.sender.email - errors[:base] << 'You can not invite yourself.' - end - end - - # @note Validation - def sender_owns_aspect? - if self.sender_id != self.aspect.user_id - errors[:base] << 'You do not own that aspect.' - end - end - - - def recipient_not_on_pod? - return true if self.recipient.nil? - if self.recipient.username? - errors[:recipient] << "The user '#{self.identifier}' (#{self.recipient.diaspora_handle}) is already on this pod, so we sent them a share request" - end - end - - # @note Validation - def valid_identifier? - return false unless self.identifier - if self.service == 'email' - unless self.identifier.match(Devise.email_regexp) - errors[:base] << 'invalid email' - end - end - end -end diff --git a/app/models/user.rb b/app/models/user.rb index f6698277c..8d941a8c5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -14,7 +14,7 @@ class User < ActiveRecord::Base scope :daily_actives, ->(time = Time.now) { logged_in_since(time - 1.day) } scope :yearly_actives, ->(time = Time.now) { logged_in_since(time - 1.year) } scope :halfyear_actives, ->(time = Time.now) { logged_in_since(time - 6.month) } - scope :active, -> { joins(:person).where(people: {closed_account: false}).where.not(username: nil) } + scope :active, -> { joins(:person).where(people: {closed_account: false}) } devise :token_authenticatable, :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable, @@ -34,7 +34,7 @@ class User < ActiveRecord::Base validate :unconfirmed_email_quasiuniqueness - validates_presence_of :person, :unless => proc {|user| user.invitation_token.present?} + validates :person, presence: true validates_associated :person validate :no_person_with_same_username @@ -48,8 +48,6 @@ class User < ActiveRecord::Base :first_name, :last_name, :gender, :participations, to: :person delegate :id, :guid, to: :person, prefix: true - has_many :invitations_from_me, :class_name => 'Invitation', :foreign_key => :sender_id - has_many :invitations_to_me, :class_name => 'Invitation', :foreign_key => :recipient_id has_many :aspects, -> { order('order_id ASC') } belongs_to :auto_follow_back_aspect, :class_name => 'Aspect' @@ -99,15 +97,6 @@ class User < ActiveRecord::Base ConversationVisibility.where(person_id: self.person_id).sum(:unread) end - #@deprecated - def ugly_accept_invitation_code - begin - self.invitations_to_me.first.sender.invitation_code - rescue Exception => e - nil - end - end - def process_invite_acceptence(invite) self.invited_by = invite.user invite.use! diff --git a/app/workers/resend_invitation.rb b/app/workers/resend_invitation.rb deleted file mode 100644 index 5b698a28d..000000000 --- a/app/workers/resend_invitation.rb +++ /dev/null @@ -1,15 +0,0 @@ -# Copyright (c) 2010-2011, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - - -module Workers - class ResendInvitation < Base - sidekiq_options queue: :low - - def perform(invitation_id) - inv = Invitation.find(invitation_id) - inv.resend - end - end -end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 99419cc2d..b94d0f24b 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -571,9 +571,6 @@ en: zero: "No invites left on this code" one: "One invite left on this code" other: "%{count} invites left on this code" - check_token: - not_found: "Invitation token not found" - a_facebook_user: "A Facebook user" layouts: header: diff --git a/config/routes.rb b/config/routes.rb index 31a740484..4fc376e2d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -122,11 +122,8 @@ Diaspora::Application.routes.draw do post "/users" => "registrations#create", :as => :user_registration end - #legacy routes to support old invite routes - get 'users/invitation/accept' => 'invitations#edit' - get 'invitations/email' => 'invitations#email', :as => 'invite_email' - get 'users/invitations' => 'invitations#new', :as => 'new_user_invitation' - post 'users/invitations' => 'invitations#create', :as => 'user_invitation' + get "users/invitations" => "invitations#new", :as => "new_user_invitation" + post "users/invitations" => "invitations#create", :as => "user_invitation" get 'login' => redirect('/users/sign_in') diff --git a/lib/account_deleter.rb b/lib/account_deleter.rb index 58707810f..83c31c528 100644 --- a/lib/account_deleter.rb +++ b/lib/account_deleter.rb @@ -34,7 +34,6 @@ class AccountDeleter #user deletion methods remove_share_visibilities_on_contacts_posts delete_standard_user_associations - disassociate_invitations disconnect_contacts tombstone_user end @@ -45,12 +44,12 @@ class AccountDeleter #user deletions def normal_ar_user_associates_to_delete - %i(tag_followings invitations_to_me services aspects user_preferences + %i(tag_followings services aspects user_preferences notifications blocks authorizations o_auth_applications pairwise_pseudonymous_identifiers) end def special_ar_user_associations - %i(invitations_from_me person profile contacts auto_follow_back_aspect) + %i(person profile contacts auto_follow_back_aspect) end def ignored_ar_user_associations @@ -70,12 +69,6 @@ class AccountDeleter end end - def disassociate_invitations - user.invitations_from_me.each do |inv| - inv.convert_to_admin! - end - end - def disconnect_contacts user.contacts.destroy_all end diff --git a/lib/email_inviter.rb b/lib/email_inviter.rb index 7f8c54475..3fdfd567d 100644 --- a/lib/email_inviter.rb +++ b/lib/email_inviter.rb @@ -5,7 +5,7 @@ class EmailInviter options = options.symbolize_keys self.message = options[:message] self.locale = options.fetch(:locale, 'en') - self.inviter = inviter + self.inviter = inviter self.emails = emails end @@ -16,7 +16,7 @@ class EmailInviter end def invitation_code - @invitation_code ||= inviter.invitation_code + @invitation_code ||= inviter.invitation_code end def send! @@ -26,6 +26,6 @@ class EmailInviter private def mail(email) - Notifier.invite(email, message, inviter, invitation_code, locale).deliver_now! + Notifier.invite(email, message, inviter, invitation_code, locale).deliver_now end end diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 06f9385fd..83cea3a8b 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/spec/controllers/invitations_controller_spec.rb @@ -126,37 +126,6 @@ describe InvitationsController, :type => :controller do end end - describe '#email' do - - it 'succeeds' do - get :email, :invitation_code => "anycode" - expect(response).to be_success - end - - context 'legacy invite tokens' do - def get_email - get :email, :invitation_token => @invitation_token - end - - context 'invalid token' do - @invitation_token = "invalidtoken" - - it 'redirects and flashes if the invitation token is invalid' do - get_email - - expect(response).to be_redirect - expect(response).to redirect_to root_url - end - - it 'flashes an error if the invitation token is invalid' do - get_email - - expect(flash[:error]).to eq(I18n.t("invitations.check_token.not_found")) - end - end - end - end - describe '#new' do it 'renders' do sign_in @user, scope: :user diff --git a/spec/lib/account_deleter_spec.rb b/spec/lib/account_deleter_spec.rb index 673791c89..8ca011b2b 100644 --- a/spec/lib/account_deleter_spec.rb +++ b/spec/lib/account_deleter_spec.rb @@ -16,18 +16,18 @@ describe AccountDeleter do end describe '#perform' do + user_removal_methods = %i( + delete_standard_user_associations + remove_share_visibilities_on_contacts_posts + disconnect_contacts tombstone_user + ) - - user_removal_methods = [:delete_standard_user_associations, - :disassociate_invitations, - :remove_share_visibilities_on_contacts_posts, - :disconnect_contacts, - :tombstone_user] - - person_removal_methods = [:delete_contacts_of_me, - :delete_standard_person_associations, - :tombstone_person_and_profile, - :remove_conversation_visibilities] + person_removal_methods = %i( + delete_contacts_of_me + delete_standard_person_associations + tombstone_person_and_profile + remove_conversation_visibilities + ) context "user deletion" do after do @@ -110,15 +110,6 @@ describe AccountDeleter do end end - describe "#disassociate_invitations" do - it "sets invitations_from_me to be admin invitations" do - invites = [double] - allow(bob).to receive(:invitations_from_me).and_return(invites) - expect(invites.first).to receive(:convert_to_admin!) - @account_deletion.disassociate_invitations - end - end - context 'person associations' do describe '#disconnect_contacts' do it "deletes all of user's contacts" do diff --git a/spec/models/invitation_spec.rb b/spec/models/invitation_spec.rb deleted file mode 100644 index 539b61582..000000000 --- a/spec/models/invitation_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -# Copyright (c) 2010-2011, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - -require 'spec_helper' - -describe Invitation, :type => :model do - let(:user) { alice } - - before do - @email = 'maggie@example.com' - Devise.mailer.deliveries = [] - end - describe 'validations' do - before do - @invitation = FactoryGirl.build(:invitation, :sender => user, :recipient => nil, :aspect => user.aspects.first, :language => "de") - end - - it 'is valid' do - expect(@invitation.sender).to eq(user) - expect(@invitation.recipient).to eq(nil) - expect(@invitation.aspect).to eq(user.aspects.first) - expect(@invitation.language).to eq("de") - expect(@invitation).to be_valid - end - - it 'ensures the sender is placing the recipient into one of his aspects' do - @invitation.aspect = FactoryGirl.build(:aspect) - expect(@invitation).not_to be_valid - end - end - - describe '#language' do - it 'returns the correct language if the language is set' do - @invitation = FactoryGirl.build(:invitation, :sender => user, :recipient => eve, :aspect => user.aspects.first, :language => "de") - expect(@invitation.language).to eq("de") - end - - it 'returns en if no language is set' do - @invitation = FactoryGirl.build(:invitation, :sender => user, :recipient => eve, :aspect => user.aspects.first) - expect(@invitation.language).to eq("en") - end - end - - it 'has a message' do - @invitation = FactoryGirl.build(:invitation, :sender => user, :recipient => eve, :aspect => user.aspects.first, :language => user.language) - @invitation.message = "!" - expect(@invitation.message).to eq("!") - end - - - describe '.batch_invite' do - before do - @emails = ['max@foo.com', 'bob@mom.com'] - @opts = {:aspect => eve.aspects.first, :sender => eve, :service => 'email', :language => eve.language} - end - - it 'returns an array of invites based on the emails passed in' do - invites = Invitation.batch_invite(@emails, @opts) - expect(invites.count).to be 2 - expect(invites.all?{|x| x.persisted?}).to be true - end - - it 'shares with people who are already on the pod' do - FactoryGirl.create(:user, :email => @emails.first) - invites = nil - expect{ - invites = Invitation.batch_invite(@emails, @opts) - }.to change(eve.contacts, :count).by(1) - expect(invites.count).to be 2 - - end - end -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index eb37f8b9d..2d636647f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -566,20 +566,6 @@ describe User, :type => :model do describe 'account deletion' do describe '#destroy' do - it 'removes invitations from the user' do - FactoryGirl.create(:invitation, :sender => alice) - expect { - alice.destroy - }.to change {alice.invitations_from_me(true).count }.by(-1) - end - - it 'removes invitations to the user' do - Invitation.new(:sender => eve, :recipient => alice, :identifier => alice.email, :aspect => eve.aspects.first).save(:validate => false) - expect { - alice.destroy - }.to change {alice.invitations_to_me(true).count }.by(-1) - end - it 'removes all service connections' do Services::Facebook.create(:access_token => 'what', :user_id => alice.id) expect { @@ -960,10 +946,8 @@ describe User, :type => :model do describe "#clearable_attributes" do it 'returns the clearable fields' do user = FactoryGirl.create :user - expect(user.send(:clearable_fields).sort).to eq(%w{ + expect(user.send(:clearable_fields).sort).to eq(%w( language - invitation_token - invitation_sent_at reset_password_sent_at reset_password_token remember_created_at @@ -973,11 +957,7 @@ describe User, :type => :model do current_sign_in_ip hidden_shareables last_sign_in_ip - invitation_service - invitation_identifier - invitation_limit invited_by_id - invited_by_type authentication_token auto_follow_back auto_follow_back_aspect_id @@ -985,7 +965,7 @@ describe User, :type => :model do confirm_email_token last_seen color_theme - }.sort) + ).sort) end end end @@ -1110,9 +1090,6 @@ describe User, :type => :model do describe "active" do before do - invited_user = FactoryGirl.build(:user, username: nil) - invited_user.save(validate: false) - closed_account = FactoryGirl.create(:user) closed_account.person.lock_access! end diff --git a/spec/workers/resend_invitation_spec.rb b/spec/workers/resend_invitation_spec.rb deleted file mode 100644 index 002cf78a5..000000000 --- a/spec/workers/resend_invitation_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# Copyright (c) 2010-2011, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - -require 'spec_helper' - -describe Workers::ResendInvitation do - describe '#perfom' do - it 'should call .resend on the object' do - invite = FactoryGirl.build(:invitation, :service => 'email', :identifier => 'foo@bar.com') - - allow(Invitation).to receive(:find).and_return(invite) - expect(invite).to receive(:resend) - Workers::ResendInvitation.new.perform(invite.id) - end - end -end From 2a553940d4156bc19b4bb65a808f0f18bfda616f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 12 Aug 2016 01:28:18 +0200 Subject: [PATCH 3/6] small design fixes for invites --- app/controllers/invitations_controller.rb | 26 ++++++++----------- app/views/invitations/new.html.haml | 4 +-- config/locales/diaspora/en.yml | 2 +- .../invitations_controller_spec.rb | 12 ++++----- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index ebfff4a7c..b7f8f9882 100644 --- a/app/controllers/invitations_controller.rb +++ b/app/controllers/invitations_controller.rb @@ -3,7 +3,6 @@ # the COPYRIGHT file. class InvitationsController < ApplicationController - before_action :authenticate_user! def new @@ -14,35 +13,32 @@ class InvitationsController < ApplicationController respond_to do |format| format.html do - render 'invitations/new', layout: false + render "invitations/new", layout: false end end end def create - emails = inviter_params[:emails].split(',').map(&:strip).uniq + emails = inviter_params[:emails].split(",").map(&:strip).uniq - valid_emails, invalid_emails = emails.partition { |email| valid_email?(email) } + valid_emails, invalid_emails = emails.partition {|email| valid_email?(email) } session[:valid_email_invites] = valid_emails session[:invalid_email_invites] = invalid_emails unless valid_emails.empty? - Workers::Mail::InviteEmail.perform_async(valid_emails.join(','), - current_user.id, - inviter_params) + Workers::Mail::InviteEmail.perform_async(valid_emails.join(","), current_user.id, inviter_params) end if emails.empty? - flash[:error] = t('invitations.create.empty') + flash[:error] = t("invitations.create.empty") elsif invalid_emails.empty? - flash[:notice] = t('invitations.create.sent', :emails => valid_emails.join(', ')) + flash[:notice] = t("invitations.create.sent", emails: valid_emails.join(", ")) elsif valid_emails.empty? - flash[:error] = t('invitations.create.rejected') + invalid_emails.join(', ') + flash[:error] = t("invitations.create.rejected", emails: invalid_emails.join(", ")) else - flash[:error] = t('invitations.create.sent', :emails => valid_emails.join(', ')) - flash[:error] << '. ' - flash[:error] << t('invitations.create.rejected') + invalid_emails.join(', ') + flash[:error] = t("invitations.create.sent", emails: valid_emails.join(", ")) + ". " + + t("invitations.create.rejected", emails: invalid_emails.join(", ")) end redirect_to :back @@ -57,9 +53,9 @@ class InvitationsController < ApplicationController def html_safe_string_from_session_array(key) return "" unless session[key].present? return "" unless session[key].respond_to?(:join) - value = session[key].join(', ').html_safe + value = session[key].join(", ").html_safe session[key] = nil - return value + value end def inviter_params diff --git a/app/views/invitations/new.html.haml b/app/views/invitations/new.html.haml index 078844276..43a360397 100644 --- a/app/views/invitations/new.html.haml +++ b/app/views/invitations/new.html.haml @@ -15,8 +15,8 @@ .col-sm-10 = text_field_tag 'email_inviter[emails]', @invalid_emails, title: t('.comma_separated_plz'), placeholder: 'foo@bar.com, max@foo.com...', class: "form-control" - #already_sent - = t('invitations.create.note_already_sent', emails: @valid_emails) unless @valid_emails.empty? + #already_sent + = t("invitations.create.note_already_sent", emails: @valid_emails) unless @valid_emails.empty? .form-group %label.col-sm-2.control-label{ for: 'email_inviter_locale' } diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index b94d0f24b..bdac8f67d 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -556,7 +556,7 @@ en: invitations: create: sent: "Invitations have been sent to: %{emails}" - rejected: "The following email addresses had problems: " + rejected: "The following email addresses had problems: %{emails}" no_more: "You have no more invitations." empty: "Please enter at least one email address." note_already_sent: "Invitations have already been sent to: %{emails}" diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 83cea3a8b..086c90991 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/spec/controllers/invitations_controller_spec.rb @@ -81,10 +81,10 @@ describe InvitationsController, :type => :controller do expect(response).to redirect_to @referer end - it 'flashes an error' do + it "flashes an error" do post :create, @invite - expected = I18n.t('invitations.create.rejected') + @emails.split(',').join(', ') + expected = I18n.t("invitations.create.rejected", emails: @emails.split(",").join(", ")) expect(flash[:error]).to eq(expected) end end @@ -108,12 +108,10 @@ describe InvitationsController, :type => :controller do expect(response).to redirect_to @referer end - it 'flashes a notice' do + it "flashes a notice" do post :create, @invite - expected = I18n.t('invitations.create.sent', :emails => - @valid_emails.split(',').join(', ')) + - '. ' + I18n.t('invitations.create.rejected') + - @invalid_emails.split(',').join(', ') + expected = I18n.t("invitations.create.sent", emails: @valid_emails.split(",").join(", ")) + ". " + + I18n.t("invitations.create.rejected", emails: @invalid_emails.split(",").join(", ")) expect(flash[:error]).to eq(expected) end end From e749bbef1524237b053949aa9577ba1cf62a0eef Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 12 Aug 2016 01:42:41 +0200 Subject: [PATCH 4/6] don't reduce number of available invites if there were errors. --- app/controllers/registrations_controller.rb | 16 ++--- app/models/invitation_code.rb | 4 +- .../registrations_controller_spec.rb | 62 ++++++++++++------- 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 5614fc112..b7dc560bd 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -5,14 +5,14 @@ class RegistrationsController < Devise::RegistrationsController before_action :check_registrations_open_or_valid_invite!, :check_valid_invite! - layout ->(c) { request.format == :mobile ? "application" : "with_header" }, :only => [:new] + layout -> { request.format == :mobile ? "application" : "with_header" } def create @user = User.build(user_params) - @user.process_invite_acceptence(invite) if invite.present? if @user.sign_up - flash[:notice] = I18n.t 'registrations.create.success' + flash[:notice] = t("registrations.create.success") + @user.process_invite_acceptence(invite) if invite.present? @user.seed_aspects @user.send_welcome_message sign_in_and_redirect(:user, @user) @@ -22,14 +22,10 @@ class RegistrationsController < Devise::RegistrationsController flash.now[:error] = @user.errors.full_messages.join(" - ") logger.info "event=registration status=failure errors='#{@user.errors.full_messages.join(', ')}'" - render action: "new", layout: request.format == :mobile ? "application" : "with_header" + render action: "new" end end - def new - super - end - private def check_valid_invite! @@ -48,9 +44,7 @@ class RegistrationsController < Devise::RegistrationsController end def invite - if params[:invite].present? - @invite ||= InvitationCode.find_by_token(params[:invite][:token]) - end + @invite ||= InvitationCode.find_by_token(params[:invite][:token]) if params[:invite].present? end helper_method :invite diff --git a/app/models/invitation_code.rb b/app/models/invitation_code.rb index 35fa80d49..10bf5c8d3 100644 --- a/app/models/invitation_code.rb +++ b/app/models/invitation_code.rb @@ -6,9 +6,9 @@ class InvitationCode < ActiveRecord::Base before_create :generate_token, :set_default_invite_count delegate :name, to: :user, prefix: true - + def to_param - token + token end def can_be_used? diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 78f5afb20..41e3916c9 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -2,20 +2,23 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' +require "spec_helper" describe RegistrationsController, type: :controller do before do request.env["devise.mapping"] = Devise.mappings[:user] - @valid_params = {:user => { - :username => "jdoe", - :email => "jdoe@example.com", - :password => "password", - :password_confirmation => "password" + end + + let(:valid_params) { + { + user: { + username: "jdoe", + email: "jdoe@example.com", + password: "password", + password_confirmation: "password" } } - allow(Person).to receive(:find_or_fetch_by_identifier).and_return(FactoryGirl.create(:person)) - end + } describe '#check_registrations_open!' do before do @@ -29,7 +32,7 @@ describe RegistrationsController, type: :controller do end it 'redirects #create to the login page' do - post :create, @valid_params + post :create, valid_params expect(flash[:error]).to eq(I18n.t('registrations.closed')) expect(response).to redirect_to new_user_session_path end @@ -58,58 +61,71 @@ describe RegistrationsController, type: :controller do it "creates a user" do expect { - get :create, @valid_params + get :create, valid_params }.to change(User, :count).by(1) end it "assigns @user" do - get :create, @valid_params + get :create, valid_params expect(assigns(:user)).to be_truthy end it "sets the flash" do - get :create, @valid_params + get :create, valid_params expect(flash[:notice]).not_to be_blank end + it "uses the invite code" do + code = InvitationCode.create(user: bob) + + expect { + get :create, valid_params.merge(invite: {token: code.token}) + }.to change { code.reload.count }.by(-1) + end + it "redirects to the home path" do - get :create, @valid_params + get :create, valid_params expect(response).to be_redirect expect(response.location).to match /^#{stream_url}\??$/ end end context "with invalid parameters" do - before do - @invalid_params = @valid_params - @invalid_params[:user][:password_confirmation] = "baddword" - end + let(:invalid_params) { valid_params.deep_merge(user: {password_confirmation: "baddword"}) } it "does not create a user" do - expect { get :create, @invalid_params }.not_to change(User, :count) + expect { get :create, invalid_params }.not_to change(User, :count) end it "does not create a person" do - expect { get :create, @invalid_params }.not_to change(Person, :count) + expect { get :create, invalid_params }.not_to change(Person, :count) end it "assigns @user" do - get :create, @invalid_params + get :create, invalid_params expect(assigns(:user)).not_to be_nil end it "sets the flash error" do - get :create, @invalid_params + get :create, invalid_params expect(flash[:error]).not_to be_blank end + it "doesn't reduce number of available invites" do + code = InvitationCode.create(user: bob) + + expect { + get :create, invalid_params.merge(invite: {token: code.token}) + }.not_to change { code.reload.count } + end + it "renders new" do - get :create, @invalid_params + get :create, invalid_params expect(response).to render_template("registrations/new") end it "keeps invalid params in form" do - get :create, @invalid_params + get :create, invalid_params expect(response.body).to match /jdoe@example.com/m end end From 3b1a5c6bdf7324da8e51d4ae7e817039d0e623d9 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 12 Aug 2016 22:50:06 +0200 Subject: [PATCH 5/6] don't reduce number of invites when registration is open otherwise the counter goes into negative ;) also reset all negative counters --- app/controllers/registrations_controller.rb | 18 ++-- app/models/user.rb | 3 +- app/views/invitations/new.html.haml | 2 +- ...4_cleanup_invitation_columns_from_users.rb | 11 ++- features/step_definitions/user_steps.rb | 1 + features/support/env.rb | 5 +- .../registrations_controller_spec.rb | 86 +++++++++++++------ 7 files changed, 81 insertions(+), 45 deletions(-) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index b7dc560bd..561c52ea0 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -3,7 +3,7 @@ # the COPYRIGHT file. class RegistrationsController < Devise::RegistrationsController - before_action :check_registrations_open_or_valid_invite!, :check_valid_invite! + before_action :check_registrations_open_or_valid_invite! layout -> { request.format == :mobile ? "application" : "with_header" } @@ -28,19 +28,11 @@ class RegistrationsController < Devise::RegistrationsController private - def check_valid_invite! - return true if AppConfig.settings.enable_registrations? #this sucks - return true if invite && invite.can_be_used? - flash[:error] = t('registrations.invalid_invite') - redirect_to new_user_session_path - end - def check_registrations_open_or_valid_invite! - return true if invite.present? - unless AppConfig.settings.enable_registrations? - flash[:error] = t('registrations.closed') - redirect_to new_user_session_path - end + return true if AppConfig.settings.enable_registrations? || invite.try(:can_be_used?) + + flash[:error] = params[:invite] ? t("registrations.invalid_invite") : t("registrations.closed") + redirect_to new_user_session_path end def invite diff --git a/app/models/user.rb b/app/models/user.rb index 8d941a8c5..e2243d73b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -99,10 +99,9 @@ class User < ActiveRecord::Base def process_invite_acceptence(invite) self.invited_by = invite.user - invite.use! + invite.use! unless AppConfig.settings.enable_registrations? end - def invitation_code InvitationCode.find_or_create_by(user_id: self.id) end diff --git a/app/views/invitations/new.html.haml b/app/views/invitations/new.html.haml index 43a360397..187a6fa12 100644 --- a/app/views/invitations/new.html.haml +++ b/app/views/invitations/new.html.haml @@ -1,7 +1,7 @@ #paste_link = t('.paste_link') %span#codes_left - = '(' + t('.codes_left', count: @invite_code.count) + ')' + = "(" + t(".codes_left", count: @invite_code.count) + ")" unless AppConfig.settings.enable_registrations? .form-horizontal .control-group = invite_link(@invite_code) diff --git a/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb b/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb index b1bbf103a..0ef827249 100644 --- a/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb +++ b/db/migrate/20160810230114_cleanup_invitation_columns_from_users.rb @@ -1,4 +1,7 @@ class CleanupInvitationColumnsFromUsers < ActiveRecord::Migration + class InvitationCode < ActiveRecord::Base + end + class User < ActiveRecord::Base end @@ -20,7 +23,7 @@ class CleanupInvitationColumnsFromUsers < ActiveRecord::Migration add_index :users, :email, name: :index_users_on_email, unique: true, length: 191 - drop_invitations_table + cleanup_invitations end def username_not_null @@ -36,10 +39,14 @@ class CleanupInvitationColumnsFromUsers < ActiveRecord::Migration end end - def drop_invitations_table + def cleanup_invitations reversible do |dir| dir.up do drop_table :invitations + + # reset negative invitation counters + new_counter = AppConfig.settings.enable_registrations? ? AppConfig["settings.invitations.count"] : 0 + InvitationCode.where("count < 0").update_all(count: new_counter) end dir.down do diff --git a/features/step_definitions/user_steps.rb b/features/step_definitions/user_steps.rb index 45e1ded4c..af59a406c 100644 --- a/features/step_definitions/user_steps.rb +++ b/features/step_definitions/user_steps.rb @@ -47,6 +47,7 @@ Given /^I have been invited by an admin$/ do end Given /^I have been invited by "([^\"]+)"$/ do |email| + AppConfig.settings.enable_registrations = false @inviter = User.find_by_email(email) @inviter_invite_count = @inviter.invitation_code.count i = EmailInviter.new("new_invitee@example.com", @inviter) diff --git a/features/support/env.rb b/features/support/env.rb index 297b63f3e..c26248a98 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -70,6 +70,9 @@ Before do |scenario| page.driver.headers = if scenario.source_tag_names.include? "@mobile" {"User-Agent" => "Mozilla/5.0 (Mobile; rv:18.0) Gecko/18.0 Firefox/18.0"} else - page.driver.headers = {} + {} end + + # Reset overridden settings + AppConfig.reset_dynamic! end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 41e3916c9..1a7c43987 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -20,32 +20,50 @@ describe RegistrationsController, type: :controller do } } - describe '#check_registrations_open!' do + describe "#check_registrations_open_or_valid_invite!" do before do AppConfig.settings.enable_registrations = false end - it 'redirects #new to the login page' do + it "redirects #new to the login page" do get :new - expect(flash[:error]).to eq(I18n.t('registrations.closed')) + expect(flash[:error]).to eq(I18n.t("registrations.closed")) expect(response).to redirect_to new_user_session_path end - it 'redirects #create to the login page' do + it "redirects #create to the login page" do post :create, valid_params - expect(flash[:error]).to eq(I18n.t('registrations.closed')) + expect(flash[:error]).to eq(I18n.t("registrations.closed")) expect(response).to redirect_to new_user_session_path end - it 'does not redirect if there is a valid invite token' do - i = InvitationCode.create(:user => bob) - get :new, :invite => {:token => i.token} + it "does not redirect if there is a valid invite token" do + code = InvitationCode.create(user: bob) + get :new, invite: {token: code.token} expect(response).not_to be_redirect end - it 'does redirect if there is an invalid invite token' do - get :new, :invite => {:token => 'fssdfsd'} - expect(response).to be_redirect + it "does redirect if there is an invalid invite token" do + get :new, invite: {token: "fssdfsd"} + expect(response).to redirect_to new_user_session_path + end + + it "does redirect if there are no invites available with this code" do + code = InvitationCode.create(user: bob) + code.update_attributes(count: 0) + + get :new, invite: {token: code.token} + expect(response).to redirect_to new_user_session_path + end + + it "does not redirect when the registration is open" do + AppConfig.settings.enable_registrations = true + + code = InvitationCode.create(user: bob) + code.update_attributes(count: 0) + + get :new, invite: {token: code.token} + expect(response).not_to be_redirect end end @@ -53,12 +71,6 @@ describe RegistrationsController, type: :controller do render_views context "with valid parameters" do - before do - AppConfig.settings.enable_registrations = true - user = FactoryGirl.build(:user) - allow(User).to receive(:build).and_return(user) - end - it "creates a user" do expect { get :create, valid_params @@ -75,18 +87,38 @@ describe RegistrationsController, type: :controller do expect(flash[:notice]).not_to be_blank end - it "uses the invite code" do - code = InvitationCode.create(user: bob) - - expect { - get :create, valid_params.merge(invite: {token: code.token}) - }.to change { code.reload.count }.by(-1) - end - it "redirects to the home path" do get :create, valid_params expect(response).to be_redirect - expect(response.location).to match /^#{stream_url}\??$/ + expect(response.location).to match(/^#{getting_started_url}$/) + end + + context "with invite code" do + it "reduces number of available invites when the registration is closed" do + AppConfig.settings.enable_registrations = false + + code = InvitationCode.create(user: bob) + + expect { + get :create, valid_params.merge(invite: {token: code.token}) + }.to change { code.reload.count }.by(-1) + end + + it "doesn't reduce number of available invites when the registration is open" do + code = InvitationCode.create(user: bob) + + expect { + get :create, valid_params.merge(invite: {token: code.token}) + }.not_to change { code.reload.count } + end + + it "links inviter with the user" do + code = InvitationCode.create(user: bob) + + post :create, valid_params.merge(invite: {token: code.token}) + + expect(User.find_by(username: "jdoe").invited_by).to eq(bob) + end end end @@ -112,6 +144,8 @@ describe RegistrationsController, type: :controller do end it "doesn't reduce number of available invites" do + AppConfig.settings.enable_registrations = false + code = InvitationCode.create(user: bob) expect { From bc6c8a059836334a495b138fbdfff63c610a9dc1 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 12 Aug 2016 23:30:16 +0200 Subject: [PATCH 6/6] disable registration with invite-code when invitations are closed also display message if the user has no invitations left and refactored InvitationsController spec and remove unused message parameter --- app/controllers/invitations_controller.rb | 12 ++ app/models/invitation_code.rb | 2 +- config/locales/diaspora/en.yml | 1 + .../invitations_controller_spec.rb | 159 ++++++++++-------- .../registrations_controller_spec.rb | 8 + 5 files changed, 107 insertions(+), 75 deletions(-) diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index b7f8f9882..66653e83e 100644 --- a/app/controllers/invitations_controller.rb +++ b/app/controllers/invitations_controller.rb @@ -4,6 +4,7 @@ class InvitationsController < ApplicationController before_action :authenticate_user! + before_action :check_invitations_available!, only: :create def new @invite_code = current_user.invitation_code @@ -46,6 +47,17 @@ class InvitationsController < ApplicationController private + def check_invitations_available! + return true if AppConfig.settings.enable_registrations? || current_user.invitation_code.can_be_used? + + flash[:error] = if AppConfig.settings.invitations.open? + t("invitations.create.no_more") + else + t("invitations.create.closed") + end + redirect_to :back + end + def valid_email?(email) User.email_regexp.match(email).present? end diff --git a/app/models/invitation_code.rb b/app/models/invitation_code.rb index 10bf5c8d3..7b732359d 100644 --- a/app/models/invitation_code.rb +++ b/app/models/invitation_code.rb @@ -12,7 +12,7 @@ class InvitationCode < ActiveRecord::Base end def can_be_used? - self.count > 0 + count > 0 && AppConfig.settings.invitations.open? end def add_invites! diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index bdac8f67d..117005596 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -560,6 +560,7 @@ en: no_more: "You have no more invitations." empty: "Please enter at least one email address." note_already_sent: "Invitations have already been sent to: %{emails}" + closed: "Invitations are closed on this diaspora* pod." new: language: "Language" invite_someone_to_join: "Invite someone to join diaspora*!" diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 086c90991..00e6b2e8c 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/spec/controllers/invitations_controller_spec.rb @@ -2,131 +2,142 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -require 'spec_helper' - -describe InvitationsController, :type => :controller do - - before do - AppConfig.settings.invitations.open = true - @user = alice - @invite = {'email_inviter' => {'message' => "test", 'emails' => "abc@example.com"}} - end +require "spec_helper" +describe InvitationsController, type: :controller do describe "#create" do + let(:referer) { "http://test.host/cats/foo" } + let(:invite_params) { {email_inviter: {emails: "abc@example.com"}} } + before do - sign_in @user, scope: :user - allow(@controller).to receive(:current_user).and_return(@user) - @referer = 'http://test.host/cats/foo' - request.env["HTTP_REFERER"] = @referer + sign_in alice, scope: :user + request.env["HTTP_REFERER"] = referer end context "no emails" do - before do - @invite = {'email_inviter' => {'message' => "test", 'emails' => ""}} - end + let(:invite_params) { {email_inviter: {emails: ""}} } - it 'does not create an EmailInviter' do + it "does not create an EmailInviter" do expect(Workers::Mail::InviteEmail).not_to receive(:perform_async) - post :create, @invite + post :create, invite_params end - it 'returns to the previous page' do - post :create, @invite - expect(response).to redirect_to @referer + it "returns to the previous page" do + post :create, invite_params + expect(response).to redirect_to referer end - it 'flashes an error' do - post :create, @invite + it "flashes an error" do + post :create, invite_params expect(flash[:error]).to eq(I18n.t("invitations.create.empty")) end end - context 'only valid emails' do - before do - @emails = 'mbs@gmail.com' - @invite = {'email_inviter' => {'message' => "test", 'emails' => @emails}} + context "only valid emails" do + let(:emails) { "mbs@gmail.com" } + let(:invite_params) { {email_inviter: {emails: emails}} } + + it "creates an InviteEmail worker" do + expect(Workers::Mail::InviteEmail).to receive(:perform_async).with( + emails, alice.id, invite_params[:email_inviter] + ) + post :create, invite_params end - it 'creates an InviteEmail worker' do - inviter = double(:emails => [@emails], :send! => true) - expect(Workers::Mail::InviteEmail).to receive(:perform_async).with(@invite['email_inviter']['emails'], @user.id, @invite['email_inviter']) - post :create, @invite + it "returns to the previous page on success" do + post :create, invite_params + expect(response).to redirect_to referer end - it 'returns to the previous page on success' do - post :create, @invite - expect(response).to redirect_to @referer - end - - it 'flashes a notice' do - post :create, @invite - expected = I18n.t('invitations.create.sent', :emails => @emails.split(',').join(', ')) + it "flashes a notice" do + post :create, invite_params + expected = I18n.t("invitations.create.sent", emails: emails) expect(flash[:notice]).to eq(expected) end end - context 'only invalid emails' do - before do - @emails = 'invalid_email' - @invite = {'email_inviter' => {'message' => "test", 'emails' => @emails}} - end + context "only invalid emails" do + let(:emails) { "invalid_email" } + let(:invite_params) { {email_inviter: {emails: emails}} } - it 'does not create an InviteEmail worker' do + it "does not create an InviteEmail worker" do expect(Workers::Mail::InviteEmail).not_to receive(:perform_async) - post :create, @invite + post :create, invite_params end - it 'returns to the previous page' do - post :create, @invite - expect(response).to redirect_to @referer + it "returns to the previous page" do + post :create, invite_params + expect(response).to redirect_to referer end it "flashes an error" do - post :create, @invite + post :create, invite_params - expected = I18n.t("invitations.create.rejected", emails: @emails.split(",").join(", ")) + expected = I18n.t("invitations.create.rejected", emails: emails) expect(flash[:error]).to eq(expected) end end - context 'mixed valid and invalid emails' do - before do - @valid_emails = 'foo@bar.com,mbs@gmail.com' - @invalid_emails = 'invalid' - @invite = {'email_inviter' => {'message' => "test", 'emails' => - @valid_emails + ',' + @invalid_emails}} + context "mixed valid and invalid emails" do + let(:valid_emails) { "foo@bar.com,mbs@gmail.com" } + let(:invalid_emails) { "invalid_email" } + let(:invite_params) { {email_inviter: {emails: valid_emails + "," + invalid_emails}} } + + it "creates an InviteEmail worker" do + expect(Workers::Mail::InviteEmail).to receive(:perform_async).with( + valid_emails, alice.id, invite_params[:email_inviter] + ) + post :create, invite_params end - it 'creates an InviteEmail worker' do - inviter = double(:emails => [@emails], :send! => true) - expect(Workers::Mail::InviteEmail).to receive(:perform_async).with(@valid_emails, @user.id, @invite['email_inviter']) - post :create, @invite - end - - it 'returns to the previous page' do - post :create, @invite - expect(response).to redirect_to @referer + it "returns to the previous page" do + post :create, invite_params + expect(response).to redirect_to referer end it "flashes a notice" do - post :create, @invite - expected = I18n.t("invitations.create.sent", emails: @valid_emails.split(",").join(", ")) + ". " + - I18n.t("invitations.create.rejected", emails: @invalid_emails.split(",").join(", ")) + post :create, invite_params + expected = I18n.t("invitations.create.sent", emails: valid_emails.split(",").join(", ")) + ". " + + I18n.t("invitations.create.rejected", emails: invalid_emails) expect(flash[:error]).to eq(expected) end end - it 'redirects if invitations are closed' do - AppConfig.settings.invitations.open = false + context "with registration disabled" do + before do + AppConfig.settings.enable_registrations = false + end - post :create, @invite - expect(response).to be_redirect + it "displays an error if invitations are closed" do + AppConfig.settings.invitations.open = false + + post :create, invite_params + + expect(flash[:error]).to eq(I18n.t("invitations.create.closed")) + end + + it "displays an error when no invitations are left" do + alice.invitation_code.update_attributes(count: 0) + + post :create, invite_params + + expect(flash[:error]).to eq(I18n.t("invitations.create.no_more")) + end + end + + it "does not display an error when registration is open" do + AppConfig.settings.invitations.open = false + alice.invitation_code.update_attributes(count: 0) + + post :create, invite_params + + expect(flash[:error]).to be_nil end end describe '#new' do it 'renders' do - sign_in @user, scope: :user + sign_in alice, scope: :user get :new end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 1a7c43987..49e07fa19 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -56,6 +56,14 @@ describe RegistrationsController, type: :controller do expect(response).to redirect_to new_user_session_path end + it "does redirect when invitations are closed now" do + code = InvitationCode.create(user: bob) + AppConfig.settings.invitations.open = false + + get :new, invite: {token: code.token} + expect(response).to redirect_to new_user_session_path + end + it "does not redirect when the registration is open" do AppConfig.settings.enable_registrations = true