]> git.lizzy.rs Git - cheatdb.git/commitdiff
Implement package states for easier reviews
authorrubenwardy <rw@rubenwardy.com>
Wed, 16 Sep 2020 16:51:03 +0000 (17:51 +0100)
committerrubenwardy <rw@rubenwardy.com>
Wed, 16 Sep 2020 16:51:03 +0000 (17:51 +0100)
20 files changed:
README.md
app/blueprints/admin/admin.py
app/blueprints/homepage/__init__.py
app/blueprints/metapackages/__init__.py
app/blueprints/metrics/__init__.py
app/blueprints/packages/packages.py
app/blueprints/threads/__init__.py
app/blueprints/todo/__init__.py
app/blueprints/users/profile.py
app/default_data.py
app/models.py
app/querybuilder.py
app/tasks/importtasks.py
app/templates/macros/package_approval.html [new file with mode: 0644]
app/templates/metapackages/view.html
app/templates/packages/view.html
app/templates/todo/list.html
app/tests/test_api.py
app/utils.py
migrations/versions/b3c7ff6655af_.py [new file with mode: 0644]

index b236c9975e5e2963b7c1f61f39eb27dcd664a185..58d31edb0c522a0a0305349dcabecd199a5b656f 100644 (file)
--- a/README.md
+++ b/README.md
@@ -10,6 +10,8 @@ Docker is the recommended way to develop and deploy ContentDB.
 
 1. Install `docker` and `docker-compose`.
 
+       Debian/Ubuntu:
+
                sudo apt install docker-ce docker-compose
 
 2. Copy `config.example.cfg` to `config.cfg`.
@@ -40,7 +42,7 @@ Docker is the recommended way to develop and deploy ContentDB.
 
 8. Create initial data
        1. `./utils/bash.sh`
-       2. Either `python setup.py -t` or `python setup.py -o`:
+       2. Either `python utils/setup.py -t` or `python utils/setup.py -o`:
                1. `-o` creates just the admin, and static data like tags, and licenses.
                2. `-t` will do `-o` and also create test packages. (Recommended)
 
index 50ea64f8e432d8c0425b2708179a8876b4fadbe4..6796565ba487600ac322a9e44fc3c1b9e6f6d3a5 100644 (file)
@@ -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):
index 85f6686bd43e9ed97e273e62235914348f6c0cb4..dfb3bd0388670c3c3b1d7dde343f2c057df4727c 100644 (file)
@@ -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]
index 896a260cad600f13d49eeab8bc4404d72832a1b0..c7d60da9fcb687f7d42f49bac161ad471cb0c926 100644 (file)
@@ -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)) \
index 18d5bb445b8c8169479100eec7b043eae3882759..4588ef21008541ac4273ae44c82f9cf75906a0de 100644 (file)
@@ -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])
index 6b24fa66ba376bdc6b82918bc83396a1bf397f26..c7a5c99beb091d149cb20fef9f41299472e2cb3b 100644 (file)
@@ -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)
index 6a2fdb8516efd0fcef3fd18ff66946029d6b6774..5380389da67a43e6e326d40c3cd7ccac58b21b5c 100644 (file)
@@ -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)
index 682052dd88b880f0f32b82395d995063ce8873e8..d0903b913b7af167ca6d96d73d31428bc9e83fee 100644 (file)
@@ -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()
 
index 5b78fb2f7bea3fb7a0e685ee996843c4523bdb3a..9e28d5276a8d3079e06e0699bef3212973d3cf17 100644 (file)
@@ -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
index eb8aec6f75c1f3dd4995c133a60ce590d96d62af..3a9f5f145127e34d8d902e5877ca76ef715dd6f5 100644 (file)
@@ -63,7 +63,7 @@ def populate_test_data(session):
 
 
        mod = Package()
-       mod.approved = True
+       mod.state = PackageState.APPROVED
        mod.name = "alpha"
        mod.title = "Alpha Test"
        mod.license = licenses["MIT"]
@@ -87,7 +87,7 @@ def populate_test_data(session):
        session.add(rel)
 
        mod1 = Package()
-       mod1.approved = True
+       mod1.state = PackageState.APPROVED
        mod1.name = "awards"
        mod1.title = "Awards"
        mod1.license = licenses["LGPLv2.1"]
@@ -124,7 +124,7 @@ awards.register_achievement("award_mesefind",{
        session.add(rel)
 
        mod2 = Package()
-       mod2.approved = True
+       mod2.state = PackageState.APPROVED
        mod2.name = "mesecons"
        mod2.title = "Mesecons"
        mod2.tags.append(tags["tools"])
@@ -213,7 +213,7 @@ No warranty is provided, express or implied, for any part of the project.
        session.add(mod2)
 
        mod = Package()
-       mod.approved = True
+       mod.state = PackageState.APPROVED
        mod.name = "handholds"
        mod.title = "Handholds"
        mod.license = licenses["MIT"]
@@ -237,7 +237,7 @@ No warranty is provided, express or implied, for any part of the project.
        session.add(rel)
 
        mod = Package()
-       mod.approved = True
+       mod.state = PackageState.APPROVED
        mod.name = "other_worlds"
        mod.title = "Other Worlds"
        mod.license = licenses["MIT"]
@@ -254,7 +254,7 @@ No warranty is provided, express or implied, for any part of the project.
        session.add(mod)
 
        mod = Package()
-       mod.approved = True
+       mod.state = PackageState.APPROVED
        mod.name = "food"
        mod.title = "Food"
        mod.license = licenses["LGPLv2.1"]
@@ -270,7 +270,7 @@ No warranty is provided, express or implied, for any part of the project.
        session.add(mod)
 
        mod = Package()
-       mod.approved = True
+       mod.state = PackageState.APPROVED
        mod.name = "food_sweet"
        mod.title = "Sweet Foods"
        mod.license = licenses["CC0"]
@@ -287,7 +287,7 @@ No warranty is provided, express or implied, for any part of the project.
        session.add(mod)
 
        game1 = Package()
-       game1.approved = True
+       game1.state = PackageState.APPROVED
        game1.name = "capturetheflag"
        game1.title = "Capture The Flag"
        game1.type = PackageType.GAME
@@ -350,7 +350,7 @@ Uses the CTF PvP Engine.
 
 
        mod = Package()
-       mod.approved = True
+       mod.state = PackageState.APPROVED
        mod.name = "pixelbox"
        mod.title = "PixelBOX Reloaded"
        mod.license = licenses["CC0"]
index 4c0ccc348fe08c442ff00b75530570f390cd89f7..c484f2e3949ea4f1829293f96dd88af90c4b1d34 100644 (file)
@@ -336,6 +336,54 @@ class PackageType(enum.Enum):
                return item if type(item) == PackageType else PackageType[item]
 
 
+class PackageState(enum.Enum):
+       WIP = "Work in Progress"
+       CHANGES_NEEDED  = "Changes Needed"
+       READY_FOR_REVIEW = "Ready for Review"
+       APPROVED  = "Approved"
+       DELETED = "Deleted"
+
+       def toName(self):
+               return self.name.lower()
+
+       def verb(self):
+               if self == self.READY_FOR_REVIEW:
+                       return "Submit for Review"
+               elif self == self.APPROVED:
+                       return "Approve"
+               elif self == self.DELETED:
+                       return "Delete"
+               else:
+                       return self.value
+
+       def __str__(self):
+               return self.name
+
+       @classmethod
+       def get(cls, name):
+               try:
+                       return PackageState[name.upper()]
+               except KeyError:
+                       return None
+
+       @classmethod
+       def choices(cls):
+               return [(choice, choice.value) for choice in cls]
+
+       @classmethod
+       def coerce(cls, item):
+               return item if type(item) == PackageState else PackageState[item]
+
+
+PACKAGE_STATE_FLOW = {
+       PackageState.WIP: set([ PackageState.READY_FOR_REVIEW ]),
+       PackageState.CHANGES_NEEDED: set([ PackageState.READY_FOR_REVIEW ]),
+       PackageState.READY_FOR_REVIEW: set([ PackageState.WIP, PackageState.CHANGES_NEEDED, PackageState.APPROVED ]),
+       PackageState.APPROVED: set([ PackageState.CHANGES_NEEDED ]),
+       PackageState.DELETED: set([ PackageState.READY_FOR_REVIEW ]),
+}
+
+
 class PackagePropertyKey(enum.Enum):
        name          = "Name"
        title         = "Title"
@@ -480,8 +528,11 @@ class Package(db.Model):
        media_license_id = db.Column(db.Integer, db.ForeignKey("license.id"), nullable=False, default=1)
        media_license    = db.relationship("License", foreign_keys=[media_license_id])
 
-       approved     = db.Column(db.Boolean, nullable=False, default=False)
-       soft_deleted = db.Column(db.Boolean, nullable=False, default=False)
+       state         = db.Column(db.Enum(PackageState), default=PackageState.WIP)
+
+       @property
+       def approved(self):
+               return self.state == PackageState.APPROVED
 
        score        = db.Column(db.Float, nullable=False, default=0)
        score_downloads = db.Column(db.Float, nullable=False, default=0)
@@ -525,7 +576,7 @@ class Package(db.Model):
 
                self.author_id = package.author_id
                self.created_at = package.created_at
-               self.approved = package.approved
+               self.state = package.state
 
                self.maintainers.append(self.author)
 
@@ -578,22 +629,6 @@ class Package(db.Model):
        def getSortedOptionalDependencies(self):
                return self.getSortedDependencies(False)
 
-       def getState(self):
-               if self.approved:
-                       return "approved"
-               elif self.review_thread_id:
-                       return "thread"
-               elif (self.type == PackageType.GAME or \
-                                       self.type == PackageType.TXP) and \
-                               self.screenshots.count() == 0:
-                       return "wip"
-               elif not self.getDownloadRelease():
-                       return "wip"
-               elif "Other" in self.license.name or "Other" in self.media_license.name:
-                       return "license"
-               else:
-                       return "ready"
-
        def getAsDictionaryKey(self):
                return {
                        "name": self.name,
@@ -682,9 +717,14 @@ class Package(db.Model):
                return url_for("packages.create_edit",
                                author=self.author.username, name=self.name)
 
-       def getApproveURL(self):
-               return url_for("packages.approve",
-                               author=self.author.username, name=self.name)
+       def getSetStateURL(self, state):
+               if type(state) == str:
+                       state = PackageState[perm]
+               elif type(state) != PackageState:
+                       raise Exception("Unknown state given to Package.canMoveToState()")
+
+               return url_for("packages.move_to_state",
+                               author=self.author.username, name=self.name, state=state.name.lower())
 
        def getRemoveURL(self):
                return url_for("packages.remove",
@@ -784,6 +824,53 @@ class Package(db.Model):
                else:
                        raise Exception("Permission {} is not related to packages".format(perm.name))
 
+
+       def canMoveToState(self, user, state):
+               if not user.is_authenticated:
+                       return False
+
+               if type(state) == str:
+                       state = PackageState[perm]
+               elif type(state) != PackageState:
+                       raise Exception("Unknown state given to Package.canMoveToState()")
+
+               if state not in PACKAGE_STATE_FLOW[self.state]:
+                       return False
+
+               if state == PackageState.READY_FOR_REVIEW or state == PackageState.APPROVED:
+                       requiredPerm = Permission.APPROVE_NEW if state == PackageState.APPROVED else Permission.EDIT_PACKAGE
+
+                       if not self.checkPerm(user, requiredPerm):
+                               return False
+
+                       if state == PackageState.APPROVED and \
+                                       ("Other" in self.license.name or "Other" in self.media_license.name):
+                               return False
+
+                       needsScreenshot = \
+                               (self.type == self.type.GAME or self.type == self.type.TXP) and \
+                                       self.screenshots.count() == 0
+                       return self.releases.count() > 0 and not needsScreenshot
+
+               elif state == PackageState.CHANGES_NEEDED:
+                       return self.checkPerm(user, Permission.APPROVE_NEW)
+
+               elif state == PackageState.WIP:
+                       return self.checkPerm(user, Permission.EDIT_PACKAGE) and user in self.maintainers
+
+               return True
+
+
+       def getNextStates(self, user):
+               states = []
+
+               for state in PackageState:
+                       if self.canMoveToState(user, state):
+                               states.append(state)
+
+               return states
+
+
        def getScoreDict(self):
                return {
                        "author": self.author.username,
index 1d6b6f063c2368117d96ba1f789da503301cf838..57f5dbeb5e297fcdd89cb794673ad2eb87ad97c0 100644 (file)
@@ -72,9 +72,9 @@ class QueryBuilder:
                query = None
                if self.order_by == "last_release":
                        query = db.session.query(Package).select_from(PackageRelease).join(Package) \
-                               .filter_by(soft_deleted=False, approved=True)
+                               .filter_by(state=PackageState.APPROVED)
                else:
-                       query = Package.query.filter_by(soft_deleted=False, approved=True)
+                       query = Package.query.filter_by(state=PackageState.APPROVED)
 
                return self.filterPackageQuery(self.orderPackageQuery(query))
 
index b0542ee5a6fa7fe8be9f53f255bee3830a87513e..d1cd87d4bbf7d1adcdd8c2b7973b6c9324116ab6 100644 (file)
@@ -321,7 +321,7 @@ def makeVCSRelease(self, id, branch):
 @celery.task()
 def importRepoScreenshot(id):
        package = Package.query.get(id)
-       if package is None or package.soft_deleted:
+       if package is None or package.state == PackageState.DELETED:
                raise Exception("Unexpected none package")
 
        # Get URL Maker
diff --git a/app/templates/macros/package_approval.html b/app/templates/macros/package_approval.html
new file mode 100644 (file)
index 0000000..acabe98
--- /dev/null
@@ -0,0 +1,101 @@
+{% macro render_banners(package, current_user, topic_error, topic_error_lvl, similar_topics) -%}
+
+<div class="row mb-4">
+       <span class="col">
+               State: <strong>{{ package.state.value }}</strong>
+       </span>
+
+       {% for state in package.getNextStates(current_user) %}
+               <form class="col-auto"  method="post" action="{{ package.getSetStateURL(state) }}">
+                       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+                       <input class="btn btn-sm btn-secondary" type="submit" value="{{ state.verb() }}" />
+               </form>
+       {% endfor %}
+</div>
+
+{% set level = "warning" %}
+{% if package.releases.count() == 0 %}
+       {% set message %}
+               <h4 class="alert-heading">Release Required</h4>
+               {% if package.checkPerm(current_user, "MAKE_RELEASE") %}
+                       <p>You need to create a release before this package can be approved.</p>
+                       <p>
+                               A release is a single downloadable version of your {{ package.type.value | lower }}.
+                               You need to create releases even if you use a rolling release development cycle,
+                               as Minetest needs them to check for updates.
+                       </p>
+                       <a class="btn" href="{{ package.getCreateReleaseURL() }}">Create Release</a>
+               {% else %}
+                       A release is required before this package can be approved.
+               {% endif %}
+       {% endset %}
+{% elif (package.type == package.type.GAME or package.type == package.type.TXP) and package.screenshots.count() == 0 %}
+       {% set message = "You need to add at least one screenshot." %}
+
+{% elif topic_error_lvl == "danger" %}
+{% elif package.state == package.state.READY_FOR_REVIEW and ("Other" in package.license.name or "Other" in package.media_license.name) %}
+       {% set message = "Please wait for the license to be added to CDB." %}
+
+{% else %}
+       {% set level = "info" %}
+       {% set message %}
+               {% if package.screenshots.count() == 0 %}
+                       <b>You should add at least one screenshot, but this isn't required.</b><br />
+               {% endif %}
+
+               {% if package.state == package.state.READY_FOR_REVIEW %}
+                       {% if not package.getDownloadRelease() %}
+                               Please wait for the release to be approved.
+                       {% elif package.checkPerm(current_user, "APPROVE_NEW") %}
+                               You can now approve this package if you're ready.
+                       {% else %}
+                               Please wait for the package to be approved.
+                       {% endif %}
+               {% else %}
+                       {% if package.checkPerm(current_user, "EDIT_PACKAGE") %}
+                               You can now submit this package for approval if you're ready.
+                       {% else %}
+                               This package can be submitted for approval when ready.
+                       {% endif %}
+               {% endif %}
+       {% endset %}
+{% endif %}
+
+{% if message %}
+       <div class="alert alert-{{ level }}">
+               <span class="icon_message"></span>
+
+               {{ message | safe }}
+
+               <div style="clear: both;"></div>
+       </div>
+{% endif %}
+
+{% if topic_error %}
+       <div class="alert alert-{{ topic_error_lvl }}">
+               <span class="icon_message"></span>
+               {{ topic_error | safe }}
+               <div style="clear: both;"></div>
+       </div>
+{% endif %}
+
+{% if similar_topics %}
+       <div class="alert alert-warning">
+               Please make sure that this package has the right to
+               the name '{{ package.name }}'.
+               See the
+               <a href="/policy_and_guidance/">Inclusion Policy</a>
+               for more info.
+       </div>
+{% endif %}
+
+{% if not package.review_thread and (package.author == current_user or package.checkPerm(current_user, "APPROVE_NEW")) %}
+       <div class="alert alert-secondary">
+               <a class="float-right btn btn-sm btn-secondary" href="{{ url_for('threads.new', pid=package.id, title='Package approval comments') }}">Open Thread</a>
+
+               Privately ask a question or give feedback
+               <div style="clear:both;"></div>
+       </div>
+{% endif %}
+
+{% endmacro %}
index 08a46740a936a4b3d2f841f698fe362597a63359..8166350b573728c07d04561832a49903cd1a848f 100644 (file)
@@ -10,7 +10,7 @@
        <h2>Provided By</h2>
 
        {% from "macros/packagegridtile.html" import render_pkggrid %}
-       {{ render_pkggrid(mpackage.packages.filter_by(approved=True, soft_deleted=False).all()) }}
+       {{ render_pkggrid(mpackage.packages.filter_by(state="APPROVED").all()) }}
 
        {% if similar_topics %}
                <p>Unforuntately, this isn't on ContentDB yet! Here's some forum topics:</p>
index 406a0c692e1d1aa36769d0f4bda7d87153bf1019..45079e0ec1d84067897182453d894c8ca07e25a4 100644 (file)
                </div>
        </header>
 
-       <main class="container mt-4">
-               {% if not package.approved %}
-                       <div class="alert alert-warning">
-                               <span class="icon_message"></span>
-                               {% if package.releases.count() == 0 %}
-                                       <h4 class="alert-heading">Release Required</h4>
-                                       {% if package.checkPerm(current_user, "MAKE_RELEASE") %}
-                                               <p>You need to create a release before this package can be approved.</p>
-                                               <p>
-                                                       A release is a single downloadable version of your {{ package.type.value | lower }}.
-                                                       You need to create releases even if you use a rolling release development cycle,
-                                                       as Minetest needs them to check for updates.
-                                               </p>
-                                               <a class="btn" href="{{ package.getCreateReleaseURL() }}">Create Release</a>
-                                       {% else %}
-                                               A release is required before this package can be approved.
-                                       {% endif %}
-
-                               {% elif (package.type == package.type.GAME or package.type == package.type.TXP) and package.screenshots.count() == 0 %}
-                                       You need to add at least one screenshot.
-
-                               {% elif topic_error_lvl == "danger" %}
-                                       Please fix the below topic issue(s).
+       {% if not package.approved %}
+               <aside class="container mt-4">
+                       {% from "macros/package_approval.html" import render_banners %}
+                       {{ render_banners(package, current_user, topic_error, topic_error_lvl, similar_topics) }}
 
-                               {% elif "Other" in package.license.name or "Other" in package.media_license.name %}
-                                       Please wait for the license to be added to CDB.
-
-                               {% else %}
-                                       {% if package.screenshots.count() == 0 %}
-                                               <b>You should add at least one screenshot, but this isn't required.</b><br />
-                                       {% endif %}
-
-                                       {% if not package.getDownloadRelease() %}
-                                               Please wait for the release to be approved.
-                                       {% elif package.checkPerm(current_user, "APPROVE_NEW") %}
-                                               <form class="float-right"  method="post" action="{{ package.getApproveURL() }}">
-                                                       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-                                                       <input class="btn btn-sm btn-warning" type="submit" value="Approve" />
-                                               </form>
-                                               You can now approve this package if you're ready.
-                                       {% else %}
-                                               Please wait for the package to be approved.
-                                       {% endif %}
+                       {% if review_thread and review_thread.checkPerm(current_user, "SEE_THREAD") %}
+                               <h2>{% if review_thread.private %}&#x1f512;{% endif %} {{ review_thread.title }}</h2>
+                               {% if review_thread.private %}
+                                       <p><i>
+                                               This thread is only visible to the package owner and users of
+                                               Editor rank or above.
+                                       </i></p>
                                {% endif %}
-                               <div style="clear: both;"></div>
-                       </div>
 
-                       {% if topic_error %}
-                               <div class="alert alert-{{ topic_error_lvl }}">
-                                       <span class="icon_message"></span>
-                                       {{ topic_error | safe }}
-                                       <div style="clear: both;"></div>
-                               </div>
-                       {% endif %}
-
-                       {% if similar_topics %}
-                               <div class="alert alert-warning">
-                                       Please make sure that this package has the right to
-                                       the name '{{ package.name }}'.
-                                       See the
-                                       <a href="/policy_and_guidance/">Inclusion Policy</a>
-                                       for more info.
-                               </div>
-                       {% endif %}
-
-                       {% if not review_thread and (package.author == current_user or package.checkPerm(current_user, "APPROVE_NEW")) %}
-                               <div class="alert alert-info">
-                                       <a class="float-right btn btn-sm btn-info" href="{{ url_for('threads.new', pid=package.id, title='Package approval comments') }}">Open Thread</a>
-
-                                       Privately ask a question or give feedback
-                                       <div style="clear:both;"></div>
-                               </div>
+                               {% from "macros/threads.html" import render_thread %}
+                               {{ render_thread(review_thread, current_user) }}
                        {% endif %}
-               {% endif %}
+               </aside>
+       {% endif %}
 
+       <main class="container mt-4">
                <aside class="float-right ml-4" style="width: 18rem;">
                        <div class="card mb-4">
                                <div class="card-header">
                        {% endif %}
                </aside>
 
-               {% if not package.approved and (package.author == current_user or package.checkPerm(current_user, "APPROVE_NEW")) %}
-                       {% if review_thread %}
-                               <h2>{% if review_thread.private %}&#x1f512;{% endif %} {{ review_thread.title }}</h2>
-                               {% if review_thread.private %}
-                                       <p><i>
-                                               This thread is only visible to the package owner and users of
-                                               Editor rank or above.
-                                       </i></p>
-                               {% endif %}
-
-                               {% from "macros/threads.html" import render_thread %}
-                               {{ render_thread(review_thread, current_user) }}
-                       {% endif %}
-               {% endif %}
-
                <ul class="screenshot_list mb-4">
                        {% for ss in package.screenshots %}
                                {% if ss.approved or package.checkPerm(current_user, "ADD_SCREENSHOTS") %}
index 24d5289b0a2520bee80905976a3902b5aee0526e..c3b81187463c6b89b17e4414010e7d72612dbe3b 100644 (file)
@@ -8,21 +8,17 @@
        <h2 class="mb-4">Approval Queue</h2>
 
        <div class="row">
-               {% if canApproveNew and packages %}
+               {% if canApproveNew and (packages or wip_packages) %}
                <div class="col-sm-6">
                        <div class="card">
                                <h3 class="card-header">Packages</h3>
                                <div class="list-group list-group-flush">
                                        {% for p in packages %}
                                                <a href="{{ p.getDetailsURL() }}" class="list-group-item list-group-item-action">
-                                                       {% if p.getState() == "thread" %}
-                                                               <span class="mr-2 badge badge-danger">Thread</span>
-                                                       {% elif p.getState() == "ready" %}
+                                                       {% if "Other" in p.license.name or "Other" in p.media_license.name %}
+                                                               <span class="mr-2 badge badge-info">License</span>
+                                                       {% else %}
                                                                <span class="mr-2 badge badge-success">Ready</span>
-                                                       {% elif p.getState() == "wip" %}
-                                                               <span class="mr-2 badge badge-warning">WIP</span>
-                                                       {% elif p.getState() == "license" %}
-                                                               <span class="mr-2 badge badge-info">WIP</span>
                                                        {% endif %}
 
                                                        {{ p.title }} by {{ p.author.display_name }}
                                        {% endfor %}
                                </div>
                        </div>
+
+                       <div class="card mt-5">
+                               <h3 class="card-header">WIP Packages</h3>
+                               <div class="list-group list-group-flush">
+                                       {% for p in wip_packages %}
+                                               <a href="{{ p.getDetailsURL() }}" class="list-group-item list-group-item-action">
+                                                       <span class="mr-2 badge badge-warning">{{ p.state.value }}</span>
+
+                                                       {{ p.title }} by {{ p.author.display_name }}
+                                               </a>
+                                       {% else %}
+                                               <li class="list-group-item"><i>No packages need reviewing.</i></li>
+                                       {% endfor %}
+                               </div>
+                       </div>
                </div>
                {% endif %}
 
index 8009656051c082c92a347f5473a4fed845b04734..154e17815d92f0f22bf83f87377f7ada3e7e1ef7 100644 (file)
@@ -46,7 +46,7 @@ def test_packages_with_contents(client):
        packages = parse_json(rv.data)
 
        assert len(packages) > 0
-       assert len(packages) == Package.query.filter_by(approved=True).count()
+       assert len(packages) == Package.query.filter_by(state=PackageState.APPROVED).count()
 
        validate_package_list(packages)
 
index d0dfd6aa7d09fdc6fc28877e23563881df27e6d3..fb698856698dcd19123d657503eead379eaaa660 100644 (file)
@@ -200,7 +200,8 @@ def getPackageByInfo(author, name):
        if user is None:
                return None
 
-       package = Package.query.filter_by(name=name, author_id=user.id, soft_deleted=False).first()
+       package = Package.query.filter_by(name=name, author_id=user.id) \
+               .filter(Package.state!=PackageState.DELETED).first()
        if package is None:
                return None
 
diff --git a/migrations/versions/b3c7ff6655af_.py b/migrations/versions/b3c7ff6655af_.py
new file mode 100644 (file)
index 0000000..3e49102
--- /dev/null
@@ -0,0 +1,37 @@
+"""empty message
+
+Revision ID: b3c7ff6655af
+Revises: dff4b87e4a76
+Create Date: 2020-09-16 14:35:43.805422
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = 'b3c7ff6655af'
+down_revision = 'dff4b87e4a76'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+       status = postgresql.ENUM('WIP', 'READY_FOR_REVIEW', 'APPROVED', 'DELETED', name='packagestate')
+       status.create(op.get_bind())
+
+       op.add_column('package', sa.Column('state', sa.Enum('WIP', 'CHANGES_NEEDED', 'READY_FOR_REVIEW', 'APPROVED', 'DELETED', name='packagestate'), nullable=True))
+       op.execute("UPDATE package SET state='APPROVED' WHERE approved=true")
+       op.execute("UPDATE package SET state='DELETED' WHERE soft_deleted=true")
+       op.drop_column('package', 'approved')
+       op.drop_column('package', 'updated_at')
+       op.drop_column('package', 'soft_deleted')
+       # ### end Alembic commands ###
+
+
+def downgrade():
+       # ### commands auto generated by Alembic - please adjust! ###
+       op.add_column('package', sa.Column('soft_deleted', sa.BOOLEAN(), server_default=sa.text('false'), autoincrement=False, nullable=False))
+       op.add_column('package', sa.Column('approved', sa.BOOLEAN(), autoincrement=False, nullable=False))
+       op.drop_column('package', 'state')
+       # ### end Alembic commands ###