study pgn validation feedback

app/controllers/Study.scala

@@ -274,10 +274,12 @@ object Study extends LilaController {
 
   def importPgn(id: String) = AuthBody { implicit ctx => me =>
     implicit val req = ctx.body
-    lila.study.DataForm.importPgn.form.bindFromRequest.fold(
-      jsonFormError,
-      data => env.api.importPgns(me, StudyModel.Id(id), data.toChapterDatas, sticky = data.sticky)
-    )
+    get("sri") ?? { sri =>
+      lila.study.DataForm.importPgn.form.bindFromRequest.fold(
+        jsonFormError,
+        data => env.api.importPgns(me, StudyModel.Id(id), data.toChapterDatas, sticky = data.sticky, lila.socket.Socket.Sri(sri))
+      )
+    }
   }
 
   def embed(id: String, chapterId: String) = Action.async { implicit req =>

modules/study/src/main/ChapterMaker.scala

@@ -22,7 +22,7 @@ private final class ChapterMaker(
   def apply(study: Study, data: Data, order: Int, userId: User.ID): Fu[Chapter] =
     data.game.??(parseGame) flatMap {
       case None =>
-        data.game.??(pgnFetch.fromUrl) flatMap {
+        data.game ?? pgnFetch.fromUrl flatMap {
           case Some(pgn) => fromFenOrPgnOrBlank(study, data.copy(pgn = pgn.some), order, userId)
           case _ => fromFenOrPgnOrBlank(study, data, order, userId)
         }
@@ -37,31 +37,31 @@ private final class ChapterMaker(
       case None => fuccess(fromFenOrBlank(study, data, order, userId))
     }
 
-  private def fromPgn(study: Study, pgn: String, data: Data, order: Int, userId: User.ID): Fu[Chapter] =
-    lightUser.asyncMany(study.members.contributorIds.toList) map { contributors =>
-      PgnImport(pgn, contributors.flatten).toOption.fold(fromFenOrBlank(study, data, order, userId)) { res =>
-        Chapter.make(
-          studyId = study.id,
-          name = (for {
-            white <- res.tags(_.White)
-            black <- res.tags(_.Black)
-            if data.name.value.isEmpty || Chapter.isDefaultName(data.name)
-          } yield Chapter.Name(s"$white - $black")) | data.name,
-          setup = Chapter.Setup(
-            none,
-            res.variant,
-            data.realOrientation
-          ),
-          root = res.root,
-          tags = res.tags,
-          order = order,
-          ownerId = userId,
-          practice = data.isPractice,
-          gamebook = data.isGamebook,
-          conceal = data.isConceal option Chapter.Ply(res.root.ply)
-        )
-      }
+  private def fromPgn(study: Study, pgn: String, data: Data, order: Int, userId: User.ID): Fu[Chapter] = for {
+    contributors <- lightUser.asyncMany(study.members.contributorIds.toList)
+    parsed <- PgnImport(pgn, contributors.flatten).future recoverWith {
+      case e: Exception => fufail(ValidationException(e.getMessage))
     }
+  } yield Chapter.make(
+    studyId = study.id,
+    name = (for {
+      white <- parsed.tags(_.White)
+      black <- parsed.tags(_.Black)
+      if data.name.value.isEmpty || Chapter.isDefaultName(data.name)
+    } yield Chapter.Name(s"$white - $black")) | data.name,
+    setup = Chapter.Setup(
+      none,
+      parsed.variant,
+      data.realOrientation
+    ),
+    root = parsed.root,
+    tags = parsed.tags,
+    order = order,
+    ownerId = userId,
+    practice = data.isPractice,
+    gamebook = data.isGamebook,
+    conceal = data.isConceal option Chapter.Ply(parsed.root.ply)
+  )
 
   private def fromFenOrBlank(study: Study, data: Data, order: Int, userId: User.ID): Chapter = {
     val variant = data.variant.flatMap(Variant.apply) | Variant.default
@@ -166,6 +166,8 @@ private final class ChapterMaker(
 
 private[study] object ChapterMaker {
 
+  case class ValidationException(message: String) extends lila.base.LilaException
+
   sealed trait Mode {
     def key = toString.toLowerCase
   }

modules/study/src/main/StudyApi.scala

@@ -500,15 +500,19 @@ final class StudyApi(
                 _.filter(_.isEmptyInitial) ?? chapterRepo.delete
               }
             } >> doAddChapter(study, chapter, sticky, sri)
+          } addFailureEffect {
+            case ChapterMaker.ValidationException(error) =>
+              sendTo(study, StudySocket.ValidationError(sri, error))
+            case u => println(u)
           }
         }
       }
     }
   }
 
-  def importPgns(byUser: User, studyId: Study.Id, datas: List[ChapterMaker.Data], sticky: Boolean) =
+  def importPgns(byUser: User, studyId: Study.Id, datas: List[ChapterMaker.Data], sticky: Boolean, sri: Sri) =
     lila.common.Future.applySequentially(datas) { data =>
-      addChapter(byUser.id, studyId, data, sticky, sri = Sri(""))
+      addChapter(byUser.id, studyId, data, sticky, sri)
     }
 
   def doAddChapter(study: Study, chapter: Chapter, sticky: Boolean, sri: Sri) =

modules/study/src/main/StudySocket.scala

@@ -8,13 +8,13 @@ import scala.concurrent.Promise
 
 import chess.Centis
 import chess.format.pgn.Glyphs
+import lila.chat.Chat
 import lila.hub.Trouper
 import lila.socket.actorApi.{ Connected => _, _ }
 import lila.socket.Socket.{ Sri, GetVersion, SocketVersion }
 import lila.socket.{ SocketTrouper, History, Historical, AnaDests }
 import lila.tree.Node.{ Shapes, Comment }
 import lila.user.User
-import lila.chat.Chat
 
 final class StudySocket(
     system: ActorSystem,
@@ -134,6 +134,10 @@ final class StudySocket(
       "s" -> sticky
     ), noMessadata)
 
+    case ValidationError(sri, error) => notifySri("validationError", Json.obj(
+      "error" -> error
+    ))(sri)
+
     case SetShapes(pos, shapes, sri) => notifyVersion("shapes", Json.obj(
       "p" -> pos,
       "s" -> shapes,
@@ -315,6 +319,7 @@ object StudySocket {
   case class DescChapter(sri: Sri, chapterId: Chapter.Id, desc: Option[String])
   case class DescStudy(sri: Sri, desc: Option[String])
   case class AddChapter(sri: Sri, position: Position.Ref, sticky: Boolean)
+  case class ValidationError(sri: Sri, error: String)
   case class SetConceal(position: Position.Ref, ply: Option[Chapter.Ply])
   case class SetLiking(liking: Study.Liking, sri: Sri)
   case class SetTags(chapterId: Chapter.Id, tags: chess.format.pgn.Tags, sri: Sri)

ui/analyse/src/study/chapterNewForm.ts

@@ -86,10 +86,11 @@ export function ctrl(send: SocketSend, chapters: Prop<StudyChapterMeta[]>, setTa
       else open();
     },
     submit(d) {
+      const study = root.study!;
       d.initial = vm.initial();
-      d.sticky = root.study!.vm.mode.sticky;
+      d.sticky = study.vm.mode.sticky;
       if (!d.pgn) send("addChapter", d);
-      else importPgn(root.study!.data.id, d);
+      else importPgn(study.data.id, d, study.sri);
       close();
       setTab();
     },

ui/analyse/src/study/interfaces.ts

@@ -58,6 +58,7 @@ export interface StudyCtrl {
   onPremoveSet(): void;
   redraw: Redraw;
   trans: Trans;
+  sri: string;
 }
 
 export type Tab = 'members' | 'chapters';

ui/analyse/src/study/studyCtrl.ts

@@ -409,6 +409,9 @@ export default function(data: StudyData, ctrl: AnalyseCtrl, tagTypes: TagTypes,
       if (ctrl.path === position.path) ctrl.withCg(cg => cg.setShapes(d.s));
       redraw();
     },
+    validationError(d) {
+      alert(d.error);
+    },
     setComment(d) {
       const position = d.p,
         who = d.w;
@@ -597,6 +600,7 @@ export default function(data: StudyData, ctrl: AnalyseCtrl, tagTypes: TagTypes,
         return true;
       }
       return !!relay && relay.socketHandler(t, d);
-    }
+    },
+    sri
   };
 };

ui/analyse/src/study/studyXhr.ts

@@ -44,10 +44,10 @@ export function practiceComplete(chapterId: string, nbMoves: number) {
   });
 }
 
-export function importPgn(studyId: string, data: any) {
+export function importPgn(studyId: string, data: any, sri: string) {
   return $.ajax({
     method: 'POST',
-    url: `/study/${studyId}/import-pgn`,
+    url: `/study/${studyId}/import-pgn?sri=${sri}`,
     data: data,
     headers
   });