<p><b>@tomhughes</b> requested changes on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1701#discussion_r157600405">app/controllers/user_roles_controller.rb</a>:</p>
<pre style='color:#555'>> @@ -8,6 +8,7 @@ class UserRolesController < ApplicationController
   before_action :require_valid_role
   before_action :not_in_role, :only => [:grant]
   before_action :in_role, :only => [:revoke]
+  before_action :not_revoke_admin_current_user
</pre>
<p>This should only be applied to the <code>revoke</code> method but in fact this is so specific that I don't think there's any point making it a filter - the point of filters is that they can be shared across multiple methods but these even has the method name in it's name so clearly it only ever makes sense to apply it to revoke.</p>
<p>As such I would just put the logic in the <code>revoke</code> method and get rid of the filter.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1701#discussion_r157600523">test/controllers/user_roles_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -134,5 +134,9 @@ def test_revoke
     end
     assert_redirected_to user_path(target_user.display_name)
     assert_equal "The string `no_such_role' is not a valid role.", flash[:error]
+
+    # Revoking administrator role from current user should fail
+    post :revoke, :params => { :display_name => administrator_user.display_name, :role => "administrator" }
+    assert_redirected_to user_path(administrator_user.display_name)
</pre>
<p>Can we assert that the flash message is shown here... I think there some examples of doing it in other tests.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1701#pullrequestreview-84269604">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLQIYgFlOjUn5kxKdV2XUpgs9ignFks5tBtPagaJpZM4Q7ZaR">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABWnLb9tUG-QHxRbzQBOJAzEwrdtJHuSks5tBtPagaJpZM4Q7ZaR.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/openstreetmap/openstreetmap-website/pull/1701#pullrequestreview-84269604"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@tomhughes requested changes on #1701"}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1701#pullrequestreview-84269604"}}}</script>