From ebb55fc95a796fea046aec507b04a052820e7ea7 Mon Sep 17 00:00:00 2001 From: Adnan Maolood Date: Mon, 4 Apr 2022 15:14:50 -0400 Subject: [PATCH] todosrht: Use GraphQL for tracker CRUD operations Use the GraphQL API for tracker CRUD operations so that GraphQL-native user webhooks will be delivered. --- todosrht/blueprints/api/tickets.py | 34 ++++--- todosrht/blueprints/api/trackers.py | 130 ++++++++++++++++++------ todosrht/blueprints/settings.py | 79 +++++++------- todosrht/blueprints/tracker.py | 77 ++++++++------ todosrht/templates/tracker-details.html | 21 ++-- todosrht/types/tracker.py | 50 --------- 6 files changed, 218 insertions(+), 173 deletions(-) diff --git a/todosrht/blueprints/api/tickets.py b/todosrht/blueprints/api/tickets.py index aaf203b..23577ea 100644 --- a/todosrht/blueprints/api/tickets.py +++ b/todosrht/blueprints/api/tickets.py @@ -1,11 +1,12 @@ from datetime import datetime, timezone -from flask import Blueprint, abort, request +from flask import Blueprint, current_app, abort, request from srht.api import paginated_response from srht.database import db +from srht.graphql import exec_gql from srht.oauth import oauth, current_token from srht.validation import Validation, valid_url from todosrht.access import get_tracker, get_ticket -from todosrht.tickets import add_comment, submit_ticket +from todosrht.tickets import add_comment from todosrht.tickets import get_participant_for_user, get_participant_for_external from todosrht.blueprints.api import get_user from todosrht.types import Ticket, TicketAccess, TicketStatus, TicketResolution @@ -79,17 +80,26 @@ def tracker_tickets_POST(username, tracker_name): if not valid.ok: return valid.response - if external_id: - participant = get_participant_for_external(external_id, external_url) - else: - participant = get_participant_for_user(current_token.user) + input = { + "subject": title, + "body": desc, + "created": created, + "externalId": external_id, + "externalUrl": external_url, + } - ticket = submit_ticket(tracker, participant, title, desc) - if created: - ticket._no_autoupdate = True - ticket.created = created - ticket.updated = created - db.session.commit() + resp = exec_gql(current_app.site, """ + mutation SubmitTicket($trackerId: Int!, $input: SubmitTicketInput!) { + submitTicket(trackerId: $trackerId, input: $input) { + id + } + } + """, user=user, valid=valid, trackerId=tracker.id, input=input) + + if not valid.ok: + return valid.response + + ticket, _ = get_ticket(tracker, resp["submitTicket"]["id"]) TrackerWebhook.deliver(TrackerWebhook.Events.ticket_create, ticket.to_dict(), diff --git a/todosrht/blueprints/api/trackers.py b/todosrht/blueprints/api/trackers.py index 705d0d0..3b5e964 100644 --- a/todosrht/blueprints/api/trackers.py +++ b/todosrht/blueprints/api/trackers.py @@ -1,13 +1,14 @@ -from flask import Blueprint, abort, request +from flask import Blueprint, current_app, abort, request from srht.api import paginated_response from srht.database import db +from srht.graphql import exec_gql from srht.oauth import oauth, current_token from srht.validation import Validation from todosrht.access import get_tracker from todosrht.blueprints.api import get_user from todosrht.tickets import get_participant_for_user from todosrht.types import Label, Tracker, TicketAccess, TicketSubscription -from todosrht.webhooks import UserWebhook, TrackerWebhook +from todosrht.webhooks import TrackerWebhook trackers = Blueprint("api_trackers", __name__) @@ -26,24 +27,53 @@ def user_trackers_GET(username): @oauth("trackers:write") def user_trackers_POST(): user = current_token.user - tracker, valid = Tracker.create_from_request(request, user) + valid = Validation(request) + name = valid.require("name", friendly_name="Name") + description = valid.optional("description") + visibility = valid.require("visibility") if not valid.ok: return valid.response - db.session.add(tracker) - db.session.commit() - UserWebhook.deliver(UserWebhook.Events.tracker_create, - tracker.to_dict(), - UserWebhook.Subscription.user_id == tracker.owner_id) - # Auto-subscribe the owner - sub = TicketSubscription() - participant = get_participant_for_user(user) - sub.tracker_id = tracker.id - sub.participant_id = participant.id - db.session.add(sub) - db.session.commit() + resp = exec_gql(current_app.site, """ + mutation CreateTracker($name: String!, $description: String, $visibility: Visibility!) { + createTracker(name: $name, description: $description, visibility: $visibility) { + id + created + updated + owner { + canonical_name: canonicalName + ... on User { + name: username + } + } + name + description + defaultACL { + browse + submit + comment + edit + triage + } + visibility + } + } + """, user=user, valid=valid, name=name, description=description, visibility=visibility) - return tracker.to_dict(), 201 + if not valid.ok: + return valid.response + + resp = resp["createTracker"] + + # Build default_access list + resp["default_access"] = [ + key for key in [ + "browse", "submit", "comment", "edit", "triage" + ] if resp["defaultACL"][key] + ] + del resp["defaultACL"] + + return resp, 201 @trackers.route("/api/user//trackers/") @trackers.route("/api/trackers/", defaults={"username": None}) @@ -93,15 +123,55 @@ def user_tracker_by_name_PUT(username, tracker_name): abort(404) if tracker.owner_id != current_token.user_id: abort(401) + valid = Validation(request) - tracker.update(valid) + rewrite = lambda value: None if value == "" else value + input = { + key: rewrite(valid.source[key]) for key in [ + "description" + ] if valid.source.get(key) is not None + } + + resp = exec_gql(current_app.site, """ + mutation UpdateTracker($id: Int!, $input: TrackerInput!) { + updateTracker(id: $id, input: $input) { + id + created + updated + owner { + canonical_name: canonicalName + ... on User { + name: username + } + } + name + description + defaultACL { + browse + submit + comment + edit + triage + } + visibility + } + } + """, user=user, valid=valid, id=tracker.id, input=input) + if not valid.ok: return valid.response - db.session.commit() - UserWebhook.deliver(UserWebhook.Events.tracker_update, - tracker.to_dict(), - UserWebhook.Subscription.user_id == tracker.owner_id) - return tracker.to_dict() + + resp = resp["updateTracker"] + + # Build default_access list + resp["default_access"] = [ + key for key in [ + "browse", "submit", "comment", "edit", "triage" + ] if resp["defaultACL"][key] + ] + del resp["defaultACL"] + + return resp @trackers.route("/api/user//trackers/", methods=["DELETE"]) @@ -115,17 +185,11 @@ def user_tracker_by_name_DELETE(username, tracker_name): abort(404) if tracker.owner_id != current_token.user_id: abort(401) - # SQLAlchemy shits itself on some of our weird constraints/relationships - # so fuck it, postgres knows what to do here - tracker_id = tracker.id - owner_id = tracker.owner_id - assert isinstance(tracker_id, int) - db.session.expunge_all() - db.engine.execute(f"DELETE FROM tracker WHERE id = {tracker_id};") - db.session.commit() - UserWebhook.deliver(UserWebhook.Events.tracker_delete, - { "id": tracker_id }, - UserWebhook.Subscription.user_id == owner_id) + exec_gql(current_app.site, """ + mutation DeleteTracker($id: Int!) { + deleteTracker(id: $id) { id } + } + """, user=user, id=tracker.id); return {}, 204 @trackers.route("/api/user//trackers//labels") diff --git a/todosrht/blueprints/settings.py b/todosrht/blueprints/settings.py index 3a5e463..02863dc 100644 --- a/todosrht/blueprints/settings.py +++ b/todosrht/blueprints/settings.py @@ -2,13 +2,14 @@ import gzip import json import os from collections import OrderedDict -from flask import Blueprint, render_template, request, url_for, abort, redirect +from flask import Blueprint, current_app, render_template, request, url_for, abort, redirect from flask import current_app, send_file from srht.config import get_origin from srht.crypto import sign_payload from srht.database import db from srht.oauth import current_user, loginrequired from srht.flask import date_handler, session +from srht.graphql import exec_gql from srht.validation import Validation from tempfile import NamedTemporaryFile from todosrht.access import get_tracker @@ -16,7 +17,6 @@ from todosrht.trackers import get_recent_users from todosrht.types import Event, EventType, Ticket, TicketAccess, Visibility from todosrht.types import ParticipantType, UserAccess, User from todosrht.urls import tracker_url -from todosrht.webhooks import UserWebhook from todosrht.tracker_import import tracker_import settings = Blueprint("settings", __name__) @@ -63,24 +63,33 @@ def details_POST(owner, name): abort(403) valid = Validation(request) - desc = valid.optional("tracker_desc", default=tracker.description) - vis = valid.require("visibility", cls=Visibility) - valid.expect(not desc or len(desc) < 4096, - "Must be less than 4096 characters", - field="tracker_desc") + rewrite = lambda value: None if value == "" else value + input = { + key: rewrite(valid.source[key]) for key in [ + "description", "visibility", + ] if valid.source.get(key) is not None + } + + resp = exec_gql(current_app.site, """ + mutation UpdateTracker($id: Int!, $input: TrackerInput!) { + updateTracker(id: $id, input: $input) { + name + owner { + canonicalName + } + } + } + """, valid=valid, id=tracker.id, input=input) + if not valid.ok: return render_template("tracker-details.html", tracker=tracker, **valid.kwargs), 400 - tracker.description = desc - tracker.visibility = vis + resp = resp["updateTracker"] - UserWebhook.deliver(UserWebhook.Events.tracker_update, - tracker.to_dict(), - UserWebhook.Subscription.user_id == tracker.owner_id) - - db.session.commit() - return redirect(tracker_url(tracker)) + return redirect(url_for("settings.details_GET", + owner=resp["owner"]["canonicalName"], + name=resp["name"])) def render_tracker_access(tracker, **kwargs): @@ -111,17 +120,23 @@ def access_POST(owner, name): valid = Validation(request) access = parse_html_perms('default', valid) + input = { + perm: ((access & TicketAccess[perm].value) != 0) for perm in [ + "browse", "submit", "comment", "edit", "triage", + ] + } + + resp = exec_gql(current_app.site, """ + mutation updateTrackerACL($id: Int!, $input: ACLInput!) { + updateTrackerACL(trackerId: $id, input: $input) { + browse + } + } + """, valid=valid, id=tracker.id, input=input) if not valid.ok: return render_tracker_access(tracker, **valid.kwargs), 400 - tracker.default_access = access - - UserWebhook.deliver(UserWebhook.Events.tracker_update, - tracker.to_dict(), - UserWebhook.Subscription.user_id == tracker.owner_id) - - db.session.commit() return redirect(tracker_url(tracker)) @settings.route("///settings/user-access/create", methods=["POST"]) @@ -198,20 +213,14 @@ def delete_POST(owner, name): abort(404) if current_user.id != tracker.owner_id: abort(403) + + exec_gql(current_app.site, """ + mutation DeleteTracker($id: Int!) { + deleteTracker(id: $id) { id } + } + """, id=tracker.id); + session["notice"] = f"{tracker.owner}/{tracker.name} was deleted." - # SQLAlchemy shits itself on some of our weird constraints/relationships - # so fuck it, postgres knows what to do here - tracker_id = tracker.id - owner_id = tracker.owner_id - assert isinstance(tracker_id, int) - db.session.expunge_all() - db.engine.execute(f"DELETE FROM tracker WHERE id = {tracker_id};") - db.session.commit() - - UserWebhook.deliver(UserWebhook.Events.tracker_delete, - { "id": tracker_id }, - UserWebhook.Subscription.user_id == owner_id) - return redirect(url_for("html.index_GET")) @settings.route("///settings/import-export") diff --git a/todosrht/blueprints/tracker.py b/todosrht/blueprints/tracker.py index e79116e..8c9dfc8 100644 --- a/todosrht/blueprints/tracker.py +++ b/todosrht/blueprints/tracker.py @@ -1,20 +1,21 @@ -from flask import Blueprint, render_template, request, url_for, abort, redirect +from flask import Blueprint, current_app, render_template, request, url_for, abort, redirect from srht.config import cfg from srht.database import db from srht.flask import paginate_query, session +from srht.graphql import exec_gql from srht.oauth import current_user, loginrequired from srht.validation import Validation -from todosrht.access import get_tracker +from todosrht.access import get_tracker, get_ticket from todosrht.color import color_from_hex, color_to_hex, get_text_color from todosrht.color import valid_hex_color_code from todosrht.filters import render_markup from todosrht.search import apply_search -from todosrht.tickets import get_participant_for_user, submit_ticket +from todosrht.tickets import get_participant_for_user from todosrht.types import Event, Label, TicketLabel from todosrht.types import TicketSubscription, Participant from todosrht.types import Tracker, Ticket, TicketAccess from todosrht.urls import tracker_url, ticket_url -from todosrht.webhooks import TrackerWebhook, UserWebhook +from todosrht.webhooks import TrackerWebhook from urllib.parse import quote import sqlalchemy as sa @@ -41,30 +42,37 @@ def create_GET(): @tracker.route("/tracker/create", methods=["POST"]) @loginrequired def create_POST(): - tracker, valid = Tracker.create_from_request(request, current_user) + valid = Validation(request) + name = valid.require("name", friendly_name="Name") + visibility = valid.require("visibility") + description = valid.optional("description") if not valid.ok: return render_template("tracker-create.html", **valid.kwargs), 400 - db.session.add(tracker) - db.session.flush() + resp = exec_gql(current_app.site, """ + mutation CreateTracker($name: String!, $description: String, $visibility: Visibility!) { + createTracker(name: $name, description: $description, visibility: $visibility) { + name + owner { + canonicalName + } + } + } + """, valid=valid, name=name, description=description, visibility=visibility) - UserWebhook.deliver(UserWebhook.Events.tracker_create, - tracker.to_dict(), - UserWebhook.Subscription.user_id == tracker.owner_id) + if not valid.ok: + return render_template("tracker-create.html", **valid.kwargs), 400 - participant = get_participant_for_user(current_user) - sub = TicketSubscription() - sub.tracker_id = tracker.id - sub.participant_id = participant.id - db.session.add(sub) - db.session.commit() + resp = resp["createTracker"] if "create-configure" in valid: return redirect(url_for("settings.details_GET", owner=current_user.canonical_name, - name=tracker.name)) + name=resp["name"])) - return redirect(tracker_url(tracker)) + return redirect(url_for("tracker.tracker_GET", + owner=resp["owner"]["canonicalName"], + name=resp["name"])) def return_tracker(tracker, access, **kwargs): another = session.get("another") or False @@ -183,20 +191,14 @@ def tracker_submit_POST(owner, name): if not TicketAccess.submit in access: abort(403) + db.session.commit() # Unlock tracker row + valid = Validation(request) title = valid.require("title", friendly_name="Title") desc = valid.optional("description") another = valid.optional("another") - valid.expect(not title or 3 <= len(title) <= 2048, - "Title must be between 3 and 2048 characters.", - field="title") - valid.expect(not desc or len(desc) < 16384, - "Description must be no more than 16384 characters.", - field="description") - if not valid.ok: - db.session.commit() # Unlock tracker row return return_tracker(tracker, access, **valid.kwargs), 400 if "preview" in request.form: @@ -204,13 +206,24 @@ def tracker_submit_POST(owner, name): return return_tracker(tracker, access, rendered_preview=preview, **valid.kwargs), 200 - # TODO: Handle unique constraint failure (contention) and retry? - participant = get_participant_for_user(current_user) - ticket = submit_ticket(tracker, participant, title, desc) + input = { + "subject": title, + "body": desc, + } + + resp = exec_gql(current_app.site, """ + mutation SubmitTicket($trackerId: Int!, $input: SubmitTicketInput!) { + submitTicket(trackerId: $trackerId, input: $input) { + id + } + } + """, valid=valid, trackerId=tracker.id, input=input) + + if not valid.ok: + return return_tracker(tracker, access, **valid.kwargs), 400 + + ticket, _ = get_ticket(tracker, resp["submitTicket"]["id"]) - UserWebhook.deliver(UserWebhook.Events.ticket_create, - ticket.to_dict(), - UserWebhook.Subscription.user_id == current_user.id) TrackerWebhook.deliver(TrackerWebhook.Events.ticket_create, ticket.to_dict(), TrackerWebhook.Subscription.tracker_id == tracker.id) diff --git a/todosrht/templates/tracker-details.html b/todosrht/templates/tracker-details.html index 499888e..e54fdbe 100644 --- a/todosrht/templates/tracker-details.html +++ b/todosrht/templates/tracker-details.html @@ -6,31 +6,30 @@
{{csrf_token()}}
-
+
- {{ valid.summary("tracker_name") }} + {{ valid.summary("name") }}
-
- +
+ - {{ valid.summary("tracker_desc") }} + >{{tracker.description}} + {{ valid.summary("description") }}
{{ valid.summary() }} diff --git a/todosrht/types/tracker.py b/todosrht/types/tracker.py index dd579a9..fb9c1bb 100644 --- a/todosrht/types/tracker.py +++ b/todosrht/types/tracker.py @@ -45,49 +45,6 @@ class Tracker(Base): import_in_progress = sa.Column(sa.Boolean, nullable=False, server_default='f') - @staticmethod - def create_from_request(request, user): - valid = Validation(request) - name = valid.require("name", friendly_name="Name") - visibility = valid.require("visibility", cls=Visibility) - desc = valid.optional("description") - if not valid.ok: - return None, valid - - valid.expect(1 <= len(name) < 256, - "Must be between 1 and 255 characters", - field="name") - valid.expect(not valid.ok or name_re.match(name), - "Name must match [A-Za-z0-9._-]+", - field="name") - valid.expect(not valid.ok or name not in [".", ".."], - "Name cannot be '.' or '..'", - field="name") - valid.expect(not valid.ok or name not in [".git", ".hg"], - "Name must not be '.git' or '.hg'", - field="name") - valid.expect(not desc or len(desc) < 4096, - "Must be less than 4096 characters", - field="description") - if not valid.ok: - return None, valid - - tracker = (Tracker.query - .filter(Tracker.owner_id == user.id) - .filter(Tracker.name.ilike(name.replace('_', '\\_'))) - ).first() - valid.expect(not tracker, - "A tracker by this name already exists", field="name") - if not valid.ok: - return None, valid - - tracker = Tracker(owner=user, - name=name, - description=desc, - visibility=visibility) - - return tracker, valid - def ref(self): return "{}/{}".format( self.owner.canonical_name, @@ -114,10 +71,3 @@ class Tracker(Base): "visibility": self.visibility, } if not short else {}) } - - def update(self, valid): - desc = valid.optional("description", default=self.description) - valid.expect(not desc or len(desc) < 4096, - "Must be less than 4096 characters", - field="description") - self.description = desc