diff --git a/src/website/shared/listeners/cache_suggestions.py b/src/website/shared/listeners/cache_suggestions.py index 409ae2cc..60525d1a 100644 --- a/src/website/shared/listeners/cache_suggestions.py +++ b/src/website/shared/listeners/cache_suggestions.py @@ -317,10 +317,12 @@ def maintainers_list(packages: dict, edits: list[MaintainersEdit]) -> list[dict] to_skip_or_seen: set[int] = { m.maintainer.github_id for m in edits - if m.type == MaintainersEdit.EditType.REMOVE + if m.edit_type == MaintainersEdit.EditType.REMOVE } to_add: list[dict] = [ - to_dict(m.maintainer) for m in edits if m.type == MaintainersEdit.EditType.ADD + to_dict(m.maintainer) + for m in edits + if m.edit_type == MaintainersEdit.EditType.ADD ] maintainers: list[dict] = list() diff --git a/src/website/shared/tests/test_github_sync.py b/src/website/shared/tests/test_github_sync.py index aa3b9348..d18ff6ae 100644 --- a/src/website/shared/tests/test_github_sync.py +++ b/src/website/shared/tests/test_github_sync.py @@ -9,6 +9,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.test import TestCase + from shared.auth import isadmin, iscommitter from shared.auth.github_state import GithubState, set_groups_for_new_user from shared.auth.github_webhook import handle_webhook diff --git a/src/website/webview/static/style.css b/src/website/webview/static/style.css index b4fb2dad..41e689c2 100644 --- a/src/website/webview/static/style.css +++ b/src/website/webview/static/style.css @@ -746,7 +746,8 @@ article .nixpkgs-packages { display: none; } -.nixpkgs-package { +.nixpkgs-package, +.nixpkg { flex-grow: 1; border: 1px solid var(--grey); border-radius: 5px; @@ -830,9 +831,8 @@ article .nixpkgs-packages { margin-top: 1em; } -.maintainers ul { +.maintainers article { margin: 0; - list-style: none; padding-left: 1em; } @@ -850,6 +850,63 @@ a:focus { text-decoration: underline; } +.maintainers-list { + display: flex; + flex-direction: column; + margin-left: 1em; +} + +.maintainer-container { + display: flex; + flex-direction: row; + align-items: center; + height: 2em; +} + +.maintainer-container input { + margin: 0; + appearance: none; + width: 2em; + height: 2em; + cursor: pointer; +} + +.maintainer-container button { + position: relative; + padding: 0; + border: none; + background: none; + text-align: center; + cursor: pointer; +} + +.maintainer-edit-add button::after { + top: -0.2em; + content: "↺"; + font-weight: 700; + font-size: 1.5em; + color: var(--grey); +} + +.maintainer-edit-remove button::after { + top: 0.2em; + content: "🗑"; + text-align: center; + font-size: 1.2em; + color: var(--dark-grey); +} + +.maintainer-container input ~ article { + margin: 0; + padding: 0; +} + +.maintainer-container .maintainer-edit-add article, +.maintainer-container .maintainer-edit-add a { + color: var(--grey); + text-decoration: line-through; +} + .suggestion .change-issue-state { display: flex; justify-content: space-between; diff --git a/src/website/webview/templates/components/maintainer.html b/src/website/webview/templates/components/maintainer.html new file mode 100644 index 00000000..57ba61eb --- /dev/null +++ b/src/website/webview/templates/components/maintainer.html @@ -0,0 +1,7 @@ +
+ @{{ maintainer.github }} + {{ maintainer.name }} + {% if maintainer.email %} + <{{ maintainer.email }}> + {% endif %} +
diff --git a/src/website/webview/templates/components/maintainers_list.html b/src/website/webview/templates/components/maintainers_list.html index 97d71eb7..74dc2334 100644 --- a/src/website/webview/templates/components/maintainers_list.html +++ b/src/website/webview/templates/components/maintainers_list.html @@ -1,18 +1,21 @@ +{% load viewutils %} + {% if maintainers %}
Notify package maintainers: {{ maintainers | length }} - +
{% endif %} diff --git a/src/website/webview/templates/components/nixpkgs_package_list.html b/src/website/webview/templates/components/nixpkgs_package_list.html index b1f3f69c..60bd002a 100644 --- a/src/website/webview/templates/components/nixpkgs_package_list.html +++ b/src/website/webview/templates/components/nixpkgs_package_list.html @@ -1,9 +1,20 @@ {% load viewutils %}
- {% for attribute_name, pdata in packages.items %} + {% for attribute, pdata in packages.items %}
- {% nixpkgs_package attribute_name pdata %} + {% if selectable %} + + {% endif %} + {% nixpkgs_package attribute pdata %}
{% endfor %}
diff --git a/src/website/webview/templates/components/selectable_maintainer.html b/src/website/webview/templates/components/selectable_maintainer.html new file mode 100644 index 00000000..06fadbf5 --- /dev/null +++ b/src/website/webview/templates/components/selectable_maintainer.html @@ -0,0 +1,15 @@ +{% load viewutils %} + +
+ + {% maintainer maintainer %} +
diff --git a/src/website/webview/templates/components/selectable_nixpkgs_package_list.html b/src/website/webview/templates/components/selectable_nixpkgs_package_list.html deleted file mode 100644 index 9427e501..00000000 --- a/src/website/webview/templates/components/selectable_nixpkgs_package_list.html +++ /dev/null @@ -1,18 +0,0 @@ -{% load viewutils %} - -
- {% for attribute, pdata in packages.items %} -
- - {% nixpkgs_package attribute pdata %} -
- {% endfor %} -
diff --git a/src/website/webview/templates/components/suggestion.html b/src/website/webview/templates/components/suggestion.html index 338cadd0..42b44a95 100644 --- a/src/website/webview/templates/components/suggestion.html +++ b/src/website/webview/templates/components/suggestion.html @@ -49,7 +49,11 @@ {% nixpkgs_package_list cached_suggestion.packages %} {% endif %} - {% maintainers_list cached_suggestion.maintainers %} + {% if status_filter == "pending" or status_filter == "accepted" %} + {% selectable_maintainers_list cached_suggestion.maintainers %} + {% else %} + {% maintainers_list cached_suggestion.maintainers %} + {% endif %} {% if user|is_maintainer_or_admin %}
diff --git a/src/website/webview/templatetags/viewutils.py b/src/website/webview/templatetags/viewutils.py index e402afed..18e8bf2a 100644 --- a/src/website/webview/templatetags/viewutils.py +++ b/src/website/webview/templatetags/viewutils.py @@ -40,6 +40,7 @@ class PackageList(TypedDict): class PackageListContext(TypedDict): packages: PackageList + selectable: bool class AffectedContext(TypedDict): @@ -59,6 +60,20 @@ class Maintainer(TypedDict): github_id: int +class MaintainerContext(TypedDict): + maintainer: Maintainer + + +class SelectableMaintainerContext(TypedDict): + maintainer: Maintainer + deleted: bool + + +class MaintainersListContext(TypedDict): + maintainers: list[Maintainer] + selectable: bool + + @register.filter def getitem(dictionary: dict, key: str) -> Any | None: return dictionary.get(key) @@ -145,7 +160,7 @@ def nixpkgs_package(attribute_name: str, pdata: Package) -> PackageContext: return {"attribute_name": attribute_name, "pdata": pdata} -@register.inclusion_tag("components/selectable_nixpkgs_package_list.html") +@register.inclusion_tag("components/nixpkgs_package_list.html") def selectable_nixpkgs_package_list(packages: PackageList) -> PackageListContext: """Renders the nixpkgs package list with additional checkboxes to have packages selectable. @@ -156,10 +171,11 @@ def selectable_nixpkgs_package_list(packages: PackageList) -> PackageListContext Context dictionary for the template Example: - {% package_list package_dict %} + {% selectable_nixpkgs_package_list package_dict %} """ return { "packages": packages, + "selectable": True, } @@ -174,10 +190,11 @@ def nixpkgs_package_list(packages: PackageList) -> PackageListContext: Context dictionary for the template Example: - {% package_list package_dict %} + {% nixpkgs_package_list package_dict %} """ return { "packages": packages, + "selectable": False, } @@ -196,6 +213,34 @@ def suggestion_activity_log( @register.inclusion_tag("components/maintainers_list.html") def maintainers_list( - maintainers: list[dict], -) -> dict[str, list[dict]]: - return {"maintainers": maintainers} + maintainers: list[Maintainer], +) -> MaintainersListContext: + return { + "maintainers": maintainers, + "selectable": False, + } + + +@register.inclusion_tag("components/maintainers_list.html") +def selectable_maintainers_list( + maintainers: list[Maintainer], +) -> MaintainersListContext: + return { + "maintainers": maintainers, + "selectable": True, + } + + +@register.inclusion_tag("components/maintainer.html") +def maintainer( + maintainer: Maintainer, +) -> MaintainerContext: + return {"maintainer": maintainer} + + +@register.inclusion_tag("components/selectable_maintainer.html") +def selectable_maintainer( + maintainer: Maintainer, + deleted: bool = False, +) -> SelectableMaintainerContext: + return {"maintainer": maintainer, "deleted": deleted} diff --git a/src/website/webview/urls.py b/src/website/webview/urls.py index 846e02e0..57b22a98 100644 --- a/src/website/webview/urls.py +++ b/src/website/webview/urls.py @@ -10,6 +10,7 @@ NixderivationPerChannelView, NixpkgsIssueListView, NixpkgsIssueView, + SelectableMaintainerView, SuggestionListView, TriageView, ) @@ -50,6 +51,12 @@ ), name="suggestions_view", ), + # This is a POST endpoint only, handling maintainers edit requests. + path( + "edit_maintainers/", + SelectableMaintainerView.as_view(), + name="edit_maintainers", + ), path( "dismissed/", SuggestionListView.as_view( diff --git a/src/website/webview/views.py b/src/website/webview/views.py index 813251ce..25914def 100644 --- a/src/website/webview/views.py +++ b/src/website/webview/views.py @@ -9,6 +9,7 @@ from django.db import transaction from django.urls import reverse from shared.github import create_gh_issue +from shared.listeners.cache_suggestions import maintainers_list from shared.logs import SuggestionActivityLog from shared.models.cached import CachedSuggestions @@ -39,6 +40,7 @@ HttpRequest, HttpResponse, HttpResponseForbidden, + HttpResponseNotAllowed, HttpResponseRedirect, ) from django.middleware.csrf import get_token @@ -54,11 +56,10 @@ IssueStatus, NixChannel, NixDerivation, + NixMaintainer, NixpkgsIssue, ) -from shared.models.linkage import ( - CVEDerivationClusterProposal, -) +from shared.models.linkage import CVEDerivationClusterProposal, MaintainersEdit from webview.forms import NixpkgsIssueForm from webview.paginators import CustomCountPaginator @@ -696,3 +697,117 @@ def suggestion_view_context() -> dict: else: # Just reload the page return redirect(f"{request.path}?page={current_page}") + + +class SelectableMaintainerView(TemplateView): + template_name = "components/selectable_maintainer.html" + + def dispatch(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse: + # Only allow POST requests + if request.method != "POST": + return HttpResponseNotAllowed(["POST"]) + return super().dispatch(request, *args, **kwargs) + + def post(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse: + if not request.user or not ( + isadmin(request.user) or ismaintainer(request.user) + ): + return HttpResponseForbidden() + + suggestion_id = request.POST.get("suggestion_id") + suggestion = get_object_or_404(CVEDerivationClusterProposal, id=suggestion_id) + cached_suggestion = get_object_or_404( + CachedSuggestions, proposal_id=suggestion_id + ) + edit_maintainer_id = request.POST.get("edit_maintainer_id") + # Which states allow for maintainer editing + editable = ( + suggestion.status == CVEDerivationClusterProposal.Status.ACCEPTED + or suggestion.status == CVEDerivationClusterProposal.Status.PENDING + ) + + if not editable: + logger.error( + f"Tried to edit maintainers on a suggestion whose status doesn't allow for maintainer edition (status: {suggestion.status})" + ) + return HttpResponseForbidden() + + if not edit_maintainer_id: + # Unprocessable Entity seems to be the more appropriate status code + # for missing parameters (the request is well-formed at the protocol + # level but some semantic precondition failed) + logger.error("Missing edit_maintainer_id in request for maintainer edition") + return HttpResponse(status=422) + + # When clicking the button to the left of a maintainer, there are two + # cases: + # + # 1. The maintainer is currently in the list of maintainers: the button + # was a remove button, and we should remove the maintainer from the + # list. + # 2. The maintainer is not in the list of maintainers: the button was + # an add button, and we should add the maintainer to the list. + # + # The button basically works as a toggle. Both cases have themselves two + # sub-cases, depending on the existence of a prior edit: + # + # 1. Removal + # a) there was no prior edit, in which case we add a new "remove" edit + # b) there was a prior "add" edit, in which case we remove the "add" edit from the list (meaning + # the maintainer wasn't part of the list originally) + # 2. Addition + # a) there was no prior edit, in which case we add a new "add" edit + # b) there was a prior "remove" edit (undo/add back case), in which case we remove the edit from the + # list + # + # Note that in both cases, if there was a prior edit, we always remove + # it from the list (1b and 2b). + # + # Also note that for now add edits are unimplemented on the front-end + # (but addition as undoing a removal is). + with transaction.atomic(): + edit = suggestion.maintainers_edits.filter( + maintainer__github_id=edit_maintainer_id + ) + # case 1b and 2b + if edit.exists(): + edit_object = edit.first() + maintainer = edit_object.maintainer + deleted = edit_object.edit_type == MaintainersEdit.EditType.ADD + edit.delete() + suggestion.save() + # case 1a and 2a + else: + maintainer = get_object_or_404( + NixMaintainer, github_id=edit_maintainer_id + ) + was_there = any( + str(m["github_id"]) == edit_maintainer_id + for m in cached_suggestion.payload["maintainers"] + ) + edit_type = ( + MaintainersEdit.EditType.REMOVE + if was_there + else MaintainersEdit.EditType.ADD + ) + deleted = was_there + edit = MaintainersEdit( + edit_type=edit_type, + maintainer=maintainer, + suggestion=suggestion, + ) + edit.save() + + # Recompute the maintainer list for the cached suggestion + cached_suggestion.payload["maintainers"] = maintainers_list( + cached_suggestion.payload["packages"], + suggestion.maintainers_edits.all(), + ) + cached_suggestion.save() + + return self.render_to_response( + { + "maintainer": maintainer, + "deleted": deleted, + } + )