diff --git a/README.md b/README.md index b236c997..58d31edb 100644 --- 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) diff --git a/app/blueprints/admin/admin.py b/app/blueprints/admin/admin.py index 50ea64f8..6796565b 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 85f6686b..dfb3bd03 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 896a260c..c7d60da9 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 18d5bb44..4588ef21 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 6b24fa66..c7a5c99b 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("Error: Another package already uses this forum topic!") 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///approve/", methods=["POST"]) + +@bp.route("/packages///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 6a2fdb85..5380389d 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 682052dd..d0903b91 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 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, diff --git a/app/querybuilder.py b/app/querybuilder.py index 1d6b6f06..57f5dbeb 100644 --- a/app/querybuilder.py +++ b/app/querybuilder.py @@ -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)) diff --git a/app/tasks/importtasks.py b/app/tasks/importtasks.py index b0542ee5..d1cd87d4 100644 --- a/app/tasks/importtasks.py +++ b/app/tasks/importtasks.py @@ -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 index 00000000..acabe98e --- /dev/null +++ b/app/templates/macros/package_approval.html @@ -0,0 +1,101 @@ +{% macro render_banners(package, current_user, topic_error, topic_error_lvl, similar_topics) -%} + +
+ + State: {{ package.state.value }} + + + {% for state in package.getNextStates(current_user) %} +
+ + +
+ {% endfor %} +
+ +{% set level = "warning" %} +{% if package.releases.count() == 0 %} + {% set message %} +

Release Required

+ {% if package.checkPerm(current_user, "MAKE_RELEASE") %} +

You need to create a release before this package can be approved.

+

+ 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. +

+ Create Release + {% 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 %} + You should add at least one screenshot, but this isn't required.
+ {% 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 %} +
+ + + {{ message | safe }} + +
+
+{% endif %} + +{% if topic_error %} +
+ + {{ topic_error | safe }} +
+
+{% endif %} + +{% if similar_topics %} +
+ Please make sure that this package has the right to + the name '{{ package.name }}'. + See the + Inclusion Policy + for more info. +
+{% endif %} + +{% if not package.review_thread and (package.author == current_user or package.checkPerm(current_user, "APPROVE_NEW")) %} +
+ Open Thread + + Privately ask a question or give feedback +
+
+{% endif %} + +{% endmacro %} diff --git a/app/templates/metapackages/view.html b/app/templates/metapackages/view.html index 08a46740..8166350b 100644 --- a/app/templates/metapackages/view.html +++ b/app/templates/metapackages/view.html @@ -10,7 +10,7 @@

Provided By

{% 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 %}

Unforuntately, this isn't on ContentDB yet! Here's some forum topics:

diff --git a/app/templates/packages/view.html b/app/templates/packages/view.html index 406a0c69..45079e0e 100644 --- a/app/templates/packages/view.html +++ b/app/templates/packages/view.html @@ -134,81 +134,27 @@ -
- {% if not package.approved %} -
- - {% if package.releases.count() == 0 %} -

Release Required

- {% if package.checkPerm(current_user, "MAKE_RELEASE") %} -

You need to create a release before this package can be approved.

-

- 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. -

- Create Release - {% else %} - A release is required before this package can be approved. - {% endif %} + {% if not package.approved %} +
- {% if topic_error %} -
- - {{ topic_error | safe }} -
-
+ {% from "macros/threads.html" import render_thread %} + {{ render_thread(review_thread, current_user) }} {% endif %} + + {% endif %} - {% if similar_topics %} -
- Please make sure that this package has the right to - the name '{{ package.name }}'. - See the - Inclusion Policy - for more info. -
- {% endif %} - - {% if not review_thread and (package.author == current_user or package.checkPerm(current_user, "APPROVE_NEW")) %} -
- Open Thread - - Privately ask a question or give feedback -
-
- {% endif %} - {% endif %} - +
- {% if not package.approved and (package.author == current_user or package.checkPerm(current_user, "APPROVE_NEW")) %} - {% if review_thread %} -

{% if review_thread.private %}🔒{% endif %} {{ review_thread.title }}

- {% if review_thread.private %} -

- This thread is only visible to the package owner and users of - Editor rank or above. -

- {% endif %} - - {% from "macros/threads.html" import render_thread %} - {{ render_thread(review_thread, current_user) }} - {% endif %} - {% endif %} -
    {% for ss in package.screenshots %} {% if ss.approved or package.checkPerm(current_user, "ADD_SCREENSHOTS") %} diff --git a/app/templates/todo/list.html b/app/templates/todo/list.html index 24d5289b..c3b81187 100644 --- a/app/templates/todo/list.html +++ b/app/templates/todo/list.html @@ -8,21 +8,17 @@

    Approval Queue

    - {% if canApproveNew and packages %} + {% if canApproveNew and (packages or wip_packages) %}

    Packages

    {% for p in packages %} - {% if p.getState() == "thread" %} - Thread - {% elif p.getState() == "ready" %} + {% if "Other" in p.license.name or "Other" in p.media_license.name %} + License + {% else %} Ready - {% elif p.getState() == "wip" %} - WIP - {% elif p.getState() == "license" %} - WIP {% endif %} {{ p.title }} by {{ p.author.display_name }} @@ -32,6 +28,21 @@ {% endfor %}
    + +
    +

    WIP Packages

    +
    + {% for p in wip_packages %} + + {{ p.state.value }} + + {{ p.title }} by {{ p.author.display_name }} + + {% else %} +
  • No packages need reviewing.
    • + {% endfor %} +
    +
    {% endif %} diff --git a/app/tests/test_api.py b/app/tests/test_api.py index 80096560..154e1781 100644 --- a/app/tests/test_api.py +++ b/app/tests/test_api.py @@ -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) diff --git a/app/utils.py b/app/utils.py index d0dfd6aa..fb698856 100644 --- a/app/utils.py +++ b/app/utils.py @@ -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 index 00000000..3e491027 --- /dev/null +++ b/migrations/versions/b3c7ff6655af_.py @@ -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 ###