Merge pull request #1403 from guardian/fp/remove-subtitles-permission

Remove subtitles permission

Author: Fweddi

app/controllers/UploadController.scala

@@ -142,93 +142,80 @@ class UploadController(
       assetVersion: Int
   ): Action[MultipartFormData[Files.TemporaryFile]] =
     LookupPermissions(parse.multipartFormData) { implicit req =>
-      if (!req.permissions.addSubtitles) {
-        Unauthorized(
-          s"User ${req.user.email} is not authorised to upload subtitle asset"
-        )
-      } else {
-        log.info(
-          s"Request to upload subtitle file under atom=$atomId assetVersion=$assetVersion"
-        )
-        val result = for {
-          file <- req.body.file("subtitle-file")
-          upload <- uploadDecorator.getUpload(s"$atomId-$assetVersion")
-        } yield try {
-          val subtitleSource = SubtitleFileUploadCommand(
-            upload,
-            file,
-            stores,
-            req.user,
-            awsConfig
-          ).process()
-
-          // add subtitle file to upload asset's list of sources
-          val rerunUpload =
-            UploadBuilder.buildForSubtitleChange(upload, Some(subtitleSource))
-
-          // reprocessing will also save the upload to the DB, but saving it here first improves UI responsiveness
-          saveUploadToDb(rerunUpload)
-
-          processSubtitleChange(rerunUpload)
-
-          log.info(
-            s"Upload being reprocessed after subtitle upload ${upload.id}"
-          )
-          Ok(Json.toJson(rerunUpload))
-        } catch {
-          commandExceptionAsResult
-        }
+      log.info(
+        s"Request to upload subtitle file under atom=$atomId assetVersion=$assetVersion"
+      )
+      val result = for {
+        file <- req.body.file("subtitle-file")
+        upload <- uploadDecorator.getUpload(s"$atomId-$assetVersion")
+      } yield try {
+        val subtitleSource = SubtitleFileUploadCommand(
+          upload,
+          file,
+          stores,
+          req.user,
+          awsConfig
+        ).process()
+
+        // add subtitle file to upload asset's list of sources
+        val rerunUpload =
+          UploadBuilder.buildForSubtitleChange(upload, Some(subtitleSource))
+
+        // reprocessing will also save the upload to the DB, but saving it here first improves UI responsiveness
+        saveUploadToDb(rerunUpload)
+
+        processSubtitleChange(rerunUpload)
 
-        result.getOrElse(
-          BadRequest
+        log.info(
+          s"Upload being reprocessed after subtitle upload ${upload.id}"
         )
+        Ok(Json.toJson(rerunUpload))
+      } catch {
+        commandExceptionAsResult
       }
+
+      result.getOrElse(
+        BadRequest
+      )
     }
 
   def deleteSubtitleFile(
       atomId: String,
       assetVersion: Int
   ): Action[AnyContent] =
     LookupPermissions { implicit req =>
-      if (!req.permissions.addSubtitles) {
-        Unauthorized(
-          s"User ${req.user.email} is not authorised to upload subtitle asset"
-        )
-      } else {
-        log.info(
-          s"Request to delete subtitle file under atom=$atomId assetVersion=$assetVersion"
-        )
-        uploadDecorator.getUpload(s"$atomId-$assetVersion") match {
-          case Some(upload) =>
-            try {
-              SubtitleFileDeleteCommand(
-                upload,
-                stores,
-                req.user,
-                awsConfig
-              ).process()
-
-              // remove subtitle files from upload asset's list of sources
-              val rerunUpload =
-                UploadBuilder.buildForSubtitleChange(upload, None)
-
-              // reprocessing will also save the upload to the DB, but saving it here first improves UI responsiveness
-              saveUploadToDb(rerunUpload)
-
-              processSubtitleChange(rerunUpload)
-
-              log.info(
-                s"Upload being reprocessed after subtitle deletion ${upload.id}"
-              )
-              Ok(Json.toJson(rerunUpload))
-            } catch {
-              commandExceptionAsResult
-            }
-          case _ =>
-            NotFound
-        }
+      log.info(
+        s"Request to delete subtitle file under atom=$atomId assetVersion=$assetVersion"
+      )
+      uploadDecorator.getUpload(s"$atomId-$assetVersion") match {
+        case Some(upload) =>
+          try {
+            SubtitleFileDeleteCommand(
+              upload,
+              stores,
+              req.user,
+              awsConfig
+            ).process()
+
+            // remove subtitle files from upload asset's list of sources
+            val rerunUpload =
+              UploadBuilder.buildForSubtitleChange(upload, None)
+
+            // reprocessing will also save the upload to the DB, but saving it here first improves UI responsiveness
+            saveUploadToDb(rerunUpload)
+
+            processSubtitleChange(rerunUpload)
+
+            log.info(
+              s"Upload being reprocessed after subtitle deletion ${upload.id}"
+            )
+            Ok(Json.toJson(rerunUpload))
+          } catch {
+            commandExceptionAsResult
+          }
+        case _ =>
+          NotFound
       }
-
     }
 
   @tailrec

common/src/main/scala/com/gu/media/Permissions.scala

@@ -11,7 +11,6 @@ import com.gu.permissions.PermissionDefinition
 case class Permissions(
     deleteAtom: Boolean = false,
     addSelfHostedAsset: Boolean = false,
-    addSubtitles: Boolean = false,
     setVideosOnAllChannelsPublic: Boolean = false,
     pinboard: Boolean = false,
     videoPlayerFormat: Boolean = false
@@ -23,7 +22,6 @@ object Permissions {
   val basicAccess = PermissionDefinition("media_atom_maker_access", app)
   val deleteAtom = PermissionDefinition("delete_atom", app)
   val addSelfHostedAsset = PermissionDefinition("add_self_hosted_asset", app)
-  val addSubtitles = PermissionDefinition("add_subtitles", app)
   val setVideosOnAllChannelsPublic =
     PermissionDefinition("set_videos_on_all_channels_public", app)
   val pinboard = PermissionDefinition("pinboard", "pinboard")
@@ -44,7 +42,6 @@ class MediaAtomMakerPermissionsProvider(
   def getAll(user: PandaUser): Permissions = Permissions(
     deleteAtom = hasPermission(deleteAtom, user),
     addSelfHostedAsset = hasPermission(addSelfHostedAsset, user),
-    addSubtitles = hasPermission(addSubtitles, user),
     setVideosOnAllChannelsPublic =
       hasPermission(setVideosOnAllChannelsPublic, user),
     pinboard = hasPermission(pinboard, user),

public/video-ui/src/components/VideoUpload/VideoAsset.test.tsx

@@ -17,7 +17,6 @@ const defaultProps = {
   deleteAsset: jest.fn(),
   startSubtitleFileUpload: jest.fn(),
   deleteSubtitle: jest.fn(),
-  permissions: {},
   activatingAssetNumber: undefined as number
 };
 

public/video-ui/src/components/VideoUpload/VideoAsset.tsx

@@ -263,15 +263,13 @@ export function Asset({
   isActive,
   selectAsset,
   deleteAsset,
-  permissions,
   activatingAssetNumber
 }: {
   videoId: string;
   upload: Upload;
   isActive: boolean;
   selectAsset: { (): void };
   deleteAsset: { (): void };
-  permissions: Record<string, boolean>;
   activatingAssetNumber: number;
 }) {
   const dispatch = useDispatch<AppDispatch>();
@@ -285,7 +283,7 @@ export function Asset({
   const isSelfHosted = asset && asset.sources;
   const subtitleFilename = metadata?.subtitleFilename;
 
-  const subtitlePanel = isSelfHosted && permissions?.addSubtitles && (
+  const subtitlePanel = isSelfHosted && (
     <div className="video-trail__item__subtitles">
       <div className="subtitle__container">
         <SubtitlesIcon />

public/video-ui/src/components/VideoUpload/VideoTrail.tsx

@@ -12,15 +12,13 @@ type Props = {
   video: Video;
   uploads: Upload[];
   setAsset: (version: number) => void;
-  permissions: Record<string, boolean>;
   activatingAssetNumber: number;
 };
 
 export const VideoTrail = ({
   video,
   uploads,
   setAsset,
-  permissions,
   activatingAssetNumber
 }: Props) => {
   const dispatch = useDispatch<AppDispatch>();
@@ -78,7 +76,6 @@ export const VideoTrail = ({
               isActive={parseInt(upload.id) === video.activeVersion}
               selectAsset={() => setAsset(parseInt(upload.id))}
               deleteAsset={() => deleteAssetsInUpload(upload.asset)}
-              permissions={permissions}
               activatingAssetNumber={activatingAssetNumber}
             />
           ))}

public/video-ui/src/pages/Upload/index.tsx

@@ -108,7 +108,6 @@ export const VideoUpload = (props: { params: { id: string } }) => {
             video={store.video}
             uploads={store.uploads}
             setAsset={setAsset}
-            permissions={store.config.permissions}
             activatingAssetNumber={store.activatingAssetNumber}
           />
         </div>