diff --git a/app/controllers/project/Channels.scala b/app/controllers/project/Channels.scala index d126c11ee..4aaf4465a 100755 --- a/app/controllers/project/Channels.scala +++ b/app/controllers/project/Channels.scala @@ -84,8 +84,8 @@ class Channels @Inject()(forms: OreForms, implicit val project: Project = request.data.project val res = for { - channelData <- bindFormEitherT[Future](this.forms.ChannelEdit)(hasErrors => Redirect(self.showList(author, slug)).withError(hasErrors.errors.head.message)) - _ <- channelData.saveTo(channelName).toLeft(()).leftMap(error => Redirect(self.showList(author, slug)).withError(error)) + channelData <- bindFormEitherT[Future](this.forms.ChannelEdit)(hasErrors => Redirect(self.showList(author, slug)).withErrors(hasErrors.errors.flatMap(_.messages))) + _ <- channelData.saveTo(channelName).leftMap(errors => Redirect(self.showList(author, slug)).withErrors(errors)) } yield Redirect(self.showList(author, slug)) res.merge @@ -103,30 +103,26 @@ class Channels @Inject()(forms: OreForms, def delete(author: String, slug: String, channelName: String) = ChannelEditAction(author, slug).async { implicit request => implicit val data = request.data EitherT.right[Status](data.project.channels.all) - .filterOrElse(_.size == 1, Redirect(self.showList(author, slug)).withError("error.channel.last")) + .filterOrElse(_.size != 1, Redirect(self.showList(author, slug)).withError("error.channel.last")) .flatMap { channels => - EitherT.fromEither[Future](channels.find(c => c.name.equals(channelName)).toRight(NotFound)) + EitherT.fromEither[Future](channels.find(_.name == channelName).toRight(NotFound)) .semiFlatMap { channel => - (channel.versions.nonEmpty, Future.sequence(channels.map(_.versions.nonEmpty)).map(l => l.count(_ == true))) + (channel.versions.isEmpty, Future.traverse(channels.toSeq)(_.versions.nonEmpty).map(_.count(identity))) .parTupled .tupleRight(channel) } .filterOrElse( - { case ((nonEmpty, channelCount), _) => nonEmpty && channelCount == 1}, + { case ((emptyChannel, nonEmptyChannelCount), _) => + emptyChannel || nonEmptyChannelCount > 1}, Redirect(self.showList(author, slug)).withError("error.channel.lastNonEmpty") ) .map(_._2) .filterOrElse( - channel => { - val reviewedChannels = channels.filter(!_.isNonReviewed) - !channel.isNonReviewed && reviewedChannels.size <= 1 && reviewedChannels.contains(channel) - }, + channel => channel.isNonReviewed || channels.count(_.isReviewed) > 1, Redirect(self.showList(author, slug)).withError("error.channel.lastReviewed") ) - .map { channel => - this.projects.deleteChannel(channel) - Redirect(self.showList(author, slug)) - } + .semiFlatMap(channel => this.projects.deleteChannel(channel)) + .map(_ => Redirect(self.showList(author, slug))) }.merge } } diff --git a/app/controllers/project/Versions.scala b/app/controllers/project/Versions.scala index 4439f1083..756450f3f 100755 --- a/app/controllers/project/Versions.scala +++ b/app/controllers/project/Versions.scala @@ -420,26 +420,24 @@ class Versions @Inject()(stats: StatTracker, (implicit req: ProjectRequest[_]): Future[Boolean] = { if (version.isReviewed) return Future.successful(true) + // check for confirmation - req.cookies.get(DownloadWarning.COOKIE).map(_.value).orElse(token) match { - case None => - // unconfirmed - Future.successful(false) - case Some(tkn) => + OptionT + .fromOption[Future](req.cookies.get(DownloadWarning.COOKIE).map(_.value).orElse(token)) + .flatMap { tkn => this.warnings.find { warn => (warn.token === tkn) && (warn.versionId === version.id.get) && (warn.address === InetString(StatTracker.remoteAddress)) && warn.isConfirmed - } exists { - warn => - if (!warn.hasExpired) true - else { - warn.remove() - false - } } - } + }.exists { warn => + if (!warn.hasExpired) true + else { + warn.remove() + false + } + } } private def _sendVersion(project: Project, version: Version)(implicit req: ProjectRequest[_]): Future[Result] = { @@ -472,7 +470,7 @@ class Versions @Inject()(stats: StatTracker, implicit val r = request.request val project = request.data.project getVersion(project, target) - .filterOrElse(_.isReviewed, Redirect(ShowProject(author, slug))) + .filterOrElse(v => !v.isReviewed, Redirect(ShowProject(author, slug))) .semiFlatMap { version => // generate a unique "warning" object to ensure the user has landed // on the warning before downloading @@ -530,7 +528,7 @@ class Versions @Inject()(stats: StatTracker, ProjectAction(author, slug) async { request => implicit val r: OreRequest[_] = request.request getVersion(request.data.project, target) - .filterOrElse(_.isReviewed, Redirect(ShowProject(author, slug))) + .filterOrElse(v => !v.isReviewed, Redirect(ShowProject(author, slug))) .flatMap(version => confirmDownload0(version.id.get, downloadType, token).toRight(Redirect(ShowProject(author, slug)))) .map { dl => dl.downloadType match { diff --git a/app/controllers/sugar/ActionHelpers.scala b/app/controllers/sugar/ActionHelpers.scala index 952b87acf..277fc66c2 100644 --- a/app/controllers/sugar/ActionHelpers.scala +++ b/app/controllers/sugar/ActionHelpers.scala @@ -1,7 +1,9 @@ package controllers.sugar import com.google.common.base.Preconditions.{checkArgument, checkNotNull} + import play.api.data.Form +import play.api.i18n.Messages import play.api.mvc.Results.Redirect import play.api.mvc.{Call, Result} @@ -36,6 +38,20 @@ trait ActionHelpers { */ def withError(error: String) = result.flashing("error" -> error) + /** + * Adds one or more errors messages to the result. + * + * @param errors Error messages + * @return Result with errors + */ + //TODO: Use NEL[String] if we get the type + def withErrors(errors: Seq[String])(implicit messages: Messages): Result = errors match { + case Seq() => result + case Seq(single) => withError(messages(single)) + case multiple => + result.flashing("error" -> multiple.map(s => messages(s)).mkString("• ", "
• ", ""), "error-israw" -> "true") + } + /** * Adds a success message to the result. * diff --git a/app/db/impl/access/ProjectBase.scala b/app/db/impl/access/ProjectBase.scala index 5a98f14cb..a1c1904dd 100644 --- a/app/db/impl/access/ProjectBase.scala +++ b/app/db/impl/access/ProjectBase.scala @@ -158,7 +158,7 @@ class ProjectBase(override val service: ModelService, _ = checkArgument(project.id.get == channel.projectId, "invalid project id", "") channels <- project.channels.all noVersion <- channel.versions.isEmpty - nonEmptyChannels <- Future.sequence(channels.map(_.versions.nonEmpty)).map(_.count(_ == true)) + nonEmptyChannels <- Future.traverse(channels.toSeq)(_.versions.nonEmpty).map(_.count(identity)) _ = checkArgument(channels.size > 1, "only one channel", "") _ = checkArgument(noVersion || nonEmptyChannels > 1, "last non-empty channel", "") reviewedChannels = channels.filter(!_.isNonReviewed) diff --git a/app/form/project/TChannelData.scala b/app/form/project/TChannelData.scala index 696a5fa93..3cd0ae163 100644 --- a/app/form/project/TChannelData.scala +++ b/app/form/project/TChannelData.scala @@ -6,6 +6,8 @@ import ore.OreConfig import ore.project.factory.ProjectFactory import util.functional.{EitherT, OptionT} import util.instances.future._ +import util.syntax._ +import util.StringUtils._ import scala.concurrent.{ExecutionContext, Future} @@ -37,9 +39,9 @@ trait TChannelData { */ def addTo(project: Project)(implicit ec: ExecutionContext): EitherT[Future, String, Channel] = { EitherT.liftF(project.channels.all) - .filterOrElse(_.size >= config.projects.get[Int]("max-channels"), "A project may only have up to five channels.") - .filterOrElse(_.exists(_.name.equalsIgnoreCase(this.channelName)), "A channel with that name already exists.") - .filterOrElse(_.exists(_.color == this.color), "A channel with that color already exists.") + .filterOrElse(_.size <= config.projects.get[Int]("max-channels"), "A project may only have up to five channels.") + .filterOrElse(_.forall(ch => !ch.name.equalsIgnoreCase(this.channelName)), "A channel with that name already exists.") + .filterOrElse(_.forall(_.color != this.color), "A channel with that color already exists.") .semiFlatMap(_ => this.factory.createChannel(project, this.channelName, this.color, this.nonReviewed)) } @@ -51,33 +53,27 @@ trait TChannelData { * @param project Project of channel * @return Error, if any */ - def saveTo(oldName: String)(implicit project: Project, ec: ExecutionContext): OptionT[Future, String] = { - OptionT( - project.channels.all.map { channels => - val channel = channels.find(_.name.equalsIgnoreCase(oldName)).get - val colorChan = channels.find(_.color.equals(this.color)) - val colorTaken = colorChan.exists(_ != channel) - if (colorTaken) { - Some("A channel with that color already exists.") - } else { - val nameChan = channels.find(_.name.equalsIgnoreCase(this.channelName)) - val nameTaken = nameChan.exists(_ != channel) - if (nameTaken) { - Some("A channel with that name already exists.") - } else { - val reviewedChannels = channels.filter(!_.isNonReviewed) - if (this.nonReviewed && reviewedChannels.size <= 1 && reviewedChannels.contains(channel)) { - Some("There must be at least one reviewed channel.") - } else { - channel.setName(this.channelName) - channel.setColor(this.color) - channel.setNonReviewed(this.nonReviewed) - None - } - } - } + //TODO: Return NEL[String] if we get the type + def saveTo(oldName: String)(implicit project: Project, ec: ExecutionContext): EitherT[Future, List[String], Unit] = { + EitherT.liftF(project.channels.all).flatMap { allChannels => + val (channelChangeSet, channels) = allChannels.partition(_.name.equalsIgnoreCase(oldName)) + val channel = channelChangeSet.toSeq.head + //TODO: Rewrite this nicer if we ever get a Validated/Validation type + val e1 = if(channels.exists(_.color == this.color)) List("error.channel.duplicateColor") else Nil + val e2 = if(channels.exists(_.name.equalsIgnoreCase(this.channelName))) List("error.channel.duplicateName") else Nil + val e3 = if(nonReviewed && channels.count(_.isReviewed) < 1) List("error.channel.minOneReviewed") else Nil + val errors = e1 ::: e2 ::: e3 + + if(errors.nonEmpty) { + EitherT.leftT[Future, Unit](errors) + } + else { + val effects = channel.setName(this.channelName) *> + channel.setColor(this.color) *> + channel.setNonReviewed(this.nonReviewed) + EitherT.right[List[String]](effects).map(_ => ()) } - ) + } } } diff --git a/app/models/project/Channel.scala b/app/models/project/Channel.scala index a89196389..e33888c72 100644 --- a/app/models/project/Channel.scala +++ b/app/models/project/Channel.scala @@ -2,6 +2,8 @@ package models.project import java.sql.Timestamp +import scala.concurrent.Future + import com.google.common.base.Preconditions._ import db.Named import db.impl.ChannelTable @@ -76,12 +78,15 @@ case class Channel(override val id: Option[Int] = None, update(ModelKeys.Color) } + def isReviewed: Boolean = !this._isNonReviewed + def isNonReviewed: Boolean = this._isNonReviewed def setNonReviewed(isNonReviewed: Boolean) = { this._isNonReviewed = isNonReviewed if (isDefined) update(IsNonReviewed) + else Future.unit } /** diff --git a/app/views/utils/alert.scala.html b/app/views/utils/alert.scala.html index 440ede7ab..92bfa964d 100644 --- a/app/views/utils/alert.scala.html +++ b/app/views/utils/alert.scala.html @@ -13,6 +13,11 @@ - @(messages(message)) + + @if(flash.get(s"$alertType-israw").contains("true")) { + @Html(message) + } else { + @(messages(message)) + } } diff --git a/conf/messages b/conf/messages index eb3269da8..012eb508d 100755 --- a/conf/messages +++ b/conf/messages @@ -80,6 +80,9 @@ error.project.invalidPluginFile = Invalid plugin file. error.channel.last = You cannot delete your only channel. error.channel.lastNonEmpty = You cannot delete your only non-empty channel. error.channel.lastReviewed = You cannot delete your only reviewed channel. +error.channel.duplicateColor = A channel with that color already exists. +error.channel.duplicateName = A channel with that name already exists. +error.channel.minOneReviewed = There must be at least one reviewed channel. error.tagline.tooLong = Tagline is too long (max {0}). error.org.disabled = Apologies, creation of Organizations is temporarily disabled. error.org.createLimit = You may only create up to {0} organizations!