Tweak the Google Docs external page feature to support native titles

The Docs title is used if no i18n one is defined
EHRI · Aug 26, 2024 · 2b89e45 · 2b89e45
1 parent aaf0c50
commit 2b89e45
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 18 deletions.
diff --git a/modules/portal/app/assets/css/portal/_style.scss b/modules/portal/app/assets/css/portal/_style.scss
@@ -2157,3 +2157,8 @@ section:last-child {
   @include make-container();
   @include make-container-max-widths();
 }
+
+// External page styling
+.external-page table {
+  @extend .table, .table-bordered, .table-striped;
+}
diff --git a/modules/portal/app/controllers/portal/Portal.scala b/modules/portal/app/controllers/portal/Portal.scala
@@ -185,10 +185,12 @@ case class Portal @Inject()(
   def externalPage(key: String): Action[AnyContent] = OptionalUserAction.async { implicit request =>
     futurePageOr404 {
       htmlPages.get(key, noCache = request.getQueryString("noCache").isDefined).map { futureData =>
-        futureData.map { case (css, html) =>
-          val title = Messages(s"pages.external.$key.title")
-          val meta = Map("description" -> Messages(s"pages.external.$key.description"))
-          Ok(views.html.layout.textLayout(title, meta = meta, styles = css)(html))
+        futureData.map { case (title, css, html) =>
+          val titleKey = s"pages.external.$key.title"
+          val descKey = s"pages.external.$key.description"
+          val i18nTitle = if (Messages.isDefinedAt(titleKey)) Messages(titleKey) else title
+          val i18nMeta = Map("description" -> (if(Messages.isDefinedAt(descKey)) Messages(descKey) else ""))
+          Ok(views.html.layout.textLayout(i18nTitle, meta = i18nMeta, styles = css)(html))
         }
       }
     }

diff --git a/modules/portal/app/services/htmlpages/GoogleDocsHtmlPages.scala b/modules/portal/app/services/htmlpages/GoogleDocsHtmlPages.scala
@@ -22,7 +22,7 @@ import scala.concurrent.{ExecutionContext, Future}
 case class GoogleDocsHtmlPages @Inject ()(ws: WSClient, config: play.api.Configuration)(
     implicit cache: AsyncCacheApi, executionContext: ExecutionContext) extends HtmlPages {
 
-  private def googleDocBody(url: String): Future[(Html, Html)] = {
+  private def googleDocBody(url: String): Future[(String, Html, Html)] = {
     ws.url(url).addQueryStringParameters(
       "e" -> "download",
       "exportFormat" -> "html",
@@ -38,6 +38,9 @@ case class GoogleDocsHtmlPages @Inject ()(ws: WSClient, config: play.api.Configu
       import org.jsoup.Jsoup
 
       val doc = Jsoup.parse(r.body)
+
+      val title = doc.title().replace(" - Google Docs", "")
+
       val body = doc
         .body()
         .tagName("div")
@@ -51,11 +54,11 @@ case class GoogleDocsHtmlPages @Inject ()(ws: WSClient, config: play.api.Configu
       body.select("div#banners").remove()
       body.select("div#footer").remove()
 
-      Html("") -> Html(body.outerHtml())
+      (title, Html(""), Html(body.outerHtml()))
     }
   }
 
-  override def get(key: String, noCache: Boolean = false)(implicit messages: Messages): Option[Future[(Html, Html)]] = {
+  override def get(key: String, noCache: Boolean = false)(implicit messages: Messages): Option[Future[(String, Html, Html)]] = {
     def getUrl: Option[String] =
       config.getOptional[String](s"pages.external.google.$key.${messages.lang.code}") orElse
           config.getOptional[String](s"pages.external.google.$key.default")

diff --git a/modules/portal/app/services/htmlpages/HtmlPages.scala b/modules/portal/app/services/htmlpages/HtmlPages.scala
@@ -17,7 +17,7 @@ trait HtmlPages {
     * @param key      the identifier for this fragment
     * @param noCache  whether to prevent the fragment coming from the cache
     * @param messages an implicit messages instance containing the current language
-    * @return An optional, future of Css -> Body HTML
+    * @return An optional, future of (Title, Css, Body HTML)
     */
-  def get(key: String, noCache: Boolean = false)(implicit messages: Messages): Option[Future[(Html, Html)]]
+  def get(key: String, noCache: Boolean = false)(implicit messages: Messages): Option[Future[(String, Html, Html)]]
 }
diff --git a/modules/portal/app/services/search/AkkaStreamsIndexMediator.scala b/modules/portal/app/services/search/AkkaStreamsIndexMediator.scala
@@ -45,7 +45,7 @@ case class AkkaStreamsIndexMediatorHandle(
   override def withChannel(actorRef: ActorRef, formatter: String => String, filter: Int => Boolean = _ % 100 == 0): AkkaStreamsIndexMediatorHandle =
     copy(chan = Some(actorRef), processFunc = formatter, progressFilter = filter)
 
-  import scala.collection.JavaConverters._
+  import scala.jdk.CollectionConverters._
   import scala.concurrent.duration._
 
   private val logger = Logger(classOf[AkkaStreamsIndexMediator])
@@ -79,8 +79,8 @@ case class AkkaStreamsIndexMediatorHandle(
     }
     .via(httpSinkFlow)
     .collect {
-      case (Success(response), tag) => response
-      case (Failure(e), tag) => throw e
+      case (Success(response), _) => response
+      case (Failure(e), _) => throw e
     }
     .flatMapConcat(_.entity.withoutSizeLimit().dataBytes)
     .toMat(Sink.fold(ByteString.empty)(_ ++ _))(Keep.right)

diff --git a/modules/portal/conf/portal.routes b/modules/portal/conf/portal.routes
@@ -15,7 +15,7 @@ GET         /about                                @controllers.portal.Portal.abo
 GET         /terms                                @controllers.portal.Portal.terms()
 
 GET         /help/datamodel                       @controllers.portal.Portal.dataModel()
-GET         /help/$page<\w+>                      @controllers.portal.Portal.externalPage(page: String)
+GET         /help/$page<\w[\w-_]+>                @controllers.portal.Portal.externalPage(page: String)
 
 # Change locale
 GET         /prefs                                @controllers.portal.Portal.prefs()

diff --git a/test/integration/portal/PortalSpec.scala b/test/integration/portal/PortalSpec.scala
@@ -164,7 +164,7 @@ class PortalSpec extends IntegrationTestRunner {
     "fetch external pages" in new ITestApp {
       val faq = FakeRequest(portalRoutes.externalPage("faq")).call()
       status(faq) must equalTo(OK)
-      contentAsString(faq) must contain(mockdata.externalPages("faq").toString())
+      contentAsString(faq) must contain(mockdata.externalPages("faq")._3.toString())
     }
 
     "return 404 for external pages with a malformed id (bug #635)" in new ITestApp {

diff --git a/test/mockdata/package.scala b/test/mockdata/package.scala
@@ -14,7 +14,7 @@ package object mockdata {
   val yahooOpenId = OpenIDAssociation("linda", "https://yahoo.com/openid", Some(moderator))
 
   val externalPages = Map(
-    "faq" -> Html("<h1>FAQ</h1>")
+    "faq" -> ("Faq", Html(""), Html("<h1>FAQ</h1>"))
   )
 
   // Users...

diff --git a/test/services/htmlpages/MockHtmlPages.scala b/test/services/htmlpages/MockHtmlPages.scala
@@ -10,9 +10,9 @@ import scala.concurrent.Future
  * Mock external pages.
  */
 case class MockHtmlPages() extends HtmlPages {
-  override def get(key: String, noCache: Boolean)(implicit messages: Messages): Option[Future[(Html, Html)]] = {
-    mockdata.externalPages.get(key).map { html =>
-      Some(Future.successful(Html("") -> html))
+  override def get(key: String, noCache: Boolean)(implicit messages: Messages): Option[Future[(String, Html, Html)]] = {
+    mockdata.externalPages.get(key).map { data =>
+      Some(Future.successful(data))
     }.getOrElse(throw new ItemNotFound())
   }
 }