Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds invitations_counter cache for triggering the number of invitations ... #425

Merged
merged 1 commit into from Jan 30, 2014
Merged

Adds invitations_counter cache for triggering the number of invitations ... #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/devise_invitable/model.rb
View file
Open in desktop
Expand Up @@ -30,9 +30,9 @@ module Invitable
included do
include ::DeviseInvitable::Inviter
if Devise.invited_by_class_name
belongs_to :invited_by, :class_name => Devise.invited_by_class_name
belongs_to :invited_by, :class_name => Devise.invited_by_class_name, :counter_cache => :invitations_count
else
belongs_to :invited_by, :polymorphic => true
belongs_to :invited_by, :polymorphic => true, :counter_cache => :invitations_count
end

include ActiveSupport::Callbacks
Expand Down
2 changes: 2 additions & 0 deletions lib/generators/active_record/templates/migration.rb
View file
Open in desktop
Expand Up @@ -7,6 +7,8 @@ def up
t.datetime :invitation_accepted_at
t.integer :invitation_limit
t.references :invited_by, :polymorphic => true
t.integer :invitations_count, default: 0
t.index :invitations_count
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need an index on invitations_count? Do you query users somewhere based only on invitation counts? And even if you do, the index doesn't seem selective enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I queried users based on that and displaying it on a view. I made it as a preventive matter if I recall correctly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt if the majority of people use it for that purpose (some kind of leaderboard), and it would lie as an unused index in their table. I personally think that this index does not belong to the gem migration but should be added separately (in their own migration) if someone needs that kind of functionality.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be right, and probably is a better idea to have the index on a separate migration file, in case someone needs it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a PR: #830

t.index :invitation_token, :unique => true # for invitable
t.index :invited_by_id
end
Expand Down
8 changes: 8 additions & 0 deletions test/models/invitable_test.rb
View file
Open in desktop
Expand Up @@ -12,6 +12,14 @@ def setup
end

test 'should not generate the raw invitation token after creating a record' do
current_user = new_user
2.times do |index|
User.invite!({:email => "valid#{index}@email.com"}, current_user)
end
assert_equal current_user.reload.invitations_count, 2
end

test 'should update the invitations count counter cache' do
assert_nil new_user.raw_invitation_token
end

Expand Down
5 changes: 5 additions & 0 deletions test/rails_app/db/migrate/20100401102949_create_tables.rb
View file
Open in desktop
Expand Up @@ -27,15 +27,20 @@ def change
t.integer :invitation_limit
t.integer :invited_by_id
t.string :invited_by_type
t.integer :invitations_count, :default => 0

t.timestamps
end
add_index :users, :invitation_token, :unique => true
add_index :users, :invitations_count

create_table :admins do |t|
## Database authenticatable
t.string :email, :null => true, :default => ""
t.string :encrypted_password, :null => true, :default => ""

t.integer :invitations_count, :default => 0
end
add_index :admins, :invitations_count
end
end