Improve Package Details UX (#298)

* Show Package Actions dropdown on every package page * Move Package List icon to the left of the header bar Link back to the package details from each Action page * Show Installs & Security on Package Details Format install counts * Fix tests Switch to extracting the text via Symfony, as that deals with HTML tags & whitespace
repman-io · Oct 15, 2020 · c43aacd · c43aacd
1 parent f4de6b5
commit c43aacd
Show file tree

Hide file tree

Showing 17 changed files with 192 additions and 84 deletions.
diff --git a/src/Controller/OrganizationController.php b/src/Controller/OrganizationController.php
@@ -131,6 +131,7 @@ public function packageDetails(Organization $organization, Package $package, Req
             'filter' => $filter,
             'count' => $this->packageQuery->versionCount($package->id()),
             'versions' => $this->packageQuery->getVersions($package->id(), $filter),
+            'installs' => $this->packageQuery->getInstalls($package->id(), 0),
         ]);
     }
 

diff --git a/src/Query/User/PackageQuery/DbalPackageQuery.php b/src/Query/User/PackageQuery/DbalPackageQuery.php
@@ -116,7 +116,22 @@ public function count(string $organizationId, Filter $filter): int
     public function getById(string $id): Option
     {
         $data = $this->connection->fetchAssoc(
-            'SELECT id, organization_id, type, repository_url, name, latest_released_version, latest_release_date, description, last_sync_at, last_sync_error, webhook_created_at, keep_last_releases
+            'SELECT
+                id,
+                organization_id,
+                type,
+                repository_url,
+                name,
+                latest_released_version,
+                latest_release_date,
+                description,
+                last_sync_at,
+                last_sync_error,
+                webhook_created_at,
+                last_scan_date,
+                last_scan_status,
+                last_scan_result,
+                keep_last_releases
             FROM "organization_package"
             WHERE id = :id', [
             ':id' => $id,

diff --git a/templates/admin/proxy/dist.html.twig b/templates/admin/proxy/dist.html.twig
@@ -17,7 +17,7 @@
         {% for package in packages %}
             <tr>
                 <td>{{ package }}</td>
-                <td>{{ downloads[package].downloads ?? 0 }}</td>
+                <td class="number-format">{{ downloads[package].downloads ?? 0 }}</td>
                 <td>
                     {% if downloads[package] is defined %}
                         <span data-toggle="tooltip" title="{{ downloads[package].lastDownload|date_time }} ({{ gmt_offset() }})">

diff --git a/templates/base.html.twig b/templates/base.html.twig
@@ -106,7 +106,7 @@
                                         <h3 class="card-title flex-grow-1">
                                             {% block header %}{% endblock %}
                                         </h3>
-                                        <div class="">
+                                        <div class="d-flex">
                                             {% block header_btn %}{% endblock %}
                                         </div>
                                     </div>

diff --git a/templates/component/js/base.js b/templates/component/js/base.js
@@ -28,4 +28,8 @@
     });
 
     $('[data-toggle="tooltip"]').tooltip();
+
+    $('.number-format').each(function() {
+        $(this).text(parseInt($(this).text()).toLocaleString());
+    });
 })();
diff --git a/templates/component/js/stats.js.twig b/templates/component/js/stats.js.twig
@@ -35,6 +35,13 @@
                 xaxis: {
                     type: 'datetime',
                 },
+                yaxis: {
+                    labels: {
+                        formatter: function (value) {
+                            return value.toLocaleString();
+                        },
+                    },
+                },
                 labels: [{{ installs.days | map(day => "'#{day.date}'") | join(',') | raw }}],
                 colors: ["#206bc4"],
                 legend: {

diff --git a/templates/component/js/statsVersions.js.twig b/templates/component/js/statsVersions.js.twig
@@ -28,6 +28,13 @@
         xaxis: {
             type: 'datetime',
         },
+        yaxis: {
+            labels: {
+                formatter: function (value) {
+                    return value.toLocaleString();
+                },
+            },
+        },
         colors: ["#206bc4"],
         noData: {
             text: 'Loading...'

diff --git a/templates/component/packageActions.html.twig b/templates/component/packageActions.html.twig
@@ -0,0 +1,56 @@
+{% set currentPath = app.request.get('_route') %}
+<span class="dropdown ml-1">
+    <button class="btn btn-secondary dropdown-toggle" data-toggle="dropdown">Actions</button>
+    <div class="dropdown-menu">
+    {% if package.name %}
+        {% if currentPath != 'organization_package_details' %}
+            <a href="{{ path('organization_package_details', {organization: organization.alias, package: package.id}) }}" class="dropdown-item">
+                {% include 'svg/package.svg' %} Details
+            </a>
+        {% endif %}
+        {% if currentPath != 'organization_package_stats' %}
+            <a href="{{ path('organization_package_stats', {organization: organization.alias, package: package.id}) }}" class="dropdown-item">
+                {% include 'svg/bar-chart.svg' %} Statistics
+            </a>
+        {% endif %}
+        {% if currentPath != 'organization_package_webhook' %}
+            <a href="{{ path('organization_package_webhook', {organization: organization.alias, package: package.id}) }}" class="dropdown-item">
+                {% include 'svg/link.svg' %} Webhook
+            </a>
+        {% endif %}
+    {% endif %}
+        {% if package.isSynchronizedSuccessfully %}
+            <a
+                    class="dropdown-item"
+                    data-target="confirmation"
+                    data-action="{{ path('organization_package_scan', {organization: organization.alias, package: package.id }) }}"
+                    data-method="POST"
+                    href="#"
+            >
+                {% include 'svg/shield.svg' %} Scan
+        </a>
+        {% endif %}
+        {% if package.name or package.lastSyncError %}
+            <a
+                    class="dropdown-item"
+                    data-target="confirmation"
+                    data-action="{{ path('organization_package_update', {organization: organization.alias, package: package.id }) }}"
+                    data-method="POST"
+                    href="#"
+            >
+                {% include 'svg/refresh.svg' %} Update
+            </a>
+            {% if is_granted('ROLE_ORGANIZATION_OWNER', organization) %}
+                <a
+                        class="dropdown-item"
+                        data-target="confirmation"
+                        data-action="{{ path('organization_package_remove', {organization: organization.alias, package: package.id }) }}"
+                        data-method="DELETE"
+                        href="#"
+                >
+                        {% include 'svg/trash.svg' %} Remove
+                </a>
+            {% endif %}
+        {% endif %}
+    </div>
+</span>
diff --git a/templates/organization/package/details.html.twig b/templates/organization/package/details.html.twig
@@ -1,10 +1,16 @@
 {% extends 'base.html.twig' %}
 
-{% block header %}{{ package.name }} details{% endblock %}
-{% block header_btn %}
-    <a href="{{ path('organization_packages', {"organization":organization.alias}) }}" class="btn btn-primary">
-        {% include 'svg/package.svg' %} Package list
+{% block header %}
+    <a href="{{ path('organization_packages', {"organization":organization.alias}) }}"
+       class="btn btn-secondary px-1 mr-1" title="Package List">
+        <span class="text-muted mr-1">&laquo;</span>
+        {% include 'svg/package.svg' %}
     </a>
+
+    {{ package.name }} details
+{% endblock %}
+{% block header_btn %}
+    {% include 'component/packageActions.html.twig' %}
 {% endblock %}
 
 {% block content %}
@@ -21,15 +27,46 @@
             </div>
         </div>
 
+        <div class="row">
+            <div class="col-6">
+                <h4>Total Installs</h4>
+                <p>
+                    <a href="{{ path('organization_package_stats', {organization: organization.alias, package: package.id}) }}"
+                       class="number-format">
+                        {{ installs.total }}
+                    </a>
+                </p>
+            </div>
+            <div class="col-6">
+                <h4>Security</h4>
+                <p>
+                    <a
+                            href="{{ path('organization_package_scan_results', {organization: organization.alias, package: package.id}) }}"
+                            data-content="{% apply escape %}{% include 'organization/package/scanResultContent.html.twig' with {'result': package.lastScanResultContent, 'simple': true} %}{% endapply %}"
+                            data-data-toggle="tooltip" title="{{ package.scanResultDate|date_time|time_diff }}"
+                            data-toggle="scan-result-popover"
+                    >
+                        {% if package.isScanResultOk %}
+                            <span class="badge badge-success">{{ package.scanResultStatus }}</span>
+                        {% elseif package.isScanResultPending or package.isScanResultNotAvailable %}
+                            <span class="badge badge-warning">{{ package.scanResultStatus }}</span>
+                        {% else %}
+                            <span class="badge badge-danger">{{ package.scanResultStatus }}</span>
+                        {% endif %}
+                    </a>
+                </p>
+            </div>
+        </div>
+
         {% if package.latestReleaseDate %}
             <div class="row">
                 <div class="col-6">
                     <h4>Latest version</h4>
                     <p>
                         {{ package.latestReleasedVersion }} (released:
-                        <span data-toggle="tooltip" title="{{ package.latestReleaseDate|date_time_utc }} ({{ gmt_offset() }})">
-                            {{ package.latestReleaseDate|date_time_utc|time_diff }}
-                        </span>)
+                        <span data-toggle="tooltip"
+                              title="{{ package.latestReleaseDate|date_time_utc }} ({{ gmt_offset() }})">
+                            {{ package.latestReleaseDate|date_time_utc|time_diff }}</span>)
                     </p>
                 </div>
                 <div class="col-6">

diff --git a/templates/organization/package/scanResults.html.twig b/templates/organization/package/scanResults.html.twig
@@ -1,10 +1,18 @@
 {% extends 'base.html.twig' %}
 
-{% block header %}Package: {{ package.name }} security scan results{% endblock %}
+{% block header %}
+    <a href="{{ path('organization_packages', {"organization":organization.alias}) }}"
+       class="btn btn-secondary px-1 mr-1" title="Package List">
+        <span class="text-muted mr-1">&laquo;</span>
+        {% include 'svg/package.svg' %}
+    </a>
+
+    <a href="{{ path('organization_package_details', {organization: organization.alias, package: package.id}) }}">{{ package.name }}</a>
+    security scan results
+{% endblock %}
+
 {% block header_btn %}
-<a href="{{ path('organization_packages', {"organization":organization.alias}) }}" class="btn btn-primary">
-    {% include 'svg/package.svg' %} Package list
-</a>
+    {% include 'component/packageActions.html.twig' %}
 {% endblock %}
 
 {% block content %}

diff --git a/templates/organization/package/stats.html.twig b/templates/organization/package/stats.html.twig
@@ -1,6 +1,20 @@
 {% extends 'organization/stats.html.twig' %}
 
-{% block header %}{{ package.name }} installs (last {{ days }} days){% endblock %}
+{% block header %}
+    <a href="{{ path('organization_packages', {"organization":organization.alias}) }}"
+       class="btn btn-secondary px-1 mr-1" title="Package List">
+        <span class="text-muted mr-1">&laquo;</span>
+        {% include 'svg/package.svg' %}
+    </a>
+
+    <a href="{{ path('organization_package_details', {organization: organization.alias, package: package.id}) }}">{{ package.name }}</a>
+    installs (last {{ days }} days)
+{% endblock %}
+
+{% block header_btn %}
+    {{ parent() }}
+    {% include 'component/packageActions.html.twig' %}
+{% endblock %}
 
 {% block content %}
     <h3>Daily installs</h3>
@@ -10,9 +24,9 @@
 
     <h3>Summary</h3>
     <p>
-        <span class="mr-5">Today installs: {{ (installs.days | last).installs }}</span>
-        <span class="mr-5">Last {{ days ?? 30 }} days installs: {{ installs.daysTotal }}</span>
-        <span class="mr-5"><strong>Total installs: {{ installs.total }}</strong></span>
+        <span class="mr-5">Today installs: <span class="number-format">{{ (installs.days | last).installs }}</span></span>
+        <span class="mr-5">Last {{ days ?? 30 }} days installs: <span class="number-format">{{ installs.daysTotal }}</span></span>
+        <span class="mr-5"><strong>Total installs: <span class="number-format">{{ installs.total }}</span></strong></span>
     </p>
 
     <hr />

diff --git a/templates/organization/package/webhook.html.twig b/templates/organization/package/webhook.html.twig
@@ -1,10 +1,17 @@
 {% extends 'base.html.twig' %}
 
-{% block header %}Package: {{ package.name }} webhook{% endblock %}
-{% block header_btn %}
-    <a href="{{ path('organization_packages', {"organization":organization.alias}) }}" class="btn btn-primary">
-        {% include 'svg/package.svg' %} Package list
+{% block header %}
+    <a href="{{ path('organization_packages', {"organization":organization.alias}) }}"
+       class="btn btn-secondary px-1 mr-1" title="Package List">
+        <span class="text-muted mr-1">&laquo;</span>
+        {% include 'svg/package.svg' %}
     </a>
+
+    <a href="{{ path('organization_package_details', {organization: organization.alias, package: package.id}) }}">{{ package.name }}</a>
+    webhook
+{% endblock %}
+{% block header_btn %}
+    {% include 'component/packageActions.html.twig' %}
 {% endblock %}
 
 {% block content %}

diff --git a/templates/organization/packages.html.twig b/templates/organization/packages.html.twig
@@ -111,55 +111,7 @@
                             {% endif %}
                         </td>
                         <td>
-                            <span class="dropdown ml-1">
-                                <button class="btn btn-secondary dropdown-toggle align-text-top" data-toggle="dropdown">Actions</button>
-                                <div class="dropdown-menu">
-                                {% if package.name %}
-                                    <a href="{{ path('organization_package_details', {organization: organization.alias, package: package.id}) }}" class="dropdown-item">
-                                        {% include 'svg/package.svg' %} Details
-                                    </a>
-                                    <a href="{{ path('organization_package_stats', {organization: organization.alias, package: package.id}) }}" class="dropdown-item">
-                                        {% include 'svg/bar-chart.svg' %} Statistics
-                                    </a>
-                                    <a href="{{ path('organization_package_webhook', {organization: organization.alias, package: package.id}) }}" class="dropdown-item">
-                                        {% include 'svg/link.svg' %} Webhook
-                                    </a>
-                                {% endif %}
-                                {% if package.isSynchronizedSuccessfully %}
-                                    <a
-                                        class="dropdown-item"
-                                        data-target="confirmation"
-                                        data-action="{{ path('organization_package_scan', {organization: organization.alias, package: package.id }) }}"
-                                        data-method="POST"
-                                        href="#"
-                                        >
-                                            {% include 'svg/shield.svg' %} Scan
-                                    </a>
-                                {% endif %}
-                                {% if package.name or package.lastSyncError %}
-                                    <a
-                                        class="dropdown-item"
-                                        data-target="confirmation"
-                                        data-action="{{ path('organization_package_update', {organization: organization.alias, package: package.id }) }}"
-                                        data-method="POST"
-                                        href="#"
-                                        >
-                                            {% include 'svg/refresh.svg' %} Update
-                                    </a>
-                                    {% if is_granted('ROLE_ORGANIZATION_OWNER', organization) %}
-                                        <a
-                                            class="dropdown-item"
-                                            data-target="confirmation"
-                                            data-action="{{ path('organization_package_remove', {organization: organization.alias, package: package.id }) }}"
-                                            data-method="DELETE"
-                                            href="#"
-                                            >
-                                                {% include 'svg/trash.svg' %} Remove
-                                        </a>
-                                    {% endif %}
-                                {% endif %}
-                                </div>
-                            </span>
+                            {% include 'component/packageActions.html.twig' %}
                         </td>
                     {% endif %}
                 </tr>

diff --git a/templates/organization/stats.html.twig b/templates/organization/stats.html.twig
@@ -20,9 +20,9 @@
 
     <h3>Summary:</h3>
     <p>
-        Today installs: {{ (installs.days | last).installs }}<br />
-        Last {{ days ?? 30 }} days installs: {{ installs.daysTotal }}<br />
-        <strong>Total installs: {{ installs.total }}</strong>
+        Today installs: <span class="number-format">{{ (installs.days | last).installs }}</span><br />
+        Last {{ days ?? 30 }} days installs: <span class="number-format">{{ installs.daysTotal }}</span><br />
+        <strong>Total installs: <span class="number-format">{{ installs.total }}</span></strong>
     </p>
 {% endblock %}
 

diff --git a/tests/Functional/Controller/Admin/OrganizationControllerTest.php b/tests/Functional/Controller/Admin/OrganizationControllerTest.php
@@ -44,9 +44,9 @@ public function testStats(): void
         $orgId = $this->fixtures->createOrganization('buddy', $this->userId);
         $packageId = $this->fixtures->addPackage($orgId, 'https://some.url');
         $this->fixtures->addPackageDownload(1, $packageId);
-        $this->client->request('GET', $this->urlTo('admin_stats'));
+        $crawler = $this->client->request('GET', $this->urlTo('admin_stats'));
 
         self::assertTrue($this->client->getResponse()->isOk());
-        self::assertStringContainsString('Total installs: 1', $this->lastResponseBody());
+        self::assertStringContainsString('Total installs: 1', $crawler->text(null, true));
     }
 }
diff --git a/tests/Functional/Controller/Admin/ProxyControllerTest.php b/tests/Functional/Controller/Admin/ProxyControllerTest.php
@@ -33,10 +33,10 @@ public function testStats(): void
             [new Package('buddy-works/repman', '1.0.0.0')],
             new \DateTimeImmutable('2020-04-27 19:34:00')
         );
-        $this->client->request('GET', $this->urlTo('admin_proxy_stats'));
+        $crawler = $this->client->request('GET', $this->urlTo('admin_proxy_stats'));
 
         self::assertTrue($this->client->getResponse()->isOk());
-        self::assertStringContainsString('Total installs: 1', $this->lastResponseBody());
+        self::assertStringContainsString('Total installs: 1', $crawler->text(null, true));
     }
 
     public function testRemoveDistPackage(): void