Implement package states for easier reviews

8 files changed, 62 insertions, 41 deletions

diff --git a/app/blueprints/admin/admin.py b/app/blueprints/admin/admin.py
index 50ea64f..6796565 100644
--- a/app/blueprints/admin/admin.py
+++ b/app/blueprints/admin/admin.py
@@ -57,7 +57,7 @@ def admin_page():
 
 		elif action == "reimportpackages":
 			tasks = []
-			for package in Package.query.filter_by(soft_deleted=False).all():
+			for package in Package.query.filter(Package.state!=PackageState.DELETED).all():
 				release = package.releases.first()
 				if release:
 					zippath = release.url.replace("/uploads/", app.config["UPLOAD_DIR"])
@@ -96,7 +96,7 @@ def admin_page():
 
 		elif action == "importscreenshots":
 			packages = Package.query \
-				.filter_by(soft_deleted=False) \
+				.filter(Package.state!=PackageState.DELETED) \
 				.outerjoin(PackageScreenshot, Package.id==PackageScreenshot.package_id) \
 				.filter(PackageScreenshot.id==None) \
 				.all()
@@ -110,7 +110,7 @@ def admin_page():
 			if package is None:
 				flash("Unknown package", "danger")
 			else:
-				package.soft_deleted = False
+				package.state = PackageState.READY_FOR_REVIEW
 				db.session.commit()
 				return redirect(url_for("admin.admin_page"))
 
@@ -163,7 +163,7 @@ def admin_page():
 		else:
 			flash("Unknown action: " + action, "danger")
 
-	deleted_packages = Package.query.filter_by(soft_deleted=True).all()
+	deleted_packages = Package.query.filter(Package.state==PackageState.DELETED).all()
 	return render_template("admin/list.html", deleted_packages=deleted_packages)
 
 class SwitchUserForm(FlaskForm):
diff --git a/app/blueprints/homepage/__init__.py b/app/blueprints/homepage/__init__.py
index 85f6686..dfb3bd0 100644
--- a/app/blueprints/homepage/__init__.py
+++ b/app/blueprints/homepage/__init__.py
@@ -15,7 +15,7 @@ def home():
 			joinedload(Package.license), \
 			joinedload(Package.media_license))
 
-	query   = Package.query.filter_by(approved=True, soft_deleted=False)
+	query   = Package.query.filter_by(state=PackageState.APPROVED)
 	count   = query.count()
 
 	new     = join(query.order_by(db.desc(Package.approved_at))).limit(8).all()
@@ -24,7 +24,7 @@ def home():
 	pop_txp = join(query.filter_by(type=PackageType.TXP).order_by(db.desc(Package.score))).limit(4).all()
 
 	updated = db.session.query(Package).select_from(PackageRelease).join(Package) \
-			.filter_by(soft_deleted=False, approved=True) \
+			.filter_by(state=PackageState.APPROVED) \
 			.order_by(db.desc(PackageRelease.releaseDate)) \
 			.limit(20).all()
 	updated = updated[:8]
diff --git a/app/blueprints/metapackages/__init__.py b/app/blueprints/metapackages/__init__.py
index 896a260..c7d60da 100644
--- a/app/blueprints/metapackages/__init__.py
+++ b/app/blueprints/metapackages/__init__.py
@@ -41,7 +41,7 @@ def view(name):
 		.filter(MetaPackage.name==name) \
 		.join(MetaPackage.dependencies) \
 		.join(Dependency.depender) \
-		.filter(Dependency.optional==False, Package.approved==True, Package.soft_deleted==False) \
+		.filter(Dependency.optional==False, Package.state==PackageState.APPROVED) \
 		.all()
 
 	optional_dependers = db.session.query(Package) \
@@ -49,11 +49,11 @@ def view(name):
 		.filter(MetaPackage.name==name) \
 		.join(MetaPackage.dependencies) \
 		.join(Dependency.depender) \
-		.filter(Dependency.optional==True, Package.approved==True, Package.soft_deleted==False) \
+		.filter(Dependency.optional==True, Package.state==PackageState.APPROVED) \
 		.all()
 
 	similar_topics = None
-	if mpackage.packages.filter_by(approved=True, soft_deleted=False).count() == 0:
+	if mpackage.packages.filter_by(state=PackageState.APPROVED).count() == 0:
 		similar_topics = ForumTopic.query \
 				.filter_by(name=name) \
 				.order_by(db.asc(ForumTopic.name), db.asc(ForumTopic.title)) \
diff --git a/app/blueprints/metrics/__init__.py b/app/blueprints/metrics/__init__.py
index 18d5bb4..4588ef2 100644
--- a/app/blueprints/metrics/__init__.py
+++ b/app/blueprints/metrics/__init__.py
@@ -45,7 +45,7 @@ def generate_metrics(full=False):
 	downloads_result = db.session.query(func.sum(Package.downloads)).one_or_none()
 	downloads = 0 if not downloads_result or not downloads_result[0] else downloads_result[0]
 
-	packages = Package.query.filter_by(approved=True, soft_deleted=False).count()
+	packages = Package.query.filter_by(state=PackageState.APPROVED).count()
 	users = User.query.filter(User.rank != UserRank.NOT_JOINED).count()
 
 	ret = ""
@@ -55,7 +55,7 @@ def generate_metrics(full=False):
 
 	if full:
 		scores = Package.query.join(User).with_entities(User.username, Package.name, Package.score) \
-			.filter(Package.approved==True, Package.soft_deleted==False).all()
+			.filter(Package.state==PackageState.APPROVED).all()
 
 		ret += write_array_stat("contentdb_package_score", "Package score", "gauge", \
 			[({ "author": score[0], "name": score[1] }, score[2])  for score in scores])
diff --git a/app/blueprints/packages/packages.py b/app/blueprints/packages/packages.py
index 6b24fa6..c7a5c99 100644
--- a/app/blueprints/packages/packages.py
+++ b/app/blueprints/packages/packages.py
@@ -122,8 +122,8 @@ def view(package):
 	alternatives = None
 	if package.type == PackageType.MOD:
 		alternatives = Package.query \
-			.filter_by(name=package.name, type=PackageType.MOD, soft_deleted=False) \
-			.filter(Package.id != package.id) \
+			.filter_by(name=package.name, type=PackageType.MOD) \
+			.filter(Package.id != package.id, Package.state!=PackageState.DELETED) \
 			.order_by(db.desc(Package.score)) \
 			.all()
 
@@ -148,9 +148,9 @@ def view(package):
 
 	topic_error = None
 	topic_error_lvl = "warning"
-	if not package.approved and package.forums is not None:
+	if package.state != PackageState.APPROVED and package.forums is not None:
 		errors = []
-		if Package.query.filter_by(forums=package.forums, soft_deleted=False).count() > 1:
+		if Package.query.filter(Package.forums==package.forums, Package.state!=PackageState.DELETED).count() > 1:
 			errors.append("<b>Error: Another package already uses this forum topic!</b>")
 			topic_error_lvl = "danger"
 
@@ -294,7 +294,7 @@ def create_edit(author=None, name=None):
 		if not package:
 			package = Package.query.filter_by(name=form["name"].data, author_id=author.id).first()
 			if package is not None:
-				if package.soft_deleted:
+				if package.state == PackageState.READY_FOR_REVIEW:
 					Package.query.filter_by(name=form["name"].data, author_id=author.id).delete()
 				else:
 					flash("Package already exists!", "danger")
@@ -305,8 +305,7 @@ def create_edit(author=None, name=None):
 			package.maintainers.append(author)
 			wasNew = True
 
-		elif package.approved and package.name != form.name.data and \
-				not package.checkPerm(current_user, Permission.CHANGE_NAME):
+		elif package.name != form.name.data and not package.checkPerm(current_user, Permission.CHANGE_NAME):
 			flash("Unable to change package name", "danger")
 			return redirect(url_for("packages.create_edit", author=author, name=name))
 
@@ -359,7 +358,7 @@ def create_edit(author=None, name=None):
 
 		return redirect(next_url)
 
-	package_query = Package.query.filter_by(approved=True, soft_deleted=False)
+	package_query = Package.query.filter_by(state=PackageState.APPROVED)
 	if package is not None:
 		package_query = package_query.filter(Package.id != package.id)
 
@@ -369,18 +368,23 @@ def create_edit(author=None, name=None):
 			packages=package_query.all(), \
 			mpackages=MetaPackage.query.order_by(db.asc(MetaPackage.name)).all())
 
-@bp.route("/packages/<author>/<name>/approve/", methods=["POST"])
+
+@bp.route("/packages/<author>/<name>/state/", methods=["POST"])
 @login_required
 @is_package_page
-def approve(package):
-	if not package.checkPerm(current_user, Permission.APPROVE_NEW):
-		flash("You don't have permission to do that.", "danger")
+def move_to_state(package):
+	state = PackageState.get(request.args.get("state"))
+	if state is None:
+		abort(400)
 
-	elif package.approved:
-		flash("Package has already been approved", "danger")
+	if not package.canMoveToState(current_user, state):
+		flash("You don't have permission to do that", "danger")
+		return redirect(package.getDetailsURL())
 
-	else:
-		package.approved = True
+	package.state = state
+	msg = "Marked {} as {}".format(package.title, state.value)
+
+	if state == PackageState.APPROVED:
 		if not package.approved_at:
 			package.approved_at = datetime.datetime.now()
 
@@ -389,10 +393,19 @@ def approve(package):
 			s.approved = True
 
 		msg = "Approved {}".format(package.title)
-		addNotification(package.maintainers, current_user, msg, package.getDetailsURL(), package)
-		severity = AuditSeverity.NORMAL if current_user == package.author else AuditSeverity.EDITOR
-		addAuditLog(severity, current_user, msg, package.getDetailsURL(), package)
-		db.session.commit()
+
+	addNotification(package.maintainers, current_user, msg, package.getDetailsURL(), package)
+	severity = AuditSeverity.NORMAL if current_user in package.maintainers else AuditSeverity.EDITOR
+	addAuditLog(severity, current_user, msg, package.getDetailsURL(), package)
+
+	db.session.commit()
+
+	if package.state == PackageState.CHANGES_NEEDED:
+		flash("Please comment what changes are needed in the review thread", "warning")
+		if package.review_thread:
+			return redirect(package.review_thread.getViewURL())
+		else:
+			return redirect(url_for('threads.new', pid=package.id, title='Package approval comments'))
 
 	return redirect(package.getDetailsURL())
 
@@ -409,7 +422,7 @@ def remove(package):
 			flash("You don't have permission to do that.", "danger")
 			return redirect(package.getDetailsURL())
 
-		package.soft_deleted = True
+		package.state = PackageState.DELETED
 
 		url = url_for("users.profile", username=package.author.username)
 		msg = "Deleted {}".format(package.title)
@@ -425,7 +438,7 @@ def remove(package):
 			flash("You don't have permission to do that.", "danger")
 			return redirect(package.getDetailsURL())
 
-		package.approved = False
+		package.state = PackageState.READY_FOR_REVIEW
 
 		msg = "Unapproved {}".format(package.title)
 		addNotification(package.maintainers, current_user, msg, package.getDetailsURL(), package)
diff --git a/app/blueprints/threads/__init__.py b/app/blueprints/threads/__init__.py
index 6a2fdb8..5380389 100644
--- a/app/blueprints/threads/__init__.py
+++ b/app/blueprints/threads/__init__.py
@@ -298,6 +298,10 @@ def new():
 		if is_review_thread:
 			package.review_thread = thread
 
+			if package.state == PackageState.READY_FOR_REVIEW and current_user not in package.maintainers:
+				package.state = PackageState.CHANGES_NEEDED
+
+
 		notif_msg = "New thread '{}'".format(thread.title)
 		if package is not None:
 			addNotification(package.maintainers, current_user, notif_msg, thread.getViewURL(), package)
diff --git a/app/blueprints/todo/__init__.py b/app/blueprints/todo/__init__.py
index 682052d..d0903b9 100644
--- a/app/blueprints/todo/__init__.py
+++ b/app/blueprints/todo/__init__.py
@@ -31,8 +31,12 @@ def view():
 	canApproveScn = Permission.APPROVE_SCREENSHOT.check(current_user)
 
 	packages = None
+	wip_packages = None
 	if canApproveNew:
-		packages = Package.query.filter_by(approved=False, soft_deleted=False).order_by(db.desc(Package.created_at)).all()
+		packages = Package.query.filter_by(state=PackageState.READY_FOR_REVIEW) \
+			.order_by(db.desc(Package.created_at)).all()
+		wip_packages = Package.query.filter(Package.state<PackageState.READY_FOR_REVIEW) \
+			.order_by(db.desc(Package.created_at)).all()
 
 	releases = None
 	if canApproveRel:
@@ -64,16 +68,16 @@ def view():
 			.filter(~ db.exists().where(Package.forums==ForumTopic.topic_id)) \
 			.count()
 
-	total_packages = Package.query.filter_by(approved=True, soft_deleted=False).count()
-	total_to_tag = Package.query.filter_by(approved=True, soft_deleted=False, tags=None).count()
+	total_packages = Package.query.filter_by(state=PackageState.APPROVED).count()
+	total_to_tag = Package.query.filter_by(state=PackageState.APPROVED, tags=None).count()
 
 	unfulfilled_meta_packages = MetaPackage.query \
-			.filter(~ MetaPackage.packages.any(approved=True, soft_deleted=False)) \
+			.filter(~ MetaPackage.packages.any(state=PackageState.APPROVED)) \
 			.filter(MetaPackage.dependencies.any(optional=False)) \
 			.order_by(db.asc(MetaPackage.name)).count()
 
 	return render_template("todo/list.html", title="Reports and Work Queue",
-		packages=packages, releases=releases, screenshots=screenshots,
+		packages=packages, wip_packages=wip_packages, releases=releases, screenshots=screenshots,
 		canApproveNew=canApproveNew, canApproveRel=canApproveRel, canApproveScn=canApproveScn,
 		topics_to_add=topics_to_add, total_topics=total_topics, \
 		total_packages=total_packages, total_to_tag=total_to_tag, \
@@ -128,7 +132,7 @@ def tags():
 @login_required
 def metapackages():
 	mpackages = MetaPackage.query \
-			.filter(~ MetaPackage.packages.any(approved=True, soft_deleted=False)) \
+			.filter(~ MetaPackage.packages.any(state=PackageState.APPROVED)) \
 			.filter(MetaPackage.dependencies.any(optional=False)) \
 			.order_by(db.asc(MetaPackage.name)).all()
 
diff --git a/app/blueprints/users/profile.py b/app/blueprints/users/profile.py
index 5b78fb2..9e28d52 100644
--- a/app/blueprints/users/profile.py
+++ b/app/blueprints/users/profile.py
@@ -115,9 +115,9 @@ def profile(username):
 			# Redirect to home page
 			return redirect(url_for("users.profile", username=username))
 
-	packages = user.packages.filter_by(soft_deleted=False)
+	packages = user.packages.filter(Package.state!=PackageState.DELETED)
 	if not current_user.is_authenticated or (user != current_user and not current_user.canAccessTodoList()):
-		packages = packages.filter_by(approved=True)
+		packages = packages.filter_by(state=PackageState.APPROVED)
 	packages = packages.order_by(db.asc(Package.title))
 
 	topics_to_add = None