Merge pull request #29 from UKGovernmentBEIS/bug/report-count

Fixed the report count on search results so that it does not include …

src/main/scala/controllers/ReportController.scala

@@ -36,37 +36,33 @@ import utils.YesNo
 import scala.concurrent.{ExecutionContext, Future}
 
 class ReportController @Inject()(
-                                  companySearch: CompanySearchService,
                                   companyAuth: CompanyAuthService,
-                                  reports: ReportService,
+                                  val companySearch: CompanySearchService,
+                                  val reportService: ReportService,
                                   val appConfig: AppConfig
-                                )(implicit ec: ExecutionContext, messages: MessagesApi) extends Controller with PageHelper {
+                                )(implicit val ec: ExecutionContext, messages: MessagesApi)
+  extends Controller
+    with PageHelper
+    with SearchHelper {
 
   import views.html.{report => pages}
 
   private val searchPageTitle = "Search for a company"
   private val signInPageTitle = "Sign in using your Companies House account"
+
   private def publishTitle(companyName: String) = s"Publish a report for $companyName"
 
+  val searchHeader = h1("Publish a report")
+  val searchLink = routes.ReportController.search(None, None, None).url
+  val companyLink = { id: CompaniesHouseId => routes.ReportController.start(id).url }
+
+  def pageLink(query: Option[String], itemsPerPage: Option[Int], pageNumber: Int) = routes.ReportController.search(query, Some(pageNumber), itemsPerPage).url
+
   def search(query: Option[String], pageNumber: Option[Int], itemsPerPage: Option[Int]) = Action.async {
-    val searchLink = routes.ReportController.search(None, None, None).url
-    val pageLink = { i: Int => routes.ReportController.search(query, Some(i), itemsPerPage).url }
-    val companyLink = { id: CompaniesHouseId => routes.ReportController.start(id).url }
-    val header = h1("Publish a report")
-
-    query match {
-      case Some(q) => companySearch.searchCompanies(q, pageNumber.getOrElse(1), itemsPerPage.getOrElse(25)).flatMap { results =>
-        val countsF = results.items.map { report =>
-          reports.byCompanyNumber(report.companiesHouseId).map(rs => (report.companiesHouseId, rs.length))
-        }
-
-        Future.sequence(countsF).map { counts =>
-          val countMap = Map(counts: _*)
-          Ok(page(searchPageTitle)(home, header, views.html.search.search(q, Some(results), countMap, searchLink, companyLink, pageLink)))
-        }
-      }
-      case None => Future.successful(Ok(page(searchPageTitle)(home, header, views.html.search.search("", None, Map.empty, searchLink, companyLink, pageLink))))
-    }
+    def resultsPage(q: String, results: Option[PagedResults[CompanySearchResult]], countMap: Map[CompaniesHouseId, Int]) =
+      page(searchPageTitle)(home, searchHeader, views.html.search.search(q, results, countMap, searchLink, companyLink, pageLink(query, itemsPerPage, _)))
+
+    doSearch(query, pageNumber, itemsPerPage, resultsPage).map(Ok(_))
   }
 
   def start(companiesHouseId: CompaniesHouseId) = Action.async { request =>

src/main/scala/controllers/SearchController.scala

@@ -25,52 +25,44 @@ import config.AppConfig
 import models.{CompaniesHouseId, ReportId}
 import org.joda.time.format.DateTimeFormat
 import play.api.mvc.{Action, Controller}
-import services.{CompanyAuthService, CompanySearchService, PagedResults, ReportService}
+import play.twirl.api.Html
+import services._
 
-import scala.concurrent.{ExecutionContext, Future}
+import scala.concurrent.ExecutionContext
 
 class SearchController @Inject()(
-                                  companySearch: CompanySearchService,
-                                  companyAuth: CompanyAuthService,
-                                  reports: ReportService,
-                                  val appConfig: AppConfig)(implicit ec: ExecutionContext)
+                                  val companySearch: CompanySearchService,
+                                  val reportService: ReportService,
+                                  val appConfig: AppConfig)(implicit val ec: ExecutionContext)
   extends Controller
-    with PageHelper {
+    with PageHelper
+    with SearchHelper {
 
   val df = DateTimeFormat.forPattern("d MMMM YYYY")
 
   def start() = Action(Ok(page("Search payment practice reports")(views.html.search.start())))
 
   private val searchForReports = "Search for reports"
+  val searchHeader = h1(searchForReports)
+  val searchLink = routes.SearchController.search(None, None, None).url
+  val searchPageTitle = "Search for a company"
+
+  def companyLink(id: CompaniesHouseId, pageNumber: Option[Int]) = routes.SearchController.company(id, pageNumber).url
+
+  def pageLink(query: Option[String], itemsPerPage: Option[Int], pageNumber: Int) = routes.ReportController.search(query, Some(pageNumber), itemsPerPage).url
 
   def search(query: Option[String], pageNumber: Option[Int], itemsPerPage: Option[Int]) = Action.async {
-    val searchLink = routes.SearchController.search(None, None, None).url
-    val pageLink = { i: Int => routes.SearchController.search(query, Some(i), itemsPerPage).url }
-    val companyLink = { id: CompaniesHouseId => routes.SearchController.company(id, pageNumber).url }
-    val header = h1(searchForReports)
-    val title = "Search for a company"
-
-    query match {
-      case Some(q) => companySearch.searchCompanies(q, pageNumber.getOrElse(1), itemsPerPage.getOrElse(25)).flatMap { results =>
-        val countsF = results.items.map { report =>
-          reports.byCompanyNumber(report.companiesHouseId).map(rs => (report.companiesHouseId, rs.length))
-        }
-
-        Future.sequence(countsF).map { counts =>
-          val countMap = Map(counts: _*)
-
-          Ok(page(title)(home, header, views.html.search.search(q, Some(results), countMap, searchLink, companyLink, pageLink)))
-        }
-      }
-      case None => Future.successful(Ok(page(title)(home, header, views.html.search.search("", None, Map.empty, searchLink, companyLink, pageLink))))
-    }
+    def resultsPage(q: String, results: Option[PagedResults[CompanySearchResult]], countMap: Map[CompaniesHouseId, Int]): Html =
+      page(searchPageTitle)(home, searchHeader, views.html.search.search(q, results, countMap, searchLink, companyLink(_, pageNumber), pageLink(query, itemsPerPage, _)))
+
+    doSearch(query, pageNumber, itemsPerPage, resultsPage).map(Ok(_))
   }
 
   def company(companiesHouseId: CompaniesHouseId, pageNumber: Option[Int]) = Action.async { implicit request =>
     val pageLink = { i: Int => routes.SearchController.company(companiesHouseId, Some(i)).url }
     val result = for {
       co <- OptionT(companySearch.find(companiesHouseId))
-      rs <- OptionT.liftF(reports.byCompanyNumber(companiesHouseId).map(rs => PagedResults.page(rs.flatMap(_.filed), pageNumber.getOrElse(1))))
+      rs <- OptionT.liftF(reportService.byCompanyNumber(companiesHouseId).map(rs => PagedResults.page(rs.flatMap(_.filed), pageNumber.getOrElse(1))))
     } yield {
       val searchCrumb = Breadcrumb(routes.SearchController.search(None, None, None), searchForReports)
       val crumbs = breadcrumbs(homeBreadcrumb, searchCrumb)
@@ -85,7 +77,7 @@ class SearchController @Inject()(
 
   def view(reportId: ReportId) = Action.async { implicit request =>
     val f = for {
-      report <- OptionT(reports.findFiled(reportId))
+      report <- OptionT(reportService.findFiled(reportId))
     } yield {
       val searchCrumb = Breadcrumb(routes.SearchController.search(None, None, None), searchForReports)
       val companyCrumb = Breadcrumb(routes.SearchController.company(report.header.companyId, None), s"${report.header.companyName} reports")

src/main/scala/controllers/SearchHelper.scala

@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2017  Department for Business, Energy and Industrial Strategy
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package controllers
+
+import models.CompaniesHouseId
+import play.twirl.api.Html
+import services.{CompanySearchResult, CompanySearchService, PagedResults, ReportService}
+
+import scala.concurrent.{ExecutionContext, Future}
+
+trait SearchHelper {
+  def companySearch: CompanySearchService
+
+  def reportService: ReportService
+
+  implicit def ec: ExecutionContext
+
+  type ResultsPageFunction = (String, Option[PagedResults[CompanySearchResult]], Map[CompaniesHouseId, Int]) => Html
+
+  def doSearch(query: Option[String], pageNumber: Option[Int], itemsPerPage: Option[Int], resultsPage: ResultsPageFunction) = {
+    query match {
+      case Some(q) => companySearch.searchCompanies(q, pageNumber.getOrElse(1), itemsPerPage.getOrElse(25)).flatMap { results =>
+        val countsF = results.items.map { result =>
+          reportService.byCompanyNumber(result.companiesHouseId).map(rs => (result.companiesHouseId, rs.count(_.isFiled)))
+        }
+
+        Future.sequence(countsF).map(counts => resultsPage(q, Some(results), Map(counts: _*)))
+      }
+
+      case None => Future.successful(resultsPage("", None, Map.empty))
+    }
+  }
+}

src/main/scala/services/ReportService.scala

@@ -45,6 +45,8 @@ case class Report(
     other <- otherInfo
     f <- filing
   } yield FiledReport(header, p, terms, hist, other, f)
+
+  def isFiled: Boolean = filed.isDefined
 }
 
 case class FiledReport(

src/main/scala/slicks/repos/ReportQueries.scala

@@ -26,7 +26,10 @@ trait ReportQueries {
   import api._
 
   /**
-    * Use left joins so that sections that are not completed come back as None
+    * This is quite an awkward query expression. The purpose is to select all report headers and, at
+    * the same time, retrieve all sections of the report that are present in the database. This allows
+    * for the situation where some sections are not yet completed. All of the section structures come
+    * back as `Option`s. Only the header is guaranteed to be present in the result.
     */
   val reportQuery = {
     reportHeaderTable