From 12bf9bcfeb329380a420dac51e22e1da11544172 Mon Sep 17 00:00:00 2001
From: David Furey <david.furey@guardian.co.uk>
Date: Wed, 3 Jan 2024 10:23:18 +0000
Subject: [PATCH 1/5] Username is passed around as string not as `User` object

Batch tags are currently always logged as "default user" because this function never receives an implicit User object.

It feels unpleasant to add another implicit String parameter, since this means the function is effectively saying to its caller "Hey, do you have a string handy that I can pretend is a username?". It would be nicer if this was passed as a User object.

This issue also highlights how easy it is for bugs to creep in with implicit parameters that have default values.
---
 app/model/TagAudit.scala | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/model/TagAudit.scala b/app/model/TagAudit.scala
index 148d720b..1d9ec1d3 100644
--- a/app/model/TagAudit.scala
+++ b/app/model/TagAudit.scala
@@ -72,12 +72,12 @@ object TagAudit extends Logging {
     )
   }
 
-  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] = None): 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("default user"), message, TagSummary(tag), None)
   }
 
 

From c24df15a1b8ac496874e57689bb481d1dae761d9 Mon Sep 17 00:00:00 2001
From: David Furey <david.furey@guardian.co.uk>
Date: Wed, 3 Jan 2024 10:41:43 +0000
Subject: [PATCH 2/5] Be more explicit about missing username information in
 audit log

---
 app/model/TagAudit.scala                        | 2 +-
 app/model/jobs/steps/RemoveTagFromContent.scala | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/model/TagAudit.scala b/app/model/TagAudit.scala
index 1d9ec1d3..fa952ca6 100644
--- a/app/model/TagAudit.scala
+++ b/app/model/TagAudit.scala
@@ -72,7 +72,7 @@ object TagAudit extends Logging {
     )
   }
 
-  def batchTag(tag: Tag, operation: String, contentCount: Int)(implicit user: Option[String] = 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)"
diff --git a/app/model/jobs/steps/RemoveTagFromContent.scala b/app/model/jobs/steps/RemoveTagFromContent.scala
index 96827d4a..f19b76d8 100644
--- a/app/model/jobs/steps/RemoveTagFromContent.scala
+++ b/app/model/jobs/steps/RemoveTagFromContent.scala
@@ -59,6 +59,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))
   }
 

From c7c8863ac689f613847b6bfe411667b0e79ee961 Mon Sep 17 00:00:00 2001
From: David Furey <david.furey@guardian.co.uk>
Date: Wed, 3 Jan 2024 10:45:38 +0000
Subject: [PATCH 3/5] Already logged in `buildBatchTagJob`

---
 app/model/jobs/steps/ModifyContentTags.scala    | 1 -
 app/model/jobs/steps/RemoveTagFromContent.scala | 1 -
 2 files changed, 2 deletions(-)

diff --git a/app/model/jobs/steps/ModifyContentTags.scala b/app/model/jobs/steps/ModifyContentTags.scala
index f59aa1cb..8861b898 100644
--- a/app/model/jobs/steps/ModifyContentTags.scala
+++ b/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] = {
diff --git a/app/model/jobs/steps/RemoveTagFromContent.scala b/app/model/jobs/steps/RemoveTagFromContent.scala
index f19b76d8..b543ddf4 100644
--- a/app/model/jobs/steps/RemoveTagFromContent.scala
+++ b/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] = {

From 0e25c15abb95c3d501b86a1763745826b03b0921 Mon Sep 17 00:00:00 2001
From: David Furey <david.furey@guardian.co.uk>
Date: Wed, 3 Jan 2024 10:55:58 +0000
Subject: [PATCH 4/5] Remove default `None` to identify places we could be
 logging username

---
 app/controllers/Reindex.scala                           | 3 +++
 app/controllers/TagManagementApi.scala                  | 2 ++
 app/model/AppAudit.scala                                | 6 +++---
 app/model/PillarAudit.scala                             | 4 ++--
 app/model/SectionAudit.scala                            | 8 ++++----
 app/model/TagAudit.scala                                | 6 +++---
 app/model/command/AddEditionToSectionCommand.scala      | 2 +-
 app/model/command/BatchTagCommand.scala                 | 2 +-
 app/model/command/ClashingSponsorshipsFetch.scala       | 2 +-
 app/model/command/Command.scala                         | 2 +-
 app/model/command/CreatePillarCommand.scala             | 2 +-
 app/model/command/CreateSectionCommand.scala            | 2 +-
 app/model/command/CreateTagCommand.scala                | 2 +-
 app/model/command/DeletePillarCommand.scala             | 2 +-
 app/model/command/DeleteTagCommand.scala                | 2 +-
 app/model/command/ExpireSectionContentCommand.scala     | 2 +-
 app/model/command/FlexTagReindexCommand.scala           | 2 +-
 app/model/command/MergeTagCommand.scala                 | 2 +-
 app/model/command/PathUsageCheck.scala                  | 2 +-
 app/model/command/ReindexPillarsCommand.scala           | 2 +-
 app/model/command/ReindexSectionsCommand.scala          | 2 +-
 app/model/command/ReindexTagsCommand.scala              | 2 +-
 app/model/command/RemoveEditionFromSectionCommand.scala | 2 +-
 app/model/command/UnexpireSectionContentCommand.scala   | 2 +-
 app/model/command/UpdatePillarCommand.scala             | 2 +-
 app/model/command/UpdateSectionCommand.scala            | 2 +-
 app/model/command/UpdateTagCommand.scala                | 2 +-
 27 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/app/controllers/Reindex.scala b/app/controllers/Reindex.scala
index 0d942715..f09deb29 100644
--- a/app/controllers/Reindex.scala
+++ b/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)
diff --git a/app/controllers/TagManagementApi.scala b/app/controllers/TagManagementApi.scala
index 64211ab1..28c8c396 100644
--- a/app/controllers/TagManagementApi.scala
+++ b/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))
diff --git a/app/model/AppAudit.scala b/app/model/AppAudit.scala
index d454afff..85c2312b 100644
--- a/app/model/AppAudit.scala
+++ b/app/model/AppAudit.scala
@@ -34,15 +34,15 @@ object AppAudit extends Logging {
     }
   }
 
-  def reindexTags()(implicit username: Option[String] = None): AppAudit = {
+  def reindexTags()(implicit username: Option[String]): AppAudit = {
     AppAudit("reindexTags", new DateTime(), username.getOrElse("default user"), "tag reindex started");
   }
 
-  def reindexSections()(implicit username: Option[String] = None): AppAudit = {
+  def reindexSections()(implicit username: Option[String]): AppAudit = {
     AppAudit("reindexSections", new DateTime(), username.getOrElse("default user"), "section reindex started");
   }
 
-  def reindexPillars()(implicit username: Option[String] = None): AppAudit = {
+  def reindexPillars()(implicit username: Option[String]): AppAudit = {
     AppAudit("reindexPillars", new DateTime(), username.getOrElse("default user"), "pillar reindex started");
   }
 }
diff --git a/app/model/PillarAudit.scala b/app/model/PillarAudit.scala
index 3f1783c3..83ed0205 100644
--- a/app/model/PillarAudit.scala
+++ b/app/model/PillarAudit.scala
@@ -45,11 +45,11 @@ object PillarAudit extends Logging {
     }
   }
 
-  def created(pillar: Pillar)(implicit user: Option[String] = None): PillarAudit = {
+  def created(pillar: Pillar)(implicit user: Option[String]): PillarAudit = {
     PillarAudit(pillar.id, "created", new DateTime(), user.getOrElse("default user"), s"pillar '${pillar.name}' created", pillar)
   }
 
-  def updated(pillar: Pillar)(implicit user: Option[String] = None): PillarAudit = {
+  def updated(pillar: Pillar)(implicit user: Option[String]): PillarAudit = {
     PillarAudit(pillar.id, "updated", new DateTime(), user.getOrElse("default user"), s"pillar '${pillar.name}' updated", pillar)
   }
 }
diff --git a/app/model/SectionAudit.scala b/app/model/SectionAudit.scala
index cc2dbc66..2536e33f 100644
--- a/app/model/SectionAudit.scala
+++ b/app/model/SectionAudit.scala
@@ -45,19 +45,19 @@ object SectionAudit extends Logging {
     }
   }
 
-  def created(section: Section)(implicit user: Option[String] = None): SectionAudit = {
+  def created(section: Section)(implicit user: Option[String]): SectionAudit = {
     SectionAudit(section.id, "created", new DateTime(), user.getOrElse("default user"), s"section '${section.name}' created", SectionSummary(section))
   }
 
-  def updated(section: Section)(implicit user: Option[String] = None): SectionAudit = {
+  def updated(section: Section)(implicit user: Option[String]): SectionAudit = {
     SectionAudit(section.id, "updated", new DateTime(), user.getOrElse("default user"), s"section '${section.name}' updated", SectionSummary(section))
   }
 
-  def addedEdition(section: Section, editionName: String)(implicit user: Option[String] = None): SectionAudit = {
+  def addedEdition(section: Section, editionName: String)(implicit user: Option[String]): SectionAudit = {
     SectionAudit(section.id, "added edition", new DateTime(), user.getOrElse("default user"), s"added ${editionName} edition to section '${section.name}", SectionSummary(section))
   }
 
-  def removedEdition(section: Section, editionName: String)(implicit user: Option[String] = None): SectionAudit = {
+  def removedEdition(section: Section, editionName: String)(implicit user: Option[String]): SectionAudit = {
     SectionAudit(section.id, "removed edition", new DateTime(), user.getOrElse("default user"), s"removed ${editionName} edition from section '${section.name}", SectionSummary(section))
   }
 }
diff --git a/app/model/TagAudit.scala b/app/model/TagAudit.scala
index fa952ca6..5757ef7f 100644
--- a/app/model/TagAudit.scala
+++ b/app/model/TagAudit.scala
@@ -49,15 +49,15 @@ object TagAudit extends Logging {
     }
   }
 
-  def created(tag: Tag)(implicit username: Option[String] = None): TagAudit = {
+  def created(tag: Tag)(implicit username: Option[String]): TagAudit = {
     TagAudit(tag.id, "created", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' created", TagSummary(tag), None)
   }
 
-  def updated(tag: Tag)(implicit username: Option[String] = None): TagAudit = {
+  def updated(tag: Tag)(implicit username: Option[String]): TagAudit = {
     TagAudit(tag.id, "updated", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' updated", TagSummary(tag), None)
   }
 
-  def deleted(tag: Tag, username: Option[String] = None): TagAudit = {
+  def deleted(tag: Tag, username: Option[String]): TagAudit = {
     TagAudit(tag.id, "deleted", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' deleted", TagSummary(tag), None)
   }
 
diff --git a/app/model/command/AddEditionToSectionCommand.scala b/app/model/command/AddEditionToSectionCommand.scala
index b3084ef3..9c7dfcbe 100644
--- a/app/model/command/AddEditionToSectionCommand.scala
+++ b/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)
diff --git a/app/model/command/BatchTagCommand.scala b/app/model/command/BatchTagCommand.scala
index 8236bb52..f2509e9e 100644
--- a/app/model/command/BatchTagCommand.scala
+++ b/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
diff --git a/app/model/command/ClashingSponsorshipsFetch.scala b/app/model/command/ClashingSponsorshipsFetch.scala
index af62e8fd..220dfb0e 100644
--- a/app/model/command/ClashingSponsorshipsFetch.scala
+++ b/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)))}
diff --git a/app/model/command/Command.scala b/app/model/command/Command.scala
index dc790ba4..35f94d35 100644
--- a/app/model/command/Command.scala
+++ b/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]]
 }
diff --git a/app/model/command/CreatePillarCommand.scala b/app/model/command/CreatePillarCommand.scala
index ff09e053..f9a1f021 100644
--- a/app/model/command/CreatePillarCommand.scala
+++ b/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
diff --git a/app/model/command/CreateSectionCommand.scala b/app/model/command/CreateSectionCommand.scala
index 6224280b..578684d1 100644
--- a/app/model/command/CreateSectionCommand.scala
+++ b/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
 
diff --git a/app/model/command/CreateTagCommand.scala b/app/model/command/CreateTagCommand.scala
index 58ea8842..c4a4825e 100644
--- a/app/model/command/CreateTagCommand.scala
+++ b/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
 
diff --git a/app/model/command/DeletePillarCommand.scala b/app/model/command/DeletePillarCommand.scala
index 170df267..39616052 100644
--- a/app/model/command/DeletePillarCommand.scala
+++ b/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)
diff --git a/app/model/command/DeleteTagCommand.scala b/app/model/command/DeleteTagCommand.scala
index 5bc518a9..ea1f4d66 100644
--- a/app/model/command/DeleteTagCommand.scala
+++ b/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)
diff --git a/app/model/command/ExpireSectionContentCommand.scala b/app/model/command/ExpireSectionContentCommand.scala
index 828ff2b7..1c01cc78 100644
--- a/app/model/command/ExpireSectionContentCommand.scala
+++ b/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 => {
diff --git a/app/model/command/FlexTagReindexCommand.scala b/app/model/command/FlexTagReindexCommand.scala
index 37cd9f13..00be7b65 100644
--- a/app/model/command/FlexTagReindexCommand.scala
+++ b/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)
 
diff --git a/app/model/command/MergeTagCommand.scala b/app/model/command/MergeTagCommand.scala
index 477e5bd1..ba5cd140 100644
--- a/app/model/command/MergeTagCommand.scala
+++ b/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
     }
diff --git a/app/model/command/PathUsageCheck.scala b/app/model/command/PathUsageCheck.scala
index 27ab5c16..56b743e4 100644
--- a/app/model/command/PathUsageCheck.scala
+++ b/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)
diff --git a/app/model/command/ReindexPillarsCommand.scala b/app/model/command/ReindexPillarsCommand.scala
index 3a4a2b1a..ba04120a 100644
--- a/app/model/command/ReindexPillarsCommand.scala
+++ b/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(())
   }
diff --git a/app/model/command/ReindexSectionsCommand.scala b/app/model/command/ReindexSectionsCommand.scala
index 1ee6443a..ff3022b1 100644
--- a/app/model/command/ReindexSectionsCommand.scala
+++ b/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(())
   }
diff --git a/app/model/command/ReindexTagsCommand.scala b/app/model/command/ReindexTagsCommand.scala
index a99a828d..cf1ed3c7 100644
--- a/app/model/command/ReindexTagsCommand.scala
+++ b/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(())
   }
diff --git a/app/model/command/RemoveEditionFromSectionCommand.scala b/app/model/command/RemoveEditionFromSectionCommand.scala
index 644f07eb..8ab2a338 100644
--- a/app/model/command/RemoveEditionFromSectionCommand.scala
+++ b/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)
diff --git a/app/model/command/UnexpireSectionContentCommand.scala b/app/model/command/UnexpireSectionContentCommand.scala
index 357ffd34..9ec7a2b0 100644
--- a/app/model/command/UnexpireSectionContentCommand.scala
+++ b/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 => {
diff --git a/app/model/command/UpdatePillarCommand.scala b/app/model/command/UpdatePillarCommand.scala
index 0f0232d8..d43b1994 100644
--- a/app/model/command/UpdatePillarCommand.scala
+++ b/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)
diff --git a/app/model/command/UpdateSectionCommand.scala b/app/model/command/UpdateSectionCommand.scala
index 5c4599c8..3ebf46ee 100644
--- a/app/model/command/UpdateSectionCommand.scala
+++ b/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)
diff --git a/app/model/command/UpdateTagCommand.scala b/app/model/command/UpdateTagCommand.scala
index 67936908..2c73cc5f 100644
--- a/app/model/command/UpdateTagCommand.scala
+++ b/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}")

From 2714f6111b30f1a144fb80cbbaf7a019fe664554 Mon Sep 17 00:00:00 2001
From: David Furey <david.furey@guardian.co.uk>
Date: Tue, 9 Jan 2024 10:28:19 +0000
Subject: [PATCH 5/5] Rename default user to unknown

This seems like a better label
---
 app/model/AppAudit.scala     | 12 ++++++------
 app/model/PillarAudit.scala  |  4 ++--
 app/model/SectionAudit.scala |  8 ++++----
 app/model/TagAudit.scala     | 10 +++++-----
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/app/model/AppAudit.scala b/app/model/AppAudit.scala
index 85c2312b..26341799 100644
--- a/app/model/AppAudit.scala
+++ b/app/model/AppAudit.scala
@@ -34,15 +34,15 @@ object AppAudit extends Logging {
     }
   }
 
-  def reindexTags()(implicit username: Option[String]): AppAudit = {
-    AppAudit("reindexTags", new DateTime(), username.getOrElse("default user"), "tag reindex started");
+  def reindexTags()(implicit username: Option[String] = None): AppAudit = {
+    AppAudit("reindexTags", new DateTime(), username.getOrElse("unknown"), "tag reindex started");
   }
 
-  def reindexSections()(implicit username: Option[String]): AppAudit = {
-    AppAudit("reindexSections", new DateTime(), username.getOrElse("default user"), "section reindex started");
+  def reindexSections()(implicit username: Option[String] = None): AppAudit = {
+    AppAudit("reindexSections", new DateTime(), username.getOrElse("unknown"), "section reindex started");
   }
 
-  def reindexPillars()(implicit username: Option[String]): AppAudit = {
-    AppAudit("reindexPillars", new DateTime(), username.getOrElse("default user"), "pillar reindex started");
+  def reindexPillars()(implicit username: Option[String] = None): AppAudit = {
+    AppAudit("reindexPillars", new DateTime(), username.getOrElse("unknown"), "pillar reindex started");
   }
 }
diff --git a/app/model/PillarAudit.scala b/app/model/PillarAudit.scala
index 83ed0205..5d5359ca 100644
--- a/app/model/PillarAudit.scala
+++ b/app/model/PillarAudit.scala
@@ -46,10 +46,10 @@ object PillarAudit extends Logging {
   }
 
   def created(pillar: Pillar)(implicit user: Option[String]): PillarAudit = {
-    PillarAudit(pillar.id, "created", new DateTime(), user.getOrElse("default user"), s"pillar '${pillar.name}' created", pillar)
+    PillarAudit(pillar.id, "created", new DateTime(), user.getOrElse("unknown"), s"pillar '${pillar.name}' created", pillar)
   }
 
   def updated(pillar: Pillar)(implicit user: Option[String]): PillarAudit = {
-    PillarAudit(pillar.id, "updated", new DateTime(), user.getOrElse("default user"), s"pillar '${pillar.name}' updated", pillar)
+    PillarAudit(pillar.id, "updated", new DateTime(), user.getOrElse("unknown"), s"pillar '${pillar.name}' updated", pillar)
   }
 }
diff --git a/app/model/SectionAudit.scala b/app/model/SectionAudit.scala
index 2536e33f..3265ee7f 100644
--- a/app/model/SectionAudit.scala
+++ b/app/model/SectionAudit.scala
@@ -46,19 +46,19 @@ object SectionAudit extends Logging {
   }
 
   def created(section: Section)(implicit user: Option[String]): SectionAudit = {
-    SectionAudit(section.id, "created", new DateTime(), user.getOrElse("default user"), s"section '${section.name}' created", SectionSummary(section))
+    SectionAudit(section.id, "created", new DateTime(), user.getOrElse("unknown"), s"section '${section.name}' created", SectionSummary(section))
   }
 
   def updated(section: Section)(implicit user: Option[String]): SectionAudit = {
-    SectionAudit(section.id, "updated", new DateTime(), user.getOrElse("default user"), s"section '${section.name}' updated", SectionSummary(section))
+    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]): SectionAudit = {
-    SectionAudit(section.id, "added edition", new DateTime(), user.getOrElse("default user"), s"added ${editionName} edition to section '${section.name}", SectionSummary(section))
+    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]): SectionAudit = {
-    SectionAudit(section.id, "removed edition", new DateTime(), user.getOrElse("default user"), s"removed ${editionName} edition from section '${section.name}", SectionSummary(section))
+    SectionAudit(section.id, "removed edition", new DateTime(), user.getOrElse("unknown"), s"removed ${editionName} edition from section '${section.name}", SectionSummary(section))
   }
 }
 
diff --git a/app/model/TagAudit.scala b/app/model/TagAudit.scala
index 5757ef7f..b5ae6021 100644
--- a/app/model/TagAudit.scala
+++ b/app/model/TagAudit.scala
@@ -50,22 +50,22 @@ object TagAudit extends Logging {
   }
 
   def created(tag: Tag)(implicit username: Option[String]): TagAudit = {
-    TagAudit(tag.id, "created", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' created", TagSummary(tag), None)
+    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]): TagAudit = {
-    TagAudit(tag.id, "updated", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' updated", TagSummary(tag), None)
+    TagAudit(tag.id, "updated", new DateTime(), username.getOrElse("unknown"), s"tag '${tag.internalName}' updated", TagSummary(tag), None)
   }
 
   def deleted(tag: Tag, username: Option[String]): TagAudit = {
-    TagAudit(tag.id, "deleted", new DateTime(), username.getOrElse("default user"), s"tag '${tag.internalName}' deleted", TagSummary(tag), None)
+    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))
@@ -77,7 +77,7 @@ object TagAudit extends Logging {
       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.getOrElse("default user"), message, TagSummary(tag), None)
+    TagAudit(tag.id, "batchtag", new DateTime(), user.getOrElse("unknown"), message, TagSummary(tag), None)
   }