Skip to content

Commit

Permalink
Moving breaking-news to onward
Browse files Browse the repository at this point in the history
I made a lot of mistakes when creating the breaking-news feature, from the
internal design choices (using AkkaActor for instance) to the location of
the endpoint (in admin-jobs)
Ideally we would need to refactor the breaking-news feature but this
patch is only addressing the location issue.
Breaking-news has nothing to do in admin-jobs. This patch is moving it
in onward, even though it might not be the best place for the POST /news-alers/alert endpoint, at least it is logical to have the GET /news-alert endpoint in onward (and because of the bad design choice we cannot simply break the 2 endpoints into 2 different apps)
  • Loading branch information
TBonnin committed Jan 19, 2017
1 parent 44894c1 commit eb33c94
Show file tree
Hide file tree
Showing 29 changed files with 35 additions and 196 deletions.
56 changes: 0 additions & 56 deletions admin-jobs/app/AppLoader.scala

This file was deleted.

11 changes: 0 additions & 11 deletions admin-jobs/app/controllers/AdminJobsControllers.scala

This file was deleted.

8 changes: 0 additions & 8 deletions admin-jobs/app/controllers/HealthCheck.scala

This file was deleted.

44 changes: 0 additions & 44 deletions admin-jobs/conf/application.conf

This file was deleted.

22 changes: 0 additions & 22 deletions admin-jobs/conf/logback.xml

This file was deleted.

8 changes: 0 additions & 8 deletions admin-jobs/conf/routes

This file was deleted.

12 changes: 0 additions & 12 deletions admin-jobs/test/AdminJobsTestSuite.scala

This file was deleted.

2 changes: 0 additions & 2 deletions dev-build/app/AppLoader.scala
Expand Up @@ -36,7 +36,6 @@ class AppLoader extends FrontendApplicationLoader {

trait Controllers
extends AdminControllers
with AdminJobsControllers
with ApplicationsControllers
with ArticleControllers
with CommercialControllers
Expand Down Expand Up @@ -65,7 +64,6 @@ trait AppComponents
with SportServices
with CommercialServices
with DiscussionServices
with AdminJobsServices
with OnwardServices
with FapiServices
with ApplicationsServices {
Expand Down
5 changes: 1 addition & 4 deletions docs/02-architecture/01-applications-architecture.md
Expand Up @@ -122,14 +122,11 @@ Admin app hosts a set of dashboards and tools used by Guardian developers to mon

[All supported routes](https://github.com/guardian/frontend/blob/master/admin/conf/routes)

# Admin-jobs
Admin-jobs app has currently two endpoints to:

- Create breaking-news alerts

- [Get all breaking-news alert](http://api.nextgen.guardianapps.co.uk/news-alert/alerts)

[All supported routes](https://github.com/guardian/frontend/blob/master/admin-jobs/conf/routes)
[All supported routes](https://github.com/guardian/frontend/blob/master/sport/conf/routes)

# Archive
In case none of the other apps can serve a given request, it is finally passed to the Archive app which checks if there is any redirect setup for this url or any old static content attached to it.
Expand Down
3 changes: 3 additions & 0 deletions onward/app/AppLoader.scala
Expand Up @@ -19,6 +19,7 @@ import play.api.routing.Router
import play.api.libs.ws.WSClient
import router.Routes
import services.OphanApi
import services.breakingnews.{BreakingNewsApi, S3BreakingNews}
import weather.WeatherApi

class AppLoader extends FrontendApplicationLoader {
Expand All @@ -43,6 +44,8 @@ trait OnwardServices {
lazy val mostViewedAudioAgent = wire[MostViewedAudioAgent]
lazy val mostViewedGalleryAgent = wire[MostViewedGalleryAgent]
lazy val mostViewedVideoAgent = wire[MostViewedVideoAgent]
lazy val s3BreakingNews = wire[S3BreakingNews]
lazy val breakingNewsApi = wire[BreakingNewsApi]
}

trait AppComponents extends FrontendComponents with OnwardControllers with OnwardServices {
Expand Down
@@ -1,6 +1,6 @@
package authentication

import play.api.mvc.{Result, Results, Request, ActionBuilder}
import play.api.mvc.{ActionBuilder, Request, Result, Results}

import scala.concurrent.Future

Expand Down
Expand Up @@ -6,13 +6,13 @@ import akka.util.Timeout
import authentication.AuthenticationSupport
import common.ExecutionContexts
import conf.Configuration
import controllers.BreakingNews.{BreakingNewsApi, BreakingNewsUpdater, GetAlertsRequest, NewNotificationRequest}
import model.Cached.RevalidatableResult
import model.{Cors, Cached}
import model.{Cached, Cors}
import models.NewsAlertNotification
import play.api.libs.json.{JsValue, Json}
import play.api.mvc.BodyParsers.parse.{json => BodyJson}
import play.api.mvc._
import services.breakingnews._

import scala.concurrent.duration._

Expand Down
6 changes: 6 additions & 0 deletions onward/app/controllers/OnwardControllers.scala
@@ -1,12 +1,14 @@
package controllers

import akka.actor.ActorSystem
import com.softwaremill.macwire._
import weather.controllers.{LocationsController, WeatherController}
import business.StocksData
import contentapi.ContentApiClient
import feed._
import model.ApplicationContext
import play.api.libs.ws.WSClient
import services.breakingnews.BreakingNewsApi
import weather.WeatherApi

trait OnwardControllers {
Expand All @@ -24,6 +26,8 @@ trait OnwardControllers {
def mostViewedVideoAgent: MostViewedVideoAgent
def mostViewedGalleryAgent: MostViewedGalleryAgent
def mostViewedAudioAgent: MostViewedAudioAgent
def breakingNewsApi: BreakingNewsApi
def actorSystem: ActorSystem

lazy val navigationController = wire[NavigationController]
lazy val weatherController = wire[WeatherController]
Expand All @@ -47,4 +51,6 @@ trait OnwardControllers {
lazy val seriesController = wire[SeriesController]
lazy val stocksController = wire[StocksController]
lazy val techFeedbackController = wire[TechFeedbackController]
lazy val newsAlertController = wire[NewsAlertController]

}
Expand Up @@ -2,6 +2,7 @@ package models

import java.net.URI
import java.util.UUID

import org.joda.time.DateTime
import play.api.libs.functional.syntax._
import play.api.libs.json._
Expand Down
Expand Up @@ -2,11 +2,9 @@ package models

import java.net.URI
import java.util.UUID

import org.joda.time.DateTime
import org.joda.time.format.ISODateTimeFormat
import play.api.libs.json._

import scala.util.{Failure, Success, Try}

case class NewsAlertNotification(uid: UUID,
Expand Down
Expand Up @@ -2,8 +2,6 @@ package models

import play.api.libs.json._

import scala.util.{Failure, Success, Try}


sealed trait NewsAlertType
object NewsAlertTypes {
Expand Down
File renamed without changes.
@@ -1,10 +1,10 @@
package controllers.BreakingNews
package services.breakingnews

import common.{ExecutionContexts, Logging}
import conf.Configuration
import play.api.libs.json._
import services.S3
import play.api.{Environment, Mode}
import services.S3

class S3BreakingNews(environment: Environment) extends S3 {
override lazy val bucket = Configuration.aws.bucket
Expand Down
@@ -1,4 +1,4 @@
package controllers.BreakingNews
package services.breakingnews

import akka.actor.Status.{Failure => ActorFailure}
import akka.actor.{Actor, Props}
Expand Down
4 changes: 4 additions & 0 deletions onward/conf/routes
Expand Up @@ -78,3 +78,7 @@ GET /most-read-mf2.json controllers.MostPopularCon
GET /related-mf2/*path.json controllers.RelatedController.renderMf2(path)
GET /series-mf2/*path.json controllers.SeriesController.renderMf2SeriesStories(path)
GET /editionalised-nav.json controllers.NavigationController.renderAmpNav()

## Breaking-news
GET /news-alert/alerts controllers.NewsAlertController.alerts()
POST /news-alert/alert controllers.NewsAlertController.create()
Expand Up @@ -6,13 +6,13 @@ import java.util.UUID
import akka.actor.Status.{Failure => ActorFailure}
import akka.actor.{Actor, ActorSystem, Props}
import akka.stream.Materializer
import controllers.BreakingNews.{BreakingNewsApi, S3BreakingNews}
import models.{NewsAlertNotification, NewsAlertTypes}
import org.joda.time.DateTime
import org.scalatest.{DoNotDiscover, Matchers, WordSpec}
import play.api.libs.json.Json
import play.api.test.FakeRequest
import play.api.test.Helpers._
import services.breakingnews.{BreakingNewsApi, S3BreakingNews}
import test.{ConfiguredTestSuite, WithTestContext}

@DoNotDiscover class NewsAlertControllerTest
Expand Down
File renamed without changes.
@@ -1,6 +1,8 @@
package models

import java.net.URI
import java.util.UUID

import org.joda.time.DateTime
import org.scalatest.{FlatSpec, Matchers}
import play.api.libs.json.Json
Expand Down
7 changes: 6 additions & 1 deletion onward/test/package.scala
@@ -1,6 +1,8 @@
package test

import controllers._
import org.scalatest.Suites
import services.breakingnews.{BreakingNewsApiTest, BreakingNewsUpdaterTest}

class OnwardTestSuite extends Suites (
new controllers.ChangeEditionControllerTest,
Expand All @@ -14,7 +16,10 @@ class OnwardTestSuite extends Suites (
new TopStoriesControllerTest,
new VideoInSectionTest,
new RichLinkControllerTest,
new NavigationControllerTest
new NavigationControllerTest,
new BreakingNewsApiTest,
new BreakingNewsUpdaterTest,
new NewsAlertControllerTest
) with SingleServerSuite {
override lazy val port: Int = 19011
}
@@ -1,4 +1,4 @@
package controllers.BreakingNews
package services.breakingnews

import common.ExecutionContexts
import org.joda.time.DateTime
Expand Down
@@ -1,4 +1,4 @@
package controllers.BreakingNews
package services.breakingnews

import java.net.URI
import java.util.UUID
Expand Down

0 comments on commit eb33c94

Please sign in to comment.