Use POST for /run/delete and add confirmation form

Fixes #248.
perftools · Jan 9, 2019 · 1bc0d57 · 1bc0d57
1 parent 2965240
commit 1bc0d57
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 16 deletions.
diff --git a/src/Xhgui/Controller/Run.php b/src/Xhgui/Controller/Run.php
@@ -108,20 +108,42 @@ public function view()
         ));
     }
 
-    public function delete()
+    public function deleteForm()
     {
         $request = $this->app->request();
         $id = $request->get('id');
+        if (!is_string($id) || !strlen($id)) {
+            throw new Exception('The "id" parameter is required.');
+        }
+
+        // Get details
+        $result = $this->profiles->get($id);
+
+        $this->_template = 'runs/delete-form.twig';
+        $this->set(array(
+            'run_id' => $id,
+            'result' => $result,
+        ));
+    }
+
+    public function deleteSubmit()
+    {
+        $request = $this->app->request();
+        $id = $request->post('id');
+        // Don't call profilers->delete() unless $id is set,
+        // otherwise it will turn the null into a MongoId and return "Sucessful".
+        if (!is_string($id) || !strlen($id)) {
+            // Form checks this already,
+            // only reachable by handcrafted or malformed requests.
+            throw new Exception('The "id" parameter is required.');
+        }
 
         // Delete the profile run.
         $delete = $this->profiles->delete($id);
 
         $this->app->flash('success', 'Deleted profile ' . $id);
 
-        $referrer = $request->getReferrer();
-        // In case route is accessed directly the referrer is not set.
-        $redirect = isset($referrer) ? $referrer : $this->app->urlFor('home');
-        $this->app->redirect($redirect);
+        $this->app->redirect($this->app->urlFor('home'));
     }
 
     public function deleteAllForm()
@@ -138,10 +160,7 @@ public function deleteAllSubmit()
 
         $this->app->flash('success', 'Deleted all profiles');
 
-        $referrer = $request->getReferrer();
-        // In case route is accessed directly the referrer is not set.
-        $redirect = isset($referrer) ? $referrer : $this->app->urlFor('home');
-        $this->app->redirect($redirect);
+        $this->app->redirect($this->app->urlFor('home'));
     }
 
     public function url()

diff --git a/src/Xhgui/Profiles.php b/src/Xhgui/Profiles.php
@@ -271,7 +271,7 @@ public function insert($profile)
     /**
      * Delete a profile run.
      *
-     * @param $id The profile id to delete.
+     * @param string $id The profile id to delete.
      * @return array|bool
      */
     public function delete($id)

diff --git a/src/routes.php b/src/routes.php
@@ -31,8 +31,13 @@
 })->name('run.view');
 
 $app->get('/run/delete', function () use ($di, $app) {
-    $di['runController']->delete();
-})->name('run.delete');
+    $app->controller = $di['runController'];
+    $app->controller->deleteForm();
+})->name('run.delete.form');
+
+$app->post('/run/delete', function () use ($di, $app) {
+    $di['runController']->deleteSubmit();
+})->name('run.delete.submit');
 
 $app->get('/run/delete_all', function () use ($di, $app) {
     $app->controller = $di['runController'];

diff --git a/src/templates/runs/delete-form.twig b/src/templates/runs/delete-form.twig
@@ -0,0 +1,35 @@
+{% extends 'layout/base.twig' %}
+
+{% block title %}
+    - Delete a run
+{% endblock %}
+
+{% block content %}
+<h1>Delete a run</h1>
+
+<form class="form-stacked" action="{{ url('run.delete.submit') }}" method="post">
+    <p>Are you sure you want to delete <a href="{{ url('run.view', {'id': result.id|trim }) }}">run {{ result.id }}</a>?</p>
+    <table class="table table-hover">
+        <tr>
+            <th class="span2">Method</th>
+            <td>{{result.meta('SERVER.REQUEST_METHOD')}}</td>
+        </tr>
+        <tr>
+            <th class="span2">URL</th>
+            <td>
+                {% set addr = result.meta('SERVER.SERVER_NAME') ~ result.meta.url %}
+                {{ addr }}
+            </td>
+        </tr>
+        <tr>
+            <th class="span2">Time</th>
+            <td>{{ result.date|date(date_format) }}</td>
+        </tr>
+    </table>
+    <div class="form-actions">
+        <input type="hidden" name="id" value="{{ run_id }}" />
+        <input class="btn btn-large btn-danger" type="submit" value="Delete run" />
+    </div>
+</form>
+
+{% endblock %}
diff --git a/src/templates/runs/paginated-list.twig b/src/templates/runs/paginated-list.twig
@@ -73,7 +73,7 @@
             <td class="right">{{ result.get('main()', 'mu') |as_bytes }}</td>
             <td class="right">{{ result.get('main()', 'pmu') |as_bytes }}</td>
             <td>
-                <a href="{{ url('run.delete', {'id': result.id|trim }) }}">
+                <a href="{{ url('run.delete.form', {'id': result.id|trim }) }}">
                     <i class="icon-trash"></i>
                 </a>
             </td>

diff --git a/tests/Controller/RunTest.php b/tests/Controller/RunTest.php
@@ -167,14 +167,17 @@ public function testCallgraphData()
         $this->assertStringStartsWith('{"', $response->body());
     }
 
-    public function testDelete()
+    public function testDeleteSubmit()
     {
         loadFixture($this->profiles, XHGUI_ROOT_DIR . '/tests/fixtures/results.json');
 
         Environment::mock(array(
+            'REQUEST_METHOD' => 'POST',
             'SCRIPT_NAME' => 'index.php',
             'PATH_INFO' => '/run/delete',
-            'QUERY_STRING' => 'id=aaaaaaaaaaaaaaaaaaaaaaaa',
+            'slim.request.form_hash' => [
+                'id' => 'aaaaaaaaaaaaaaaaaaaaaaaa',
+            ],
         ));
 
         $this->app->expects($this->once())
@@ -187,7 +190,7 @@ public function testDelete()
         $result = $this->profiles->getAll();
         $this->assertCount(5, $result['results']);
 
-        $this->runs->delete();
+        $this->runs->deleteSubmit();
 
         $result = $this->profiles->getAll();
         $this->assertCount(4, $result['results']);