Show available metrics in Program views, in addition to existing metrics

Move retention offset to Event::AVAILABLE_METRICS Bug: https://phabricator.wikimedia.org/T187380
wikimedia · Feb 28, 2018 · e54eb5a · e54eb5a
1 parent 9b8e998
commit e54eb5a
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 14 deletions.
diff --git a/app/Resources/views/programs/index.html.twig b/app/Resources/views/programs/index.html.twig
@@ -46,7 +46,7 @@
                             </td>
                             {% for metric in metrics|keys %}
                                 <td class="text-nowrap">
-                                    {{ program.statistic(metric)|num_format }}
+                                    {{ (program.statistic(metric) ? program.statistic(metric).value|num_format : '&ndash;')|raw }}
                                 </td>
                             {% endfor %}
                         </tr>

diff --git a/app/Resources/views/programs/show.html.twig b/app/Resources/views/programs/show.html.twig
@@ -64,9 +64,9 @@
                                 </td>
                             {% endif %}
                             <td class="text-nowrap">{{ event.numParticipants|num_format }}</td>
-                            {% for metric in metrics|keys %}
+                            {% for metric in metrics %}
                                 <td class="text-nowrap">
-                                    {{ event.statistic(metric) ? event.statistic(metric).value|num_format : 0 }}
+                                    {{ (event.statistic(metric) ? event.statistic(metric).value|num_format : '&ndash;')|raw }}
                                 </td>
                             {% endfor %}
                         </tr>

diff --git a/app/config/parameters.yml.dist b/app/config/parameters.yml.dist
@@ -39,9 +39,6 @@ parameters:
     # Do not include a trailing slash.
     app.root_path: ''
 
-    # Number of days after the event, at which a user is consider to have been retained.
-    app.retention_offset: 7
-
     # Number of revisions to show per page on the Event Data page.
     app.revisions_per_page: 100
 

diff --git a/src/AppBundle/Controller/EventController.php b/src/AppBundle/Controller/EventController.php
@@ -348,7 +348,8 @@ public function showAction()
 
     /**
      * Get EventStats from the given Event. If there are none, empty EventStats
-     * are returned for each metric type specified by self::METRIC_TYPES.
+     * are returned for each metric type specified by EventStat::METRIC_TYPES,
+     * with the default 'offset' values sepcified by Event::getAvailableMetrics().
      * This way we can show placeholders in the view.
      * @param Event $event
      * @return EventStat[]
@@ -360,9 +361,7 @@ private function getEventStats(Event $event)
         }
 
         return array_map(function ($metric) use ($event) {
-            $offset = $metric === 'retention'
-                ? $this->container->getParameter('app.retention_offset')
-                : null;
+            $offset = Event::getAvailableMetrics()[$metric];
             return new EventStat($event, $metric, null, $offset);
         }, EventStat::getMetricTypes());
     }

diff --git a/src/AppBundle/Controller/ProgramController.php b/src/AppBundle/Controller/ProgramController.php
@@ -5,6 +5,7 @@
 
 namespace AppBundle\Controller;
 
+use AppBundle\Model\Event;
 use AppBundle\Model\Program;
 use AppBundle\Model\Organizer;
 use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
@@ -48,7 +49,7 @@ public function indexAction()
             'programs' => $organizer->getPrograms(),
             'programRepo' => $programRepo,
             'gmTitle' => 'my-programs',
-            'retentionThreshold' => $this->container->getParameter('app.retention_offset'),
+            'retentionThreshold' => Event::getAvailableMetrics()['retention'],
             'metrics' => $organizerRepo->getUniqueMetrics($organizer),
         ]);
     }
@@ -140,7 +141,7 @@ public function showAction()
 
         return $this->render('programs/show.html.twig', [
             'program' => $this->program,
-            'retentionThreshold' => $this->container->getParameter('app.retention_offset'),
+            'retentionThreshold' => Event::getAvailableMetrics()['retention'],
             'metrics' => $programRepo->getUniqueMetrics($this->program),
             'isOrganizer' => $this->authUserIsOrganizer($this->program),
         ]);

diff --git a/src/AppBundle/Model/Event.php b/src/AppBundle/Model/Event.php
@@ -35,6 +35,22 @@
  */
 class Event
 {
+    /**
+     * Available metrics type, hard-coded here for accessibility,
+     * while the logic to compute these stats lives in EventProcessor.
+     *
+     * Keys are i18n message keys, values are the 'offset' values.
+     *
+     * @see EventProcessor
+     * @see EventStat
+     */
+    const AVAILABLE_METRICS = [
+        'new-editors' => null,
+        'pages-created' => null,
+        'pages-improved' => null,
+        'retention' => 7,
+    ];
+
     /**
      * NOTE: Some methods pertaining to titles and Participants
      * live in the TitleUserTrait trait.
@@ -174,6 +190,19 @@ public function getId()
         return $this->id;
     }
 
+    /**
+     * Get the available metric types and their default offset values.
+     * @return array
+     * @static
+     *
+     * No need to test a hard-coded list.
+     * @codeCoverageIgnore
+     */
+    public static function getAvailableMetrics()
+    {
+        return self::AVAILABLE_METRICS;
+    }
+
     /***********
      * PROGRAM *
      ***********/

diff --git a/src/AppBundle/Repository/ProgramRepository.php b/src/AppBundle/Repository/ProgramRepository.php
@@ -5,6 +5,7 @@
 
 namespace AppBundle\Repository;
 
+use AppBundle\Model\Event;
 use AppBundle\Model\EventStat;
 use AppBundle\Model\Organizer;
 use AppBundle\Model\Program;
@@ -31,6 +32,8 @@ public function getEntityClass()
 
     /**
      * Get the unique metrics for this Program, across all Events.
+     * This also combines the configured metrics in Event::getAvailableMetrics(),
+     * regardless if the stats exist on the Program or its Events.
      * @param  Program $program
      * @return string[]
      */
@@ -49,8 +52,11 @@ public function getUniqueMetrics(Program $program)
         $eventWikiMetrics = $this->getEventWikiMetrics($rqb, $eventIds);
 
         $metrics = array_merge($eventMetrics, $eventWikiMetrics);
-        $uniqueMetrics = [];
 
+        // Start with available metrics.
+        $uniqueMetrics = Event::getAvailableMetrics();
+
+        // Merge in any differing metrics that exist on the Program.
         foreach ($metrics as $metric) {
             // For each $metric, the first element is the metric name,
             // and the second element is the offset value.
@@ -59,6 +65,7 @@ public function getUniqueMetrics(Program $program)
             }
         }
 
+        // Include configured metrics.
         return $uniqueMetrics;
     }
 

diff --git a/src/AppBundle/Service/EventProcessor.php b/src/AppBundle/Service/EventProcessor.php
@@ -161,7 +161,7 @@ private function setRetention()
     {
         $this->log("\nFetching retention...");
 
-        $retentionOffset = (int)$this->container->getParameter('app.retention_offset');
+        $retentionOffset = Event::getAvailableMetrics()['retention'];
         $end = $this->event->getEndWithTimezone()->modify("+$retentionOffset days");
         $usernames = $this->event->getParticipantNames();