From f03de673167201b97352874cf8b2230e43560c92 Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 09:57:16 +0200 Subject: [PATCH 1/8] Disable user operations on setup.sql --- setup.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.sql b/setup.sql index 1e0d56e..636f249 100755 --- a/setup.sql +++ b/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` ( From 2b34271c0b9502a493b168aa964ff7aed1d97847 Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 11:47:02 +0200 Subject: [PATCH 2/8] Add views data loader --- src/Service/ViewsDataLoader.php | 39 ++++++++++++++ src/Service/YearlyViewsDataLoader.php | 74 +++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 src/Service/ViewsDataLoader.php create mode 100644 src/Service/YearlyViewsDataLoader.php diff --git a/src/Service/ViewsDataLoader.php b/src/Service/ViewsDataLoader.php new file mode 100644 index 0000000..35ce568 --- /dev/null +++ b/src/Service/ViewsDataLoader.php @@ -0,0 +1,39 @@ +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(); + +} diff --git a/src/Service/YearlyViewsDataLoader.php b/src/Service/YearlyViewsDataLoader.php new file mode 100644 index 0000000..b06b93a --- /dev/null +++ b/src/Service/YearlyViewsDataLoader.php @@ -0,0 +1,74 @@ +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, sum(v.views), month(v.date) 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), MONTH(date) + order + by p.profile_name + "); + } + + /** + * Set year + * + * @param int $year + * @return this + */ + public function setYear($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'); + } +} From 61ae79f9adb9a56bdd11f5c606da479321a38e14 Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 12:14:01 +0200 Subject: [PATCH 3/8] Update views data query --- src/Service/YearlyViewsDataLoader.php | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Service/YearlyViewsDataLoader.php b/src/Service/YearlyViewsDataLoader.php index b06b93a..edc4cc5 100644 --- a/src/Service/YearlyViewsDataLoader.php +++ b/src/Service/YearlyViewsDataLoader.php @@ -34,7 +34,9 @@ protected function getQuery() return $this->db ->query(" select - p.profile_name, sum(v.views), month(v.date) from profiles p + p.profile_name, + {$this->caseMonths()} + from profiles p left join views v on @@ -42,12 +44,28 @@ protected function getQuery() where YEAR(v.date) = {$this->getYear()} group by - (v.profile_id), MONTH(date) + 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 * From f5f665a6affe2688b0083419f7e70de2f098671c Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 12:54:01 +0200 Subject: [PATCH 4/8] Add console data renderer --- src/Service/ConsoleViewsDataRenderer.php | 36 ++++++++++++++++++++++++ src/Service/ViewsDataRenderer.php | 23 +++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 src/Service/ConsoleViewsDataRenderer.php create mode 100644 src/Service/ViewsDataRenderer.php diff --git a/src/Service/ConsoleViewsDataRenderer.php b/src/Service/ConsoleViewsDataRenderer.php new file mode 100644 index 0000000..2f669c6 --- /dev/null +++ b/src/Service/ConsoleViewsDataRenderer.php @@ -0,0 +1,36 @@ +writeln('n/a'); + return; + } + + $table = new Table($io); + $table + ->setHeaders(['Profiles', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul','Aug','Sep','Oct','Nov','Dec']) + ; + + foreach($viewsData as $viewData) + { + $table->addRow( + $this->formatRow($viewData) + ); + } + + $table->render(); + } +} diff --git a/src/Service/ViewsDataRenderer.php b/src/Service/ViewsDataRenderer.php new file mode 100644 index 0000000..e6aeaf5 --- /dev/null +++ b/src/Service/ViewsDataRenderer.php @@ -0,0 +1,23 @@ + Date: Tue, 23 Apr 2019 12:54:37 +0200 Subject: [PATCH 5/8] Update report command --- src/Command/ReportYearlyCommand.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Command/ReportYearlyCommand.php b/src/Command/ReportYearlyCommand.php index 97f026f..8c075c1 100755 --- a/src/Command/ReportYearlyCommand.php +++ b/src/Command/ReportYearlyCommand.php @@ -2,11 +2,16 @@ 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; use Symfony\Component\Console\Style\SymfonyStyle; +use Symfony\Component\Console\Helper\Table; +use Symfony\Component\Console\Helper\TableCell; + class ReportYearlyCommand extends ContainerAwareCommand { protected function configure() @@ -14,19 +19,29 @@ 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) { + /** @var $db Connection */ $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); + $profilesData = $dataLoader + ->setYear($input->getArgument('year')[0]) + ->load(); + $renderer = new ConsoleViewsDataRenderer; + $renderer->render($io,$profilesData); } } From a7dd6d9100e3559e3c77d69896d95e8c97310704 Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 12:57:42 +0200 Subject: [PATCH 6/8] Cleanup --- src/Command/ReportYearlyCommand.php | 4 ++-- src/Service/ConsoleViewsDataRenderer.php | 2 +- src/Service/ViewsDataRenderer.php | 10 ++++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Command/ReportYearlyCommand.php b/src/Command/ReportYearlyCommand.php index 8c075c1..d23f51a 100755 --- a/src/Command/ReportYearlyCommand.php +++ b/src/Command/ReportYearlyCommand.php @@ -37,11 +37,11 @@ protected function execute(InputInterface $input, OutputInterface $output) */ $dataLoader = new YearlyViewsDataLoader($db); - $profilesData = $dataLoader + $profilesVIews = $dataLoader ->setYear($input->getArgument('year')[0]) ->load(); $renderer = new ConsoleViewsDataRenderer; - $renderer->render($io,$profilesData); + $renderer->render($io, $profilesVIews); } } diff --git a/src/Service/ConsoleViewsDataRenderer.php b/src/Service/ConsoleViewsDataRenderer.php index 2f669c6..c13bfbe 100644 --- a/src/Service/ConsoleViewsDataRenderer.php +++ b/src/Service/ConsoleViewsDataRenderer.php @@ -21,7 +21,7 @@ public function render($io, $viewsData) $table = new Table($io); $table - ->setHeaders(['Profiles', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul','Aug','Sep','Oct','Nov','Dec']) + ->setHeaders($this->getHeader()) ; foreach($viewsData as $viewData) diff --git a/src/Service/ViewsDataRenderer.php b/src/Service/ViewsDataRenderer.php index e6aeaf5..4ab11d4 100644 --- a/src/Service/ViewsDataRenderer.php +++ b/src/Service/ViewsDataRenderer.php @@ -20,4 +20,14 @@ protected function formatRow($row) return $value ?? 'n/a'; }, $row); } + + /** + * Return header array + * + * @return array + */ + protected function getHeader() + { + return ['Profiles', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul','Aug','Sep','Oct','Nov','Dec']; + } } From c12a616bfc318112f8207f07839b4c22c88f9c52 Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 14:29:42 +0200 Subject: [PATCH 7/8] Refactor --- src/Command/ReportYearlyCommand.php | 15 ++++---- src/Service/ConsoleViewsDataRenderer.php | 42 +++++++++++++++++--- src/Service/ViewsDataRenderer.php | 12 +----- src/Service/YearlyViewsDataLoader.php | 2 +- src/Tests/YearlyReport.feature | 49 ++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 24 deletions(-) diff --git a/src/Command/ReportYearlyCommand.php b/src/Command/ReportYearlyCommand.php index d23f51a..6491dd6 100755 --- a/src/Command/ReportYearlyCommand.php +++ b/src/Command/ReportYearlyCommand.php @@ -9,9 +9,6 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; -use Symfony\Component\Console\Helper\Table; -use Symfony\Component\Console\Helper\TableCell; - class ReportYearlyCommand extends ContainerAwareCommand { protected function configure() @@ -20,13 +17,11 @@ protected function configure() ->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) { - /** @var $db Connection */ $io = new SymfonyStyle($input,$output); $db = $this->getContainer()->get('database_connection'); @@ -41,7 +36,13 @@ protected function execute(InputInterface $input, OutputInterface $output) ->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->render($io, $profilesVIews); + $renderer + ->setHeader($header) + ->render($io, $profilesVIews); } } diff --git a/src/Service/ConsoleViewsDataRenderer.php b/src/Service/ConsoleViewsDataRenderer.php index c13bfbe..05b3584 100644 --- a/src/Service/ConsoleViewsDataRenderer.php +++ b/src/Service/ConsoleViewsDataRenderer.php @@ -3,26 +3,58 @@ 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($io, $viewsData) + public function render(SymfonyStyle $io, array $viewsData) { if(empty($viewsData)) { $io->writeln('n/a'); return; } - $table = new Table($io); - $table - ->setHeaders($this->getHeader()) - ; + $table = new Table($io); + + if($this->getHeader()) { + $table->setHeaders($this->getHeader()); + } foreach($viewsData as $viewData) { diff --git a/src/Service/ViewsDataRenderer.php b/src/Service/ViewsDataRenderer.php index 4ab11d4..710b192 100644 --- a/src/Service/ViewsDataRenderer.php +++ b/src/Service/ViewsDataRenderer.php @@ -11,7 +11,7 @@ abstract class ViewsDataRenderer * * @return array */ - protected function formatRow($row) + protected function formatRow(array $row) { return array_map (function($value) { if(is_numeric($value)) { @@ -20,14 +20,4 @@ protected function formatRow($row) return $value ?? 'n/a'; }, $row); } - - /** - * Return header array - * - * @return array - */ - protected function getHeader() - { - return ['Profiles', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul','Aug','Sep','Oct','Nov','Dec']; - } } diff --git a/src/Service/YearlyViewsDataLoader.php b/src/Service/YearlyViewsDataLoader.php index edc4cc5..ab729ec 100644 --- a/src/Service/YearlyViewsDataLoader.php +++ b/src/Service/YearlyViewsDataLoader.php @@ -72,7 +72,7 @@ protected function caseMonths() * @param int $year * @return this */ - public function setYear($year) + public function setYear(int $year) { $this->year = $year; diff --git a/src/Tests/YearlyReport.feature b/src/Tests/YearlyReport.feature index e69de29..a868e23 100755 --- a/src/Tests/YearlyReport.feature +++ b/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" \ No newline at end of file From 337749628e37c28d6a154bc65c51cf42f1cac765 Mon Sep 17 00:00:00 2001 From: dlabs dlabs Date: Tue, 23 Apr 2019 14:30:02 +0200 Subject: [PATCH 8/8] Update solution --- SOLUTION.md | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/SOLUTION.md b/SOLUTION.md index defe675..7292109 100755 --- a/SOLUTION.md +++ b/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). \ No newline at end of file