Skip to content

Bug delete user #398 #236

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.

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

Merged
merged 1 commit into from
Nov 3, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,12 @@ class User < ActiveRecord::Base
has_one :github_profile , class_name: 'Users::Github::Profile', dependent: :destroy
has_many :github_repositories, through: :github_profile , source: :repositories


geocoded_by :location, latitude: :lat, longitude: :lng, country: :country, state_code: :state_name
# FIXME: Move to background job
after_validation :geocode_location, if: :location_changed? unless Rails.env.test?
Copy link

Choose a reason for hiding this comment

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

From my experience it can lead to a hard debuggable issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

It fine he just moved it from below. we should however remove it.
@YaroSpace , could you add a comment on top of that line # FIXME: Move to background job


before_destroy ->{ protips.destroy_all }, prepend: true

def near
User.near([lat, lng])
end
Expand Down Expand Up @@ -130,7 +132,6 @@ def self.with_username(username, provider = :username)
where(["UPPER(#{sql_injection_safe_where_clause}) = UPPER(?)", username]).first
end


# Todo State machine
def banned?
banned_at.present?
Expand Down Expand Up @@ -923,7 +924,6 @@ def make_referral_token
end

after_save :refresh_dependencies
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Triggering :refresh_protips was pointless, since all the associated protips are already deleted by this time, besides it is Protip's responsibility to refresh its index.

after_destroy :refresh_protips

def refresh_dependencies
if username_changed? or avatar_changed? or team_document_id_changed?
Expand Down
2 changes: 2 additions & 0 deletions spec/features/helpers/general_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ def login_as(settings = {})
fill_in 'user_location', with: settings[:location]
click_button 'Finish'
end

user
end

def create_team(name = 'TEST_TEAM')
Expand Down
58 changes: 58 additions & 0 deletions spec/features/users/user_management_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
require "rails_helper"

feature "User management", js: true do
describe 'deleting a user' do
before do
stub_request(:post, /api.mixpanel.com/)
end

let!(:user) { login_as(username: 'alice', bypass_ui_login: true) }

scenario 'user is presented with confirmation dialog when deletes his account' do
visit '/settings'
find('.delete').click_link 'click here.'

expect(page).to have_content 'Warning: clicking this link below will permenatly delete your Coderwall account and its data.'
expect(page).to have_button 'Delete your account & sign out'
end

scenario 'user is redirected to /welcome after deleting hios account' do
visit '/settings'
find('.delete').click_link 'click here.'
find('.save').click_button 'Delete your account & sign out'

expect(current_path).to eq('/welcome')
end

scenario 'user cannot login after deleting his account' do
visit '/settings'
find('.delete').click_link 'click here.'
find('.save').click_button 'Delete your account & sign out'

visit "/auth/developer"
fill_in 'name', with: user.username
fill_in 'email', with: user.email
click_button 'Sign In'

expect(current_path).to eq(new_user_path)
end

scenario 'users protips are not displayed after he deletes his account' do
Protip.rebuild_index
protip_1, protip_2 = Fabricate.times(2, :protip, user: user)
protip_3 = Fabricate(:protip)

visit '/settings'
find('.delete').click_link 'click here.'
find('.save').click_button 'Delete your account & sign out'

login_as(username: 'bob', bypass_ui_login: true)
visit '/p/fresh'

expect(page).not_to have_content(protip_1.title)
expect(page).not_to have_content(protip_2.title)
expect(page).to have_content(protip_3.title)
end
end

end
9 changes: 9 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,16 @@ class AlsoNotaBadge < BadgeBase
it "should not default to banned" do
expect(user.banned?).to eq(false)
end
end

describe 'deleting a user', focus: true do
it 'deletes asosciated prtotips' do
user = Fabricate(:user)
protip = Fabricate(:protip, user: user)

expect(user.reload.protips).to receive(:destroy_all).and_return(false)
user.destroy
end
end

end
Expand Down
20 changes: 20 additions & 0 deletions spec/requests/users_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
RSpec.describe "User management", :type => :request do

describe 'deleting a user' do
it 'deletes associated protips and reindex search index' do
user = Fabricate(:user)

Protip.rebuild_index
protip_1, protip_2 = Fabricate.times(2, :protip, user: user)
protip_3 = Fabricate(:protip)

user.reload.destroy
search = Protip.search('*').map(&:title)

expect(search).not_to include(protip_1.title)
expect(search).not_to include(protip_2.title)
expect(search).to include(protip_3.title)
end
end

end