diff --git a/SOLUTION.md b/SOLUTION.md index defe675..e8570ac 100755 --- a/SOLUTION.md +++ b/SOLUTION.md @@ -3,11 +3,27 @@ SOLUTION Estimation ---------- -Estimated: n hours +Estimated: 8 hours -Spent: x hours +Spent: 7 hours Solution -------- -Comments on your solution + +I have little experience with PHP and no experience with Symphony, so this was quite a challenge for me. + +I tried to approach the problem so the logic for getting data and dsiplaying data are separated in different classes. The idea behind is that this can be extended or parts more easily replaced. + +The future improvements are numerous: + +- an UI for end user +- access permission depending on the user +- better validation of input data +- possible export to different file formats not just display in console/UI +- API access for 3rd party developers +- a caching strategy for performance gains +- historical data with graphs +- comparing profiles +- data analysis tools so we can see how profiles perform depending on day of the week, month etc +- track also view per minute in the day not just per day \ No newline at end of file diff --git a/setup.sql b/setup.sql index 1e0d56e..b19a438 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'; -CREATE USER 'bof-test'@'localhost' IDENTIFIED BY 'bof-test'; +-- 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 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` ( diff --git a/src/Command/ReportYearlyCommand.php b/src/Command/ReportYearlyCommand.php index 97f026f..866c668 100755 --- a/src/Command/ReportYearlyCommand.php +++ b/src/Command/ReportYearlyCommand.php @@ -6,6 +6,8 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +use BOF\Domain\YearlyReport; +use BOF\Domain\TableDisplay; class ReportYearlyCommand extends ContainerAwareCommand { @@ -14,19 +16,44 @@ protected function configure() $this ->setName('report:profiles:yearly') ->setDescription('Page views report') + ->addArgument('year', InputOption::VALUE_REQUIRED, 'Please select a year you are interested in:') ; } protected function execute(InputInterface $input, OutputInterface $output) { /** @var $db Connection */ - $io = new SymfonyStyle($input,$output); - $db = $this->getContainer()->get('database_connection'); + $io = new SymfonyStyle($input,$output); - $profiles = $db->query('SELECT profile_name FROM profiles')->fetchAll(); + $db = $this->getContainer()->get('database_connection'); + // get year from cli arguments + $year = $input->getArgument('year'); - // Show data in a table - headers, data - $io->table(['Profile'], $profiles); + // create the report + $report = new YearlyReport($db, $year); + + // get report data + $data = $report->getData(); + + // generate heders + $headers = $this->generateHeaders($year); + + // create display table + $tableDisplay = new TableDisplay($headers); + + // ouput to the console + $tableDisplay->display($data, $io); + } + private function generateHeaders($year){ + // first column + $headers = array('Profile '.$year); + for ($m=1; $m<=12; $m++) { + // get english month name + $month = date('F', mktime(0,0,0, $m, 1, date('Y'))); + // add first 3 letters to headers + $headers[$m] = substr($month, 0, 3); + } + return $headers; } } diff --git a/src/Domain/TableDisplay.php b/src/Domain/TableDisplay.php new file mode 100644 index 0000000..597bf67 --- /dev/null +++ b/src/Domain/TableDisplay.php @@ -0,0 +1,35 @@ +headers = $headers; + } + + function display($data, $io){ + $table = new Table($io); + $table->setHeaders($this->headers); + // append each row to the table for display + foreach($data as $row){ + // transform column values + $row = array_map(function ($value) { + if (empty($value)){ // return n/a for empty values + return 'n/a'; + } + if (is_numeric($value)){ + return number_format($value, 0, '.', ','); + } + // if it is enither empty or number, return whatever + // was passed + return $value; + }, $row); + // add transformed rows to the table + $table->addRow($row); + } + $table->render(); + } +} \ No newline at end of file diff --git a/src/Domain/YearlyReport.php b/src/Domain/YearlyReport.php new file mode 100644 index 0000000..d3cbf39 --- /dev/null +++ b/src/Domain/YearlyReport.php @@ -0,0 +1,50 @@ +connection = $conn; + $this->sql = $this->buildQuery($year); + } + + public function getData(){ + return $this->connection->query($this->sql)->fetchAll(); + } + + public function buildQuery($year){ + $monthColumns = array(); + // create an array of sql sum statements so we can + // pivot the table and diplay row contents as columns + for($i = 0; $i < 12; $i++){ + $monthIndex = $i + 1; + $monthColumns[$i] = 'SUM(CASE WHEN MONTH(v.date) = '.$monthIndex.' THEN v.views END) AS m'.$monthIndex; + } + // join into 1 string + $monthColumns = implode(', ', $monthColumns); + + // build the query. sadly I do not know how to properly + // bind parameters to the query in php, but checking it for + // int should be enough to prevent sql injection + $sql = 'SELECT p.profile_name as profileName, + '.$monthColumns.' + FROM profiles p + LEFT JOIN views v ON p.profile_id = v.profile_id + WHERE YEAR(v.date) = '.$year.' + GROUP BY p.profile_name + ORDER BY profile_name;'; + // echo($sql); + return $sql; + } +} \ No newline at end of file diff --git a/src/Tests/YearlyReport.feature b/src/Tests/YearlyReport.feature index e69de29..f84ac04 100755 --- a/src/Tests/YearlyReport.feature +++ b/src/Tests/YearlyReport.feature @@ -0,0 +1,33 @@ +Story: +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 + +Test cases: +1. +Scenario: A properly formated value for the year is supplied +Expected result: Report is shown that is alphabetically ordered with view count groupped by month + +2. +Scenario: A year is supplied where there is missing data for months +Expected result: Same as first case, but cells have "n/a" displayed instead of view count + +3. +Scenario: No year supplied +Expected result: Application throws an error saying that year should be an integer greater than 0 + +4. +Scenario: A year is supplied for which there is no data +Expected result: Empty table is shown + +5. +Scenario: A year is supplied but its value is less than 0 +Expected result: Application throws an error saying this is invaldi value \ No newline at end of file