<p>Thanks! It looks good. I would ask you to make two more changes:</p>
<ol>
<li>In the test, you have</li>
</ol>
<blockquote>
<p>post :delete, :params => { :display_name => <strong>admin</strong>.display_name, :id => public_trace_file.id }, :session => { :user => admin }</p>
</blockquote>
<p>The <code>display_name</code> parameter (i.e. the name used in the URL) should be for the user of the trace, not the admin doing the deleting. I know that the <code>display_name</code> is currently ignored by the code, but it's best to change it to:</p>
<blockquote>
<p>post :delete, :params => { :display_name => <strong>public_trace_file.user</strong>.display_name, :id => public_trace_file.id }, :session => { :user => admin }</p>
</blockquote>
<ol start="2">
<li>When an admin or moderator deletes a trace, I think it is best for them to be redirected to the list of traces for the user that owned that trace, rather than redirecting the admin to their own list of traces. This can be achieved by changing (in app/controllers/trace_controller.rb)</li>
</ol>
<blockquote>
<p>redirect_to :action => :list, :display_name => <strong>current_user</strong>.display_name</p>
</blockquote>
<p>to</p>
<blockquote>
<p>redirect_to :action => :list, :display_name => <strong>trace.user</strong>.display_name</p>
</blockquote>
<p>and adjusting the test accordingly.</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/1765#issuecomment-368379799">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLS9BO3B26XaLTgxmS9QXOWIaIt_fks5tYiQ8gaJpZM4SR6-k">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABWnLfZW3FPLjHWEypYbr64SlOOWDAOXks5tYiQ8gaJpZM4SR6-k.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/1765#issuecomment-368379799"></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":"@gravitystorm in #1765: Thanks! It looks good. I would ask you to make two more changes:\r\n\r\n1) In the test, you have\r\n\r\n\u003e post :delete, :params =\u003e { :display_name =\u003e **admin**.display_name, :id =\u003e public_trace_file.id }, :session =\u003e { :user =\u003e admin }\r\n\r\nThe `display_name` parameter (i.e. the name used in the URL) should be for the user of the trace, not the admin doing the deleting. I know that the `display_name` is currently ignored by the code, but it's best to change it to: \r\n\r\n\u003e post :delete, :params =\u003e { :display_name =\u003e **public_trace_file.user**.display_name, :id =\u003e public_trace_file.id }, :session =\u003e { :user =\u003e admin }\r\n\r\n2) When an admin or moderator deletes a trace, I think it is best for them to be redirected to the list of traces for the user that owned that trace, rather than redirecting the admin to their own list of traces. This can be achieved by changing (in app/controllers/trace_controller.rb)\r\n\r\n\u003e  redirect_to :action =\u003e :list, :display_name =\u003e **current_user**.display_name\r\n\r\nto \r\n\r\n\u003e redirect_to :action =\u003e :list, :display_name =\u003e **trace.user**.display_name\r\n\r\nand adjusting the test accordingly."}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1765#issuecomment-368379799"}}}</script>