Ruby tutorial Ch9 Упражнение № 9 - Не позволяйте Администратору удалять себя

Question

Я абсолютный новичок в Ruby и Rails, поэтому я изучаю Rails Tutorial Майкла Хартла. Я застрял в главе 9, упражнение № 9. Я обновил код def destroy в Users Controller для:

def destroy
  user = User.find(params[:id])
  if (current_user == user) && (current_user.admin?)
    flash[:error] = "Can not delete own admin account!"
  else
    user.destroy
    flash[:success] = "User destroyed."
  end
redirect_to users_path
end

Кажется, это работает, когда я тестирую в браузере, добавляя ссылку «удалить» к current_user, когда администратор вошел в систему. Но в упражнении говорится сначала написать тест - что я и сделал, но, похоже, не работает. Вот что у меня есть для теста:

describe "as admin user" do
  let(:user_admin) { FactoryGirl.create(:admin) }

  before { sign_in user_admin }

  describe "submitting a DELETE request to destroy own admin account" do
    before { delete user_path(user_admin) }
    it { should have_selector('div.alert.alert-error', text: 'delete own admin') }
  end
end

Может быть, то, что я тестирую, не должно быть проверено. Как вы тестируете модификацию кода def destroy в Users Controller?

Answer 1

Я пытался заставить код в исходном посте работать, но безуспешно. Вместо этого я заставил это работать как это (заставил это терпеть неудачу, затем передать). Это проверяет правильность перенаправления, а также правильное флэш-сообщение.

ТЕСТ: authentication_pages_spec.rb

  describe "as admin user" do
    let(:admin) { FactoryGirl.create(:admin) }
    before { sign_in admin }

    describe "can't delete self by submitting DELETE request to Users#destroy" do
      before { delete user_path(admin) }
      specify { response.should redirect_to(users_path), 
                  flash[:error].should =~ /Can not delete own admin account!/i }
    end
  end

ОСУЩЕСТВЛЕНИЕ: Пользователи # уничтожить

def destroy
    user = User.find(params[:id])
    if (current_user == user) && (current_user.admin?)
      flash[:error] = "Can not delete own admin account!"
    else
      user.destroy
      flash[:success] = "User destroyed."
    end
  redirect_to users_path
  end

Возможно, причина в том, что оригинальный тест не работал, в том, как мы выдаем запрос? Я попытался добавить каждый из следующих элементов по отдельности в блок описания, но все не получилось:

it { should have_selector('div.alert.alert-error', text: 'delete own admin') }

it { should have_selector('title', text: 'All users') }
it { should have_selector('h1', text: 'All users') }

Итак, похоже, что Capybara на самом деле не перенаправляет на страницу для проверки этих селекторов. Я пробовал «title» и «h1», думая, что, возможно, возникла какая-то проблема с селектором «div.alert.alert-error» ... но «title» и «h1» потерпели неудачу с тем же «ожидаемым CSS, чтобы вернуть что-то» ...

Кто-нибудь знает больше о том, как работают specify { response.should ... } тесты стиля? Если они не следуют перенаправлениям, когда они нажимают на действие контроллера?

Answer 2

Стоит также отметить, что в учебнике в листинге 9.43 частичное представление в app / views / users / _user.html.erb имеет проверку, чтобы не показывать ссылку «удалить» для текущего авторизованный администратор на странице индекса пользователей.

Таким образом, даже если пользователь не может удалить свою учетную запись через веб-интерфейс, я полагаю, что упражнение 9.9 идет еще дальше, обеспечивая также наличие логики на уровне контроллера на случай, если кто-то создаст и отправит запрос на удаление http для текущего пользователя.

Вероятно, это хорошая практика - добавлять эти защитные устройства в ваши приложения на rails, чтобы предотвратить появление каких-либо странных ошибок.

Поскольку фильтр в частичном представлении делает его таким, что вы все равно не увидите, что ошибка мигает, вы также можете упростить действие уничтожения контроллера пользователя, удалив 'else'.

def destroy
  user = User.find(params[:id])
  unless current_user?(user)
    user.destroy
    flash[:success] = "User deleted."
  end
  redirect_to users_url
end

Answer 3

Я также новичок в Rails Tutorial (и Rails в целом), и у меня возникла та же проблема, и ваш вопрос помог мне выяснить ответ.

Я до сих пор не уверен, почему ваш код не работает, точно, но следующие шаги определенно сработали.

Во-первых, немного измените тестовый код, чтобы использовать следующую структуру (здесь я не указал, где разместить этот блок описания - у вас уже есть правильное место):

describe "deleting herself" do
  it "should not be possible" do
    expect { delete user_path(admin) }.to_not change(User, :count).by(-1)
  end
end

Обратите внимание, что для отслеживания количества объектов User я использую блок автомата {}.Это определенно приводит к тому, что тесты становятся красными (что хорошо в данный момент), тогда как проверка на Flash также делает тест красным, но проверка на наличие ошибки Flash здесь не работает.Я действительно не знаю почему!Может быть, что-то делать с двойным перенаправлением, которое происходит?

Далее, напишите код защиты, чтобы тесты снова стали зелеными.Ваш код работает (я думаю), но я думаю, что мой код немного более идиоматичен в том смысле, что он использует помощники сеансов, определенные ранее в главе 9.

def destroy
  user = User.find(params[:id])
  if (current_user? user) && (current_user.admin?)
    flash[:error] = "You are not allowed to delete yourself as an admin."
  else
    user.destroy
    flash[:success] = "User destroyed. ID: #{user.id}"
  end
  redirect_to users_path
end

Это изменение сделало мой тест снова «зеленым», которая успешно выполняет упражнение 10.

Answer 4

Действие контроллера

def destroy
  usertodestroy = User.find(params[:id])
  if (current_user == usertodestroy)
    flash[:error] = 'Can´t delete own user'
    redirect_to root_url
  else
    usertodestroy.destroy
    flash[:success] = "User destroyed. ID: #{usertodestroy.name}"
    redirect_to users_url
  end
end

и тест

describe "as admin user" do
  let(:admin) { FactoryGirl.create(:admin) }
  before { sign_in admin, no_capybara: true }

  it "attempting to delete self" do
    expect{ delete user_path(admin) }.not_to change(User, :count)
  end
end

работа для меня.

Answer 5

Я применил предложения eblume навсегда, хотя у меня есть некоторые замечания и сомнения:

Во-первых, тест можно было бы упростить, поскольку нам не нужно проверять, изменяется ли число на одну единицу, но если оно изменяется на любое число:

expect { delete user_path(admin) }.not_to change(User, :count)

Что касается кода в контроллере, его также можно упростить. Как мы уже кодировали перед действием для уничтожения:

def admin_user
    redirect_to(root_url) unless current_user.admin?
end

Нет необходимости проверять в методе 'destroy', является ли пользователь администратором, он должен быть администратором.

Таким образом, условие if становится:

if (current_user? user)

Моя проблема сейчас: я не понимаю этот код, я не знаю, что делает эта проверка.

Моя первая попытка использовала следующее:

if (current_user.id == params[:id])

Но это не работает, я не понимаю, почему.

Answer 6

Несмотря на следующие рекомендации, мой тест был все еще не пройден, с ошибкой:

undefined method `admin?' for nil:NilClass

Это означает, что при входе в систему возникли некоторые проблемы, поскольку она вызывается только как часть проверки 'admin_user' before_filter.

Мне удалось решить эту проблему, используя не-капибарскую версию метода входа в систему

before { signin admin, no_capybara: true }

Спасибо!

Answer 7

Я также новичок в Rails, просто делаю учебник впервые, ваши посты мне очень помогают, но просто для того, чтобы внести свой вклад, вам действительно не нужно проверять, является ли пользователь администратором в разрушении как уничтожение будет доступен только администратору при добавлении строки

before_action :admin_user,     only: :destroy

В пользовательском контроллере.

Так что достаточно просто спросить, не совпадает ли с текущим пользователем

def destroy
  usertodestroy = User.find(params[:id])
  if (current_user == usertodestroy)
    flash[:error] = 'Can´t delete own user'
  else
    usertodestroy.destroy
    flash[:success] = "User destroyed. ID: #{usertodestroy.name}"
    redirect_to users_url
  end
end

Также тест должен просто спросить, что число не изменилось после попытки удалить

describe "as admin user" do
  let(:admin) { FactoryGirl.create(:admin) }
  before { sign_in(admin) }

  it "should not be able to delete itself" do
    expect { delete user_path(admin)  }.not_to change(User, :count) 
  end
end

Оба не изменяют результаты, но они просто упрощают процесс.

Почему я не смог понять, почему следующий код в любом случае удаляет пользователя при тестировании:

describe "as admin user" do
  let(:admin) { FactoryGirl.create(:admin) }
  before { sign_in(admin) }

  it "should not be able to delete itself" do
    expect { admin.destroy  }.not_to change(User, :count) 
  end
end

По-моему, это прямой вызов UsersController, поэтому его не следует удалять.