Solution

SOLUTION.md

@@ -3,11 +3,44 @@ SOLUTION
 
 Estimation
 ----------
-Estimated: n hours
+Estimated: 4 hours
 
-Spent: x hours
+Spent: 3 hours (+ 1 hour the solution and improvement description)
 
+I've given myself some buffer when making an estimate, primarily on the account
+of unfamiliar development environment and no prior experience with the Symfony framework.
+
+This estimate was provided with a simplifed solution in mind. Ideally I would prefer more
+time research the Symfony functionalities (Commands, DependencyInjection, etc..), and to back up the solution with implemented unit tests.
 
 Solution
 --------
-Comments on your solution
+
+My approach was to split the functionality into smaller 'service' classes that can be more easily tested and mocked when using the unit tests.
+
+The command functionality was extended to accept the 'year' attribute which is used to load the 
+data for that year. If no year is provided the application default to current year. 
+
+I've created a simple 'loader' class (YearlyViewsDataLoader, abstraction of ViewsDataLoader) which 
+accepts the arguments (year at this point), builds the required query and return the result data.
+
+To avoid too data parsing by PHP scripts, I've tried to get the query result output as close
+as possible to the expected example output.
+
+This data is passed on the 'renderer' class (in this case the ConsoleViewsDataRenderer) which parses
+the data and outputs it in a console table.
+
+Improvements
+-------- 
+
+There are possibilites for perfomance improvement. It's harder to anticipate any bottlenecks
+without knowing how the solution would work as a part of a big picture or what the actual data size would be, but here are some possibilities:
+
+- Chunking the query result when data sets are too large to avoid issues with memory performance.
+- Consider delegating the functionality to a job, so the report will be generated when the server load is lighter.
+- Implementing a caching system, especially for older data which (I assume) no longer changes.
+- Adding an option to specify a set of profiles for which the data should be loaded, to avoid loading data for all existing profiles.
+- Similary, going down a level, specifying a range of months for which the data should be loaded.
+
+
+The 'product' could be expanded by enabling more report output formats (e.g. JSON, CSV...), adding an option to send generated reports to email or deploy them to external storage or to create reports automatically on the set time interval (end of the month, end of year).

setup.sql

@@ -1,12 +1,12 @@
 DROP DATABASE IF EXISTS `bof_test`;
 CREATE DATABASE IF NOT EXISTS `bof_test` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;
 
-DROP USER IF EXISTS 'bof-test'@'localhost';
+/* DROP USER IF EXISTS 'bof-test'@'localhost';
 CREATE USER 'bof-test'@'localhost' IDENTIFIED BY 'bof-test';
 
 GRANT USAGE ON * . * TO 'bof-test'@'localhost' IDENTIFIED BY 'bof-test' WITH MAX_QUERIES_PER_HOUR 0 MAX_CONNECTIONS_PER_HOUR 0 MAX_UPDATES_PER_HOUR 0 MAX_USER_CONNECTIONS 0 ;
 
-GRANT ALL PRIVILEGES ON `bof\_test` . * TO 'bof-test'@'localhost' WITH GRANT OPTION ;
+GRANT ALL PRIVILEGES ON `bof\_test` . * TO 'bof-test'@'localhost' WITH GRANT OPTION ; */
 
 DROP TABLE IF EXISTS `bof_test`.`profiles`;
 CREATE TABLE `bof_test`.`profiles` (

src/Command/ReportYearlyCommand.php

@@ -2,6 +2,8 @@
 namespace BOF\Command;
 
 use Doctrine\DBAL\Driver\Connection;
+use BOF\Service\YearlyViewsDataLoader;
+use BOF\Service\ConsoleViewsDataRenderer;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -14,7 +16,8 @@ protected function configure()
         $this
             ->setName('report:profiles:yearly')
             ->setDescription('Page views report')
-        ;
+            ->addArgument('year', InputOption::VALUE_OPTIONAL, 'Year for which the data will be generated.')
+            ;
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
@@ -23,10 +26,23 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $io = new SymfonyStyle($input,$output);
         $db = $this->getContainer()->get('database_connection');
 
-        $profiles = $db->query('SELECT profile_name FROM profiles')->fetchAll();
+        /**
+         * Ideally the classes used below could be injected 
+         * in a constructor or as a method argument.
+         */
+        $dataLoader = new YearlyViewsDataLoader($db);
 
-        // Show data in a table - headers, data
-        $io->table(['Profile'], $profiles);
+        $profilesVIews = $dataLoader
+                    ->setYear($input->getArgument('year')[0])
+                    ->load();
 
+        $header = ['Profiles ' . $dataLoader->getYear(),
+                    'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun',
+                    'Jul', 'Aug', 'Sep','Oct','Nov','Dec'];
+
+        $renderer = new ConsoleViewsDataRenderer;
+        $renderer
+            ->setHeader($header)
+            ->render($io, $profilesVIews);
     }
 }

src/Service/ConsoleViewsDataRenderer.php

@@ -0,0 +1,68 @@
+<?php
+namespace BOF\Service;
+
+use Symfony\Component\Console\Helper\Table;
+use Symfony\Component\Console\Helper\TableCell;
+use Symfony\Component\Console\Style\SymfonyStyle;
+
+class ConsoleViewsDataRenderer extends ViewsDataRenderer
+{
+	/**
+	 * Table header row
+	 * 
+	 * @param array
+	 */
+	protected $header;
+
+	/**
+	 * Return header array
+	 * 
+	 * @return array
+	 */
+	public function getHeader()
+	{
+		return $this->header;
+	}
+
+	/**
+	 * Set table header
+	 * 
+	 * @param array $header
+	 * @return this
+	 */
+	public function setHeader($header)
+	{
+		$this->header = $header;
+
+		return $this;
+	}
+
+	/**
+	 * Output the provided data 
+	 * in console table format.
+	 * 
+	 * @return void
+	 */
+	public function render(SymfonyStyle $io, array $viewsData)
+	{
+		if(empty($viewsData)) {
+			$io->writeln('n/a');
+			return;
+        }
+
+				$table = new Table($io);
+
+				if($this->getHeader()) {
+					$table->setHeaders($this->getHeader());
+				}
+
+        foreach($viewsData as $viewData) 
+        {
+            $table->addRow(
+				$this->formatRow($viewData)
+			);
+        }
+
+        $table->render();
+	}
+}

src/Service/ViewsDataLoader.php

@@ -0,0 +1,39 @@
+<?php
+namespace BOF\Service;
+
+use Doctrine\DBAL\Connection;
+
+abstract class ViewsDataLoader
+{
+	/**
+	 * Db instance
+	 * 
+	 * @param \Doctrine\DBAL\Connection $db
+	 */
+	protected $db;
+
+	/**
+	 * Constructor
+	 * 
+	 * @param \Doctrine\DBAL\Connection $db
+	 */
+	public function __construct(Connection $db)
+	{
+		$this->db = $db;
+	}
+
+	/**
+	 * Load data from the database
+	 * 
+	 * @return array
+	 */
+	abstract protected function load();
+
+	/**
+	 * Return a query build with applied query parameters
+	 * 
+	 * @return \Doctrine\DBAL\Driver\PDOStatement
+	 */
+	abstract protected function getQuery();
+
+}

src/Service/ViewsDataRenderer.php

@@ -0,0 +1,23 @@
+<?php
+namespace BOF\Service;
+
+abstract class ViewsDataRenderer
+{
+	/**
+	 * Format the data by replacing empty fields
+	 * with 'n/a' and format the numbers.
+	 * 
+	 * @param array $row
+	 * 
+	 * @return array
+	 */
+	protected function formatRow(array $row) 
+	{
+		return array_map (function($value) {
+			if(is_numeric($value)) {
+				$value = number_format($value, 0, '', ',');
+			}
+			return $value ?? 'n/a';
+		}, $row);
+	}
+}

src/Service/YearlyViewsDataLoader.php

@@ -0,0 +1,92 @@
+<?php
+namespace BOF\Service;
+
+use Doctrine\DBAL\Connection;
+
+class YearlyViewsDataLoader extends ViewsDataLoader
+{
+	/**
+	 * Year for which the data is loaded
+	 * 
+	 * @param int
+	 */
+	protected $year;
+
+	/**
+	 * @see ViewsDataLoader::load
+	 */
+	public function load()
+	{
+		return $this
+			->getQuery()
+			->fetchAll();
+	}
+
+	/**
+	 * @see ViewsDataLoader::getQuery
+	 */
+	protected function getQuery()
+	{
+		/**
+		 * Get total count of user views for  
+		 * selected year grouped by user and month.
+		 */
+		return $this->db
+			->query("
+				select 
+					p.profile_name,
+					{$this->caseMonths()}
+					from profiles p
+				left join 
+					views v
+				on 
+					p.profile_id = v.profile_id
+				where 
+					YEAR(v.date) = {$this->getYear()}
+				group by 
+					v.profile_id
+				order 
+					by p.profile_name
+			");
+	}
+
+	/**
+	 * Case views months so the groups are displayed horizontally
+	 * 
+	 * @return string
+	 */
+	protected function caseMonths()
+	{
+		$months = '';
+
+		foreach(range(1,12) as $month) {
+			$months.= "sum(CASE WHEN month(v.date) = {$month} THEN v.views END) AS month_{$month}, ";
+		}
+
+		return rtrim($months,', ');
+	}
+
+	/**
+	 * Set year
+	 * 
+	 * @param int $year
+	 * @return this
+	 */
+	public function setYear(int $year)
+	{
+		$this->year = $year;
+
+		return $this;
+	}
+
+	/**
+	 * Get year, if null default to current year
+	 * 
+	 * @return int $year
+	 */
+	public function getYear()
+	{
+		return $this->year 
+			?? $this->year = date('Y');
+	}
+}

src/Tests/YearlyReport.feature

@@ -0,0 +1,49 @@
+GIVEN that there is historical data available
+WHEN I execute the Yearly Views report
+THEN I expect to see a monthly breakdown of the total views per profiles
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report
+THEN I expect to have the profiles names listed in alphabetical order
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report
+THEN I expect to see "n/a" when data is not available
+
+
+# Added test cases
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report
+THEN I expect to see "n/a" in monthly column when data for that month is not available
+
+GIVEN that there is historical data available
+WHEN I execute the Yearly Views report without the year argument
+THEN I expect to see a monthly breakdown of the total views per profiles for current year
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report with the year argument
+THEN I expect to see monthly breakdown of the total views per profiles for that year
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report with out of range year (e.g. 10000)
+THEN I expect to see "n/a"
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report with year argument where profiles have no views (e.g. 2020)
+THEN I expect to see "n/a"
+
+
+# Future test cases (not yet implemented)
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report with invalid year argument (e.g. 'foo' string instead of numeric)
+THEN I expect to see "n/a"
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report with profiles set argument
+THEN I expect to see monthly breakdown of the total views per profiles for those profiles only
+
+GIVEN that there is historical data available
+WHEN I view the Yearly Views report with empty profiles set argument
+THEN I expect to see "n/a"