Merge pull request #506 from guardian/batchtag-user-logging

Fix 'user' on batch tag operations when viewed via Audit Logs

app/controllers/Reindex.scala

@@ -19,6 +19,7 @@ class Reindex(
   with Logging {
 
   def reindexTags = Action.async { req =>
+    implicit val username: Option[String] = None // unfortunately we don't have a username available
     ReindexProgressRepository.isTagReindexInProgress.flatMap { reindexing =>
       if (reindexing) {
         Future.successful(Forbidden)
@@ -33,6 +34,7 @@ class Reindex(
   }
 
   def reindexSections = Action.async { req =>
+    implicit val username: Option[String] = None // unfortunately we don't have a username available
     ReindexProgressRepository.isSectionReindexInProgress.flatMap { reindexing =>
       if (reindexing) {
         Future.successful(Forbidden)
@@ -47,6 +49,7 @@ class Reindex(
   }
 
   def reindexPillars = Action.async { req =>
+    implicit val username: Option[String] = None // unfortunately we don't have a username available
     ReindexProgressRepository.isPillarReindexInProgress.flatMap { reindexing =>
       if (reindexing) {
         Future.successful(Forbidden)

app/controllers/TagManagementApi.scala

@@ -230,6 +230,7 @@ class TagManagementApi(
   }
 
   def checkPathInUse(tagType: String, slug: String, section: Option[Long], tagSubType: Option[String]) = APIAuthAction.async { req =>
+    implicit val username = Option(req.user.email)
     new PathUsageCheck(tagType, slug, section, tagSubType).process.map{ result =>
       result.map{ t => Ok(Json.toJson(t)) } getOrElse BadRequest
     } recover {
@@ -329,6 +330,7 @@ class TagManagementApi(
 
   def clashingSponsorships(id: Option[Long], tagIds: Option[String], sectionIds: Option[String], validFrom: Option[Long],
                            validTo: Option[Long], editions: Option[String]) = APIAuthAction.async { req =>
+    implicit val username = Option(req.user.email)
     val editionSearch = editions.map(_.split(",").toList)
     val tagSearch: Option[List[Long]] = tagIds.map(_.split(",").toList.filter(_.length > 0).map(_.toLong))
     val sectionSearch: Option[List[Long]] = sectionIds.map(_.split(",").toList.filter(_.length > 0).map(_.toLong))

app/model/AppAudit.scala

@@ -35,14 +35,14 @@ object AppAudit extends Logging {
   }
 
   def reindexTags()(implicit username: Option[String] = None): AppAudit = {
-    AppAudit("reindexTags", new DateTime(), username.getOrElse("default user"), "tag reindex started");
+    AppAudit("reindexTags", new DateTime(), username.getOrElse("unknown"), "tag reindex started");
   }
 
   def reindexSections()(implicit username: Option[String] = None): AppAudit = {
-    AppAudit("reindexSections", new DateTime(), username.getOrElse("default user"), "section reindex started");
+    AppAudit("reindexSections", new DateTime(), username.getOrElse("unknown"), "section reindex started");
   }
 
   def reindexPillars()(implicit username: Option[String] = None): AppAudit = {
-    AppAudit("reindexPillars", new DateTime(), username.getOrElse("default user"), "pillar reindex started");
+    AppAudit("reindexPillars", new DateTime(), username.getOrElse("unknown"), "pillar reindex started");
   }
 }

app/model/PillarAudit.scala

@@ -45,11 +45,11 @@ object PillarAudit extends Logging {
     }
   }
 
-  def created(pillar: Pillar)(implicit user: Option[String] = None): PillarAudit = {
-    PillarAudit(pillar.id, "created", new DateTime(), user.getOrElse("default user"), s"pillar '${pillar.name}' created", pillar)
+  def created(pillar: Pillar)(implicit user: Option[String]): PillarAudit = {
+    PillarAudit(pillar.id, "created", new DateTime(), user.getOrElse("unknown"), s"pillar '${pillar.name}' created", pillar)
   }
 
-  def updated(pillar: Pillar)(implicit user: Option[String] = None): PillarAudit = {
-    PillarAudit(pillar.id, "updated", new DateTime(), user.getOrElse("default user"), s"pillar '${pillar.name}' updated", pillar)
+  def updated(pillar: Pillar)(implicit user: Option[String]): PillarAudit = {
+    PillarAudit(pillar.id, "updated", new DateTime(), user.getOrElse("unknown"), s"pillar '${pillar.name}' updated", pillar)
   }
 }

app/model/SectionAudit.scala

@@ -45,20 +45,20 @@ object SectionAudit extends Logging {
     }
   }
 
-  def created(section: Section)(implicit user: Option[String] = None): SectionAudit = {
-    SectionAudit(section.id, "created", new DateTime(), user.getOrElse("default user"), s"section '${section.name}' created", SectionSummary(section))
+  def created(section: Section)(implicit user: Option[String]): SectionAudit = {
+    SectionAudit(section.id, "created", new DateTime(), user.getOrElse("unknown"), s"section '${section.name}' created", SectionSummary(section))
   }
 
-  def updated(section: Section)(implicit user: Option[String] = None): SectionAudit = {
-    SectionAudit(section.id, "updated", new DateTime(), user.getOrElse("default user"), s"section '${section.name}' updated", SectionSummary(section))
+  def updated(section: Section)(implicit user: Option[String]): SectionAudit = {
+    SectionAudit(section.id, "updated", new DateTime(), user.getOrElse("unknown"), s"section '${section.name}' updated", SectionSummary(section))
   }
 
-  def addedEdition(section: Section, editionName: String)(implicit user: Option[String] = None): SectionAudit = {
-    SectionAudit(section.id, "added edition", new DateTime(), user.getOrElse("default user"), s"added ${editionName} edition to section '${section.name}", SectionSummary(section))
+  def addedEdition(section: Section, editionName: String)(implicit user: Option[String]): SectionAudit = {
+    SectionAudit(section.id, "added edition", new DateTime(), user.getOrElse("unknown"), s"added ${editionName} edition to section '${section.name}", SectionSummary(section))
   }
 
-  def removedEdition(section: Section, editionName: String)(implicit user: Option[String] = None): SectionAudit = {
-    SectionAudit(section.id, "removed edition", new DateTime(), user.getOrElse("default user"), s"removed ${editionName} edition from section '${section.name}", SectionSummary(section))
+  def removedEdition(section: Section, editionName: String)(implicit user: Option[String]): SectionAudit = {
+    SectionAudit(section.id, "removed edition", new DateTime(), user.getOrElse("unknown"), s"removed ${editionName} edition from section '${section.name}", SectionSummary(section))
   }
 }
 

app/model/TagAudit.scala

@@ -49,35 +49,35 @@ object TagAudit extends Logging {
     }
   }
 
-  def created(tag: Tag)(implicit username: Option[String] = None): TagAudit = {
-    TagAudit(tag.id, "created", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' created", TagSummary(tag), None)
+  def created(tag: Tag)(implicit username: Option[String]): TagAudit = {
+    TagAudit(tag.id, "created", new DateTime(), username.getOrElse("unknown"), s"tag '${tag.internalName}' created", TagSummary(tag), None)
   }
 
-  def updated(tag: Tag)(implicit username: Option[String] = None): TagAudit = {
-    TagAudit(tag.id, "updated", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' updated", TagSummary(tag), None)
+  def updated(tag: Tag)(implicit username: Option[String]): TagAudit = {
+    TagAudit(tag.id, "updated", new DateTime(), username.getOrElse("unknown"), s"tag '${tag.internalName}' updated", TagSummary(tag), None)
   }
 
-  def deleted(tag: Tag, username: Option[String] = None): TagAudit = {
-    TagAudit(tag.id, "deleted", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' deleted", TagSummary(tag), None)
+  def deleted(tag: Tag, username: Option[String]): TagAudit = {
+    TagAudit(tag.id, "deleted", new DateTime(), username.getOrElse("unknown"), s"tag '${tag.internalName}' deleted", TagSummary(tag), None)
   }
 
   def merged(removingTag: Tag, replacementTag: Tag, username: Option[String]): TagAudit = {
     TagAudit(removingTag.id,
       "merged" ,
       new DateTime(),
-      username.getOrElse("default user"),
+      username.getOrElse("unknown"),
       s"tag '${removingTag.internalName}' merged with '${replacementTag.internalName}'",
       TagSummary(removingTag),
       Some(TagSummary(replacementTag))
     )
   }
 
-  def batchTag(tag: Tag, operation: String, contentCount: Int)(implicit user: Option[User] = None): TagAudit = {
+  def batchTag(tag: Tag, operation: String, contentCount: Int)(implicit user: Option[String]): TagAudit = {
     val message = operation match {
       case "remove" => s"tag '${tag.internalName}' removed from $contentCount items(s)"
       case _ => s"tag '${tag.internalName}' added to $contentCount items(s)"
     }
-    TagAudit(tag.id, "batchtag", new DateTime(), user.map(_.email).getOrElse("default user"), message, TagSummary(tag), None)
+    TagAudit(tag.id, "batchtag", new DateTime(), user.getOrElse("unknown"), message, TagSummary(tag), None)
   }
 
 

app/model/command/AddEditionToSectionCommand.scala

@@ -15,7 +15,7 @@ case class AddEditionToSectionCommand(sectionId: Long, editionName: String) exte
 
   type T = Section
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Section]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Section]] = Future{
     logger.info(s"add $editionName to section $sectionId")
 
     val section = SectionRepository.getSection(sectionId).getOrElse(SectionNotFound)

app/model/command/BatchTagCommand.scala

@@ -11,7 +11,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class BatchTagCommand(contentIds: List[String], toAddToTop: Option[Long], toAddToBottom: List[Long], toRemove: List[Long]) extends Command {
   type T = Unit
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Unit]] = Future {
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Unit]] = Future {
     val toTopList = toAddToTop.toList
 
     // We'd prefer if people didn't add and remove

app/model/command/ClashingSponsorshipsFetch.scala

@@ -12,7 +12,7 @@ class ClashingSponsorshipsFetch(id: Option[Long], tagIds: Option[List[Long]], se
 
   type T = List[Sponsorship]
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[List[Sponsorship]]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[List[Sponsorship]]] = Future{
 
     val targetedSponsorships = (tagIds.map{ tids =>
       tids.flatMap{ tagId => SponsorshipRepository.searchSponsorships(SponsorshipSearchCriteria(tagId = Some(tagId)))}

app/model/command/Command.scala

@@ -5,5 +5,5 @@ import scala.concurrent.{ExecutionContext, Future}
 trait Command {
   type T
 
-  def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]]
+  def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]]
 }

app/model/command/CreatePillarCommand.scala

@@ -18,7 +18,7 @@ case class CreatePillarCommand(
 
   type T = Pillar
 
-  def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Pillar]] = {
+  def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Pillar]] = {
 
     val pageIdFuture: Future[Long] = Future { try { PathManager.registerPathAndGetPageId(path) } catch {
       case p: PathRegistrationFailed => PathInUse

app/model/command/CreateSectionCommand.scala

@@ -21,7 +21,7 @@ case class CreateSectionCommand(
 
           type T = Section
 
-          def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Section]] = {
+          def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Section]] = {
 
             val calculatedPath = wordsForUrl
 

app/model/command/CreateTagCommand.scala

@@ -86,7 +86,7 @@ case class CreateTagCommand(
 
   type T = Tag
 
-  def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Tag]] = Future {
+  def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Tag]] = Future {
 
     val tagId = Sequences.tagId.getNextId
 

app/model/command/DeletePillarCommand.scala

@@ -10,7 +10,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class DeletePillarCommand(id: Long) extends Command {
   override type T = Long
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Long]] = Future {
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Long]] = Future {
     for {
       pillar <- PillarRepository.getPillar(id)
       _ <- PillarRepository.deletePillar(id)

app/model/command/DeleteTagCommand.scala

@@ -10,7 +10,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class DeleteTagCommand(removingTagId: Long) extends Command {
   override type T = Unit
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
     val removingTag = TagRepository.getTag(removingTagId) getOrElse(TagNotFound)
 
     JobHelper.beginTagDeletion(removingTag)

app/model/command/ExpireSectionContentCommand.scala

@@ -12,7 +12,7 @@ case class ExpireSectionContentCommand(sectionId: Long) extends Command with Log
 
   type T = Section
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Section]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Section]] = Future{
     logger.info(s"Expiring Content for Section: $sectionId")
 
     SectionRepository.getSection(sectionId).map(section => {

app/model/command/FlexTagReindexCommand.scala

@@ -12,7 +12,7 @@ case class FlexTagReindexCommand(tag: Tag) extends Command with Logging {
 
   type T = Tag
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]] = {
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = {
     Future {
       val contentIds = ContentAPI.getContentIdsForTag(tag.path)
 

app/model/command/MergeTagCommand.scala

@@ -12,7 +12,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class MergeTagCommand(removingTagId: Long, replacementTagId: Long) extends Command {
   override type T = Unit
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
     if (removingTagId == replacementTagId) {
       AttemptedSelfMergeTag
     }

app/model/command/PathUsageCheck.scala

@@ -10,7 +10,7 @@ class PathUsageCheck(tagType: String, slug: String, sectionId: Option[Long], tag
 
   type T = Map[String, Boolean]
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Some[Map[String, Boolean]]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Some[Map[String, Boolean]]] = Future{
     val calculatedPath = TagPathCalculator calculatePath(tagType, slug, sectionId, tagSubType)
 
     val inUse = PathManager isPathInUse(calculatedPath)

app/model/command/ReindexPillarsCommand.scala

@@ -7,7 +7,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class ReindexPillarsCommand() extends Command {
   override type T = Unit
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
     JobHelper.beginPillarReindex
     Some(())
   }

app/model/command/ReindexSectionsCommand.scala

@@ -7,7 +7,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class ReindexSectionsCommand() extends Command {
   override type T = Unit
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
     JobHelper.beginSectionReindex
     Some(())
   }

app/model/command/ReindexTagsCommand.scala

@@ -7,7 +7,7 @@ import scala.concurrent.{Future, ExecutionContext}
 case class ReindexTagsCommand() extends Command {
   override type T = Unit
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[T]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[T]] = Future{
     JobHelper.beginTagReindex
     Some(())
   }

app/model/command/RemoveEditionFromSectionCommand.scala

@@ -14,7 +14,7 @@ case class RemoveEditionFromSectionCommand(sectionId: Long, editionName: String)
 
   type T = Section
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Section]] = Future {
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Section]] = Future {
     logger.info(s"removing $editionName from section $sectionId")
 
     val section = SectionRepository.getSection(sectionId).getOrElse(SectionNotFound)

app/model/command/UnexpireSectionContentCommand.scala

@@ -12,7 +12,7 @@ case class UnexpireSectionContentCommand(sectionId: Long) extends Command with L
 
   type T = Section
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Section]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Section]] = Future{
     logger.info(s"Unexpiring Content for Section: $sectionId")
 
     SectionRepository.getSection(sectionId).map(section => {

app/model/command/UpdatePillarCommand.scala

@@ -13,7 +13,7 @@ case class UpdatePillarCommand(pillar: Pillar) extends Command with Logging {
 
   type T = Pillar
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Pillar]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Pillar]] = Future{
     logger.info(s"updating pillar ${pillar.id}")
 
     val result = PillarRepository.updatePillar(pillar)

app/model/command/UpdateSectionCommand.scala

@@ -13,7 +13,7 @@ case class UpdateSectionCommand(section: Section) extends Command with Logging {
 
   type T = Section
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Section]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Section]] = Future{
     logger.info(s"updating section ${section.id}")
 
     val result = SectionRepository.updateSection(section)

app/model/command/UpdateTagCommand.scala

@@ -15,7 +15,7 @@ case class UpdateTagCommand(denormalisedTag: DenormalisedTag) extends Command wi
 
   type T = Tag
 
-  override def process()(implicit username: Option[String] = None, ec: ExecutionContext): Future[Option[Tag]] = Future{
+  override def process()(implicit username: Option[String], ec: ExecutionContext): Future[Option[Tag]] = Future{
     val (tag, sponsorship) = denormalisedTag.normalise()
 
     logger.info(s"updating tag ${tag.id}")

app/model/jobs/steps/ModifyContentTags.scala

@@ -39,7 +39,6 @@ case class ModifyContentTags(
       KinesisStreams.taggingOperationsStream.publishUpdate(contentPath.take(MAX_PARTITION_KEY_LENGTH), taggingOperation)
       logger.info(s"raising $op for ${tag.id} on $contentPath operation")
     }
-    TagAuditRepository.upsertTagAudit(TagAudit.batchTag(tag, op, contentIds.length))
   }
 
   override def waitDuration: Option[Duration] = {

app/model/jobs/steps/RemoveTagFromContent.scala

@@ -33,7 +33,6 @@ case class RemoveTagFromContent(
       KinesisStreams.taggingOperationsStream.publishUpdate(contentPath.take(128), taggingOperation)
       logger.info(s"raising ${OperationType.Remove} for ${tag.id} on $contentPath operation")
     }
-    TagAuditRepository.upsertTagAudit(TagAudit.batchTag(tag, "remove", contentIds.length))
   }
 
   override def waitDuration: Option[Duration] = {
@@ -59,6 +58,7 @@ case class RemoveTagFromContent(
       )
       KinesisStreams.taggingOperationsStream.publishUpdate(contentPath.take(200), taggingOperation)
     }
+    implicit val username: Option[String] = None // unfortunately we don't have easy access to job.createdBy here
     TagAuditRepository.upsertTagAudit(TagAudit.batchTag(tag, OperationType.AddToBottom.toString, contentIds.length))
   }