diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index b2bb220f..d8cfa577 100644 --- a/docs/api/schemas/latest/patchwork.yaml +++ b/docs/api/schemas/latest/patchwork.yaml @@ -319,6 +319,19 @@ paths: - $ref: '#/components/parameters/PageSize' - $ref: '#/components/parameters/Order' - $ref: '#/components/parameters/Search' + - in: query + name: type + required: false + description: | + Filter comment type. Use 'notes' to show only maintainer notes, + 'comments' to show only regular comments, or omit for default behavior + (which only shows regular comments). + schema: + title: 'Comment type' + type: string + enum: + - notes + - comments responses: '200': description: 'List of comments' @@ -339,6 +352,43 @@ paths: $ref: '#/components/schemas/Error' tags: - comments + post: + summary: Create a maintainer note in the cover + description: | + Create a maintainer note in the cover + operationId: cover_maintainer_note_create + requestBody: + $ref: '#/components/requestBodies/MaintainerNote' + responses: + '201': + description: 'Created maintainer note' + headers: + Link: + $ref: '#/components/headers/Link' + content: + application/json: + schema: + $ref: '#/components/schemas/Comment' + '400': + description: 'Invalid request' + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMaintainerNote' + '403': + description: 'Forbidden' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: 'Not found' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - comments /api/covers/{cover_id}/comments/{comment_id}: parameters: - in: path @@ -356,7 +406,7 @@ paths: title: Comment ID type: integer get: - summary: Show a cover letter comment. + summary: Show a cover letter comment or maintainer note. description: | Retrieve a cover letter comment by its ID. operationId: cover_comments_read @@ -376,7 +426,7 @@ paths: tags: - comments patch: - summary: Update a cover letter comment (partial). + summary: Update a cover letter comment or maintainer note (partial). description: Partially update an existing cover letter comment. You must be a maintainer of the project that the cover letter comment belongs to. @@ -725,6 +775,19 @@ paths: - $ref: '#/components/parameters/PageSize' - $ref: '#/components/parameters/Order' - $ref: '#/components/parameters/Search' + - in: query + name: type + required: false + description: | + Filter comment type. Use 'notes' to show only maintainer notes, + 'comments' to show only regular comments, or omit for default behavior + (which only shows regular comments). + schema: + title: 'Comment type' + type: string + enum: + - notes + - comments responses: '200': description: 'List of comments' @@ -745,6 +808,43 @@ paths: $ref: '#/components/schemas/Error' tags: - comments + post: + summary: Create a maintainer note in the patch + description: | + Create a maintainer note in the patch + operationId: patch_maintainer_note_create + requestBody: + $ref: '#/components/requestBodies/MaintainerNote' + responses: + '201': + description: 'Created maintainer note' + headers: + Link: + $ref: '#/components/headers/Link' + content: + application/json: + schema: + $ref: '#/components/schemas/Comment' + '400': + description: 'Invalid request' + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMaintainerNote' + '403': + description: 'Forbidden' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: 'Not found' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - comments /api/patches/{patch_id}/comments/{comment_id}: parameters: - in: path @@ -762,7 +862,7 @@ paths: title: Comment ID type: integer get: - summary: Show a patch comment. + summary: Show a patch comment or maintainer note. description: | Retrieve a patch comment by its ID and the ID of the patch. operationId: patch_comments_read @@ -782,7 +882,7 @@ paths: tags: - comments patch: - summary: Update a patch comment (partial). + summary: Update a patch comment or mantainer note (partial). description: Partially update an existing patch comment. You must be a maintainer of the project that the patch comment belongs to. @@ -1478,6 +1578,14 @@ components: application/json: schema: $ref: '#/components/schemas/CommentUpdate' + MaintainerNote: + required: true + description: | + A maintainer note for patch or cover letters. + content: + application/json: + schema: + $ref: '#/components/schemas/MaintainerNote' Patch: required: true description: | @@ -1770,7 +1878,6 @@ components: title: Message ID type: string readOnly: true - minLength: 1 maxLength: 255 list_archive_url: title: List archive URL @@ -1801,7 +1908,6 @@ components: content: title: Content type: string - readOnly: true minLength: 1 headers: title: Headers @@ -1831,6 +1937,20 @@ components: type: - 'null' - 'boolean' + MaintainerNote: + type: object + title: Maintainer note + description: | + The fields to set on a maintainer note. + properties: + content: + title: Content + type: string + addressed: + title: Addressed + type: + - 'null' + - 'boolean' CoverList: type: object title: Cover letters @@ -3153,6 +3273,22 @@ components: type: array items: type: string + ErrorMaintainerNote: + type: object + title: A maintainer note create or update error. + description: | + A mapping of field names to validation failures. + properties: + content: + title: Content + type: array + items: + type: string + addressed: + title: Addressed + type: array + items: + type: string ErrorPatchUpdate: type: object title: A patch update error. diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index f37d3213..f8c9cff7 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -330,6 +330,21 @@ paths: - $ref: '#/components/parameters/PageSize' - $ref: '#/components/parameters/Order' - $ref: '#/components/parameters/Search' +{% if version >= (1, 4) %} + - in: query + name: type + required: false + description: | + Filter comment type. Use 'notes' to show only maintainer notes, + 'comments' to show only regular comments, or omit for default behavior + (which only shows regular comments). + schema: + title: 'Comment type' + type: string + enum: + - notes + - comments +{% endif %} responses: '200': description: 'List of comments' @@ -350,6 +365,45 @@ paths: $ref: '#/components/schemas/Error' tags: - comments +{% if version >= (1, 4) %} + post: + summary: Create a maintainer note in the cover + description: | + Create a maintainer note in the cover + operationId: cover_maintainer_note_create + requestBody: + $ref: '#/components/requestBodies/MaintainerNote' + responses: + '201': + description: 'Created maintainer note' + headers: + Link: + $ref: '#/components/headers/Link' + content: + application/json: + schema: + $ref: '#/components/schemas/Comment' + '400': + description: 'Invalid request' + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMaintainerNote' + '403': + description: 'Forbidden' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: 'Not found' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - comments +{% endif %} {% if version >= (1, 3) %} /api/{{ version_url }}covers/{cover_id}/comments/{comment_id}: parameters: @@ -368,7 +422,7 @@ paths: title: Comment ID type: integer get: - summary: Show a cover letter comment. + summary: Show a cover letter comment or maintainer note. description: | Retrieve a cover letter comment by its ID. operationId: cover_comments_read @@ -388,7 +442,7 @@ paths: tags: - comments patch: - summary: Update a cover letter comment (partial). + summary: Update a cover letter comment or maintainer note (partial). description: Partially update an existing cover letter comment. You must be a maintainer of the project that the cover letter comment belongs to. @@ -748,6 +802,21 @@ paths: - $ref: '#/components/parameters/PageSize' - $ref: '#/components/parameters/Order' - $ref: '#/components/parameters/Search' +{% if version >= (1, 4) %} + - in: query + name: type + required: false + description: | + Filter comment type. Use 'notes' to show only maintainer notes, + 'comments' to show only regular comments, or omit for default behavior + (which only shows regular comments). + schema: + title: 'Comment type' + type: string + enum: + - notes + - comments +{% endif %} responses: '200': description: 'List of comments' @@ -768,6 +837,45 @@ paths: $ref: '#/components/schemas/Error' tags: - comments +{% if version >= (1, 4) %} + post: + summary: Create a maintainer note in the patch + description: | + Create a maintainer note in the patch + operationId: patch_maintainer_note_create + requestBody: + $ref: '#/components/requestBodies/MaintainerNote' + responses: + '201': + description: 'Created maintainer note' + headers: + Link: + $ref: '#/components/headers/Link' + content: + application/json: + schema: + $ref: '#/components/schemas/Comment' + '400': + description: 'Invalid request' + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMaintainerNote' + '403': + description: 'Forbidden' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: 'Not found' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - comments +{% endif %} {% if version >= (1, 3) %} /api/{{ version_url }}patches/{patch_id}/comments/{comment_id}: parameters: @@ -786,7 +894,7 @@ paths: title: Comment ID type: integer get: - summary: Show a patch comment. + summary: Show a patch comment or maintainer note. description: | Retrieve a patch comment by its ID and the ID of the patch. operationId: patch_comments_read @@ -806,7 +914,7 @@ paths: tags: - comments patch: - summary: Update a patch comment (partial). + summary: Update a patch comment or mantainer note (partial). description: Partially update an existing patch comment. You must be a maintainer of the project that the patch comment belongs to. @@ -1518,6 +1626,16 @@ components: application/json: schema: $ref: '#/components/schemas/CommentUpdate' +{% endif %} +{% if version >= (1, 4) %} + MaintainerNote: + required: true + description: | + A maintainer note for patch or cover letters. + content: + application/json: + schema: + $ref: '#/components/schemas/MaintainerNote' {% endif %} Patch: required: true @@ -1834,7 +1952,9 @@ components: title: Message ID type: string readOnly: true +{% if version < (1, 4) %} minLength: 1 +{% endif %} maxLength: 255 {% if version >= (1, 2) %} list_archive_url: @@ -1867,7 +1987,9 @@ components: content: title: Content type: string +{% if version < (1, 4) %} readOnly: true +{% endif %} minLength: 1 headers: title: Headers @@ -1898,6 +2020,22 @@ components: type: - 'null' - 'boolean' +{% endif %} +{% if version >= (1, 4) %} + MaintainerNote: + type: object + title: Maintainer note + description: | + The fields to set on a maintainer note. + properties: + content: + title: Content + type: string + addressed: + title: Addressed + type: + - 'null' + - 'boolean' {% endif %} CoverList: type: object @@ -3274,6 +3412,24 @@ components: type: array items: type: string +{% endif %} +{% if version >= (1, 4) %} + ErrorMaintainerNote: + type: object + title: A maintainer note create or update error. + description: | + A mapping of field names to validation failures. + properties: + content: + title: Content + type: array + items: + type: string + addressed: + title: Addressed + type: array + items: + type: string {% endif %} ErrorPatchUpdate: type: object diff --git a/docs/api/schemas/v1.3/patchwork.yaml b/docs/api/schemas/v1.3/patchwork.yaml index 41b44832..c6d0f3d7 100644 --- a/docs/api/schemas/v1.3/patchwork.yaml +++ b/docs/api/schemas/v1.3/patchwork.yaml @@ -356,7 +356,7 @@ paths: title: Comment ID type: integer get: - summary: Show a cover letter comment. + summary: Show a cover letter comment or maintainer note. description: | Retrieve a cover letter comment by its ID. operationId: cover_comments_read @@ -376,7 +376,7 @@ paths: tags: - comments patch: - summary: Update a cover letter comment (partial). + summary: Update a cover letter comment or maintainer note (partial). description: Partially update an existing cover letter comment. You must be a maintainer of the project that the cover letter comment belongs to. @@ -762,7 +762,7 @@ paths: title: Comment ID type: integer get: - summary: Show a patch comment. + summary: Show a patch comment or maintainer note. description: | Retrieve a patch comment by its ID and the ID of the patch. operationId: patch_comments_read @@ -782,7 +782,7 @@ paths: tags: - comments patch: - summary: Update a patch comment (partial). + summary: Update a patch comment or mantainer note (partial). description: Partially update an existing patch comment. You must be a maintainer of the project that the patch comment belongs to. diff --git a/docs/api/schemas/v1.4/patchwork.yaml b/docs/api/schemas/v1.4/patchwork.yaml index 036fe15f..6c1a43b3 100644 --- a/docs/api/schemas/v1.4/patchwork.yaml +++ b/docs/api/schemas/v1.4/patchwork.yaml @@ -319,6 +319,19 @@ paths: - $ref: '#/components/parameters/PageSize' - $ref: '#/components/parameters/Order' - $ref: '#/components/parameters/Search' + - in: query + name: type + required: false + description: | + Filter comment type. Use 'notes' to show only maintainer notes, + 'comments' to show only regular comments, or omit for default behavior + (which only shows regular comments). + schema: + title: 'Comment type' + type: string + enum: + - notes + - comments responses: '200': description: 'List of comments' @@ -339,6 +352,43 @@ paths: $ref: '#/components/schemas/Error' tags: - comments + post: + summary: Create a maintainer note in the cover + description: | + Create a maintainer note in the cover + operationId: cover_maintainer_note_create + requestBody: + $ref: '#/components/requestBodies/MaintainerNote' + responses: + '201': + description: 'Created maintainer note' + headers: + Link: + $ref: '#/components/headers/Link' + content: + application/json: + schema: + $ref: '#/components/schemas/Comment' + '400': + description: 'Invalid request' + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMaintainerNote' + '403': + description: 'Forbidden' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: 'Not found' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - comments /api/1.4/covers/{cover_id}/comments/{comment_id}: parameters: - in: path @@ -356,7 +406,7 @@ paths: title: Comment ID type: integer get: - summary: Show a cover letter comment. + summary: Show a cover letter comment or maintainer note. description: | Retrieve a cover letter comment by its ID. operationId: cover_comments_read @@ -376,7 +426,7 @@ paths: tags: - comments patch: - summary: Update a cover letter comment (partial). + summary: Update a cover letter comment or maintainer note (partial). description: Partially update an existing cover letter comment. You must be a maintainer of the project that the cover letter comment belongs to. @@ -725,6 +775,19 @@ paths: - $ref: '#/components/parameters/PageSize' - $ref: '#/components/parameters/Order' - $ref: '#/components/parameters/Search' + - in: query + name: type + required: false + description: | + Filter comment type. Use 'notes' to show only maintainer notes, + 'comments' to show only regular comments, or omit for default behavior + (which only shows regular comments). + schema: + title: 'Comment type' + type: string + enum: + - notes + - comments responses: '200': description: 'List of comments' @@ -745,6 +808,43 @@ paths: $ref: '#/components/schemas/Error' tags: - comments + post: + summary: Create a maintainer note in the patch + description: | + Create a maintainer note in the patch + operationId: patch_maintainer_note_create + requestBody: + $ref: '#/components/requestBodies/MaintainerNote' + responses: + '201': + description: 'Created maintainer note' + headers: + Link: + $ref: '#/components/headers/Link' + content: + application/json: + schema: + $ref: '#/components/schemas/Comment' + '400': + description: 'Invalid request' + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMaintainerNote' + '403': + description: 'Forbidden' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: 'Not found' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - comments /api/1.4/patches/{patch_id}/comments/{comment_id}: parameters: - in: path @@ -762,7 +862,7 @@ paths: title: Comment ID type: integer get: - summary: Show a patch comment. + summary: Show a patch comment or maintainer note. description: | Retrieve a patch comment by its ID and the ID of the patch. operationId: patch_comments_read @@ -782,7 +882,7 @@ paths: tags: - comments patch: - summary: Update a patch comment (partial). + summary: Update a patch comment or mantainer note (partial). description: Partially update an existing patch comment. You must be a maintainer of the project that the patch comment belongs to. @@ -1478,6 +1578,14 @@ components: application/json: schema: $ref: '#/components/schemas/CommentUpdate' + MaintainerNote: + required: true + description: | + A maintainer note for patch or cover letters. + content: + application/json: + schema: + $ref: '#/components/schemas/MaintainerNote' Patch: required: true description: | @@ -1770,7 +1878,6 @@ components: title: Message ID type: string readOnly: true - minLength: 1 maxLength: 255 list_archive_url: title: List archive URL @@ -1801,7 +1908,6 @@ components: content: title: Content type: string - readOnly: true minLength: 1 headers: title: Headers @@ -1831,6 +1937,20 @@ components: type: - 'null' - 'boolean' + MaintainerNote: + type: object + title: Maintainer note + description: | + The fields to set on a maintainer note. + properties: + content: + title: Content + type: string + addressed: + title: Addressed + type: + - 'null' + - 'boolean' CoverList: type: object title: Cover letters @@ -3153,6 +3273,22 @@ components: type: array items: type: string + ErrorMaintainerNote: + type: object + title: A maintainer note create or update error. + description: | + A mapping of field names to validation failures. + properties: + content: + title: Content + type: array + items: + type: string + addressed: + title: Addressed + type: array + items: + type: string ErrorPatchUpdate: type: object title: A patch update error. diff --git a/htdocs/css/style.css b/htdocs/css/style.css index 268a8c37..6c2aeca8 100644 --- a/htdocs/css/style.css +++ b/htdocs/css/style.css @@ -325,6 +325,26 @@ table.patch-meta tr th, table.patch-meta tr td { color: #f7977a; } +.submission-message.maintainer a { + color: blue; +} + +.submission-message.maintainer div.text-warning { + color: yellow; +} + +.submission-message.maintainer div.text-success { + color: lightgreen; +} +.submission-message.maintainer .meta { + background-color: red; + color: white; +} + +.submission-message.maintainer .content { + background-color: #ff7d7d; +} + .submission-message .meta { display: flex; align-items: center; @@ -449,6 +469,16 @@ div.patch-form { align-items: center; } +#maintainer-note-form .form-actions { + display: flex; + justify-content: flex-start; /* Align buttons to the left */ + gap: 10px; /* Add space between buttons */ +} +#maintainer-note-form div:first-of-type { + display: flex; + flex-direction: column; +} + select[class^=change-property-], .archive-patch-select, .add-bundle { padding: 4px; margin-right: 8px; @@ -476,7 +506,7 @@ select[class^=change-property-], .archive-patch-select, .add-bundle { padding: 4px; } -#patch-form-bundle, #add-to-bundle, #remove-bundle { +#patch-form-bundle, #add-to-bundle, #remove-bundle, #note-button { margin-left: 16px; } diff --git a/patchwork/admin.py b/patchwork/admin.py index d1c389a1..b2cc5bff 100644 --- a/patchwork/admin.py +++ b/patchwork/admin.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later from django.contrib import admin +from django.contrib.admin import SimpleListFilter from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.auth.models import User from django.db.models import Prefetch @@ -112,7 +113,25 @@ def is_pull_request(self, patch): admin.site.register(Patch, PatchAdmin) +class MaintainerNoteFilter(SimpleListFilter): + title = 'comment type' + parameter_name = 'type' + + def lookups(self, request, model_admin): + return [ + ('comments', 'Comments'), + ('notes', 'Maintainer notes'), + ] + + def queryset(self, request, queryset): + if self.value() == 'notes': + return queryset.filter(msgid='') + + return queryset.exclude(msgid='') + + class CoverCommentAdmin(admin.ModelAdmin): + list_filter = [MaintainerNoteFilter] list_display = ('cover', 'submitter', 'date') search_fields = ('cover__name', 'submitter__name', 'submitter__email') date_hierarchy = 'date' @@ -122,6 +141,7 @@ class CoverCommentAdmin(admin.ModelAdmin): class PatchCommentAdmin(admin.ModelAdmin): + list_filter = [MaintainerNoteFilter] list_display = ('patch', 'submitter', 'date') search_fields = ('patch__name', 'submitter__name', 'submitter__email') date_hierarchy = 'date' diff --git a/patchwork/api/base.py b/patchwork/api/base.py index 16e5cb8d..546f0372 100644 --- a/patchwork/api/base.py +++ b/patchwork/api/base.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: GPL-2.0-or-later +from django.urls import resolve import rest_framework from django.conf import settings @@ -15,6 +16,8 @@ from rest_framework.utils.urls import replace_query_param from patchwork.api import utils +from patchwork.models import Cover +from patchwork.models import Patch DRF_VERSION = tuple(int(x) for x in rest_framework.__version__.split('.')) @@ -22,30 +25,69 @@ if DRF_VERSION > (3, 11): + class CurrentPersonDefault(object): + requires_context = True + + def __call__(self, serializer_field): + return ( + serializer_field.context['request'] + .user.person_set.all() + .first() + ) + class CurrentPatchDefault(object): requires_context = True def __call__(self, serializer_field): + req = serializer_field.context['request'] + _, _, kwargs = resolve(req.path) + if 'patch_id' in kwargs: + return get_object_or_404(Patch, id=kwargs['patch_id']) return serializer_field.context['request'].patch class CurrentCoverDefault(object): requires_context = True def __call__(self, serializer_field): - return serializer_field.context['request'].cover + req = serializer_field.context['request'] + _, _, kwargs = resolve(req.path) + if 'cover_id' in kwargs: + return get_object_or_404(Cover, id=kwargs['cover_id']) + return req.cover else: + class CurrentPersonDefault(object): + def set_context(self, serializer_field): + self.person = ( + serializer_field.context['request'] + .user.person_set.all() + .first() + ) + + def __call__(self): + return self.person + class CurrentPatchDefault(object): def set_context(self, serializer_field): - self.patch = serializer_field.context['request'].patch + req = serializer_field.context['request'] + _, _, kwargs = resolve(req.path) + if 'patch_id' in kwargs: + self.patch = get_object_or_404(Patch, id=kwargs['patch_id']) + else: + self.patch = req.patch def __call__(self): return self.patch class CurrentCoverDefault(object): def set_context(self, serializer_field): - self.patch = serializer_field.context['request'].cover + req = serializer_field.context['request'] + _, _, kwargs = resolve(req.path) + if 'cover_id' in kwargs: + self.cover = get_object_or_404(Cover, id=kwargs['cover_id']) + else: + self.cover = req.cover def __call__(self): return self.cover diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py index 88707b84..ee518f1a 100644 --- a/patchwork/api/comment.py +++ b/patchwork/api/comment.py @@ -5,17 +5,26 @@ import email.parser +from rest_framework.generics import RetrieveUpdateDestroyAPIView from rest_framework.generics import get_object_or_404 -from rest_framework.generics import ListAPIView -from rest_framework.generics import RetrieveUpdateAPIView +from rest_framework.generics import ListCreateAPIView +from rest_framework.permissions import SAFE_METHODS +from rest_framework.serializers import CreateOnlyDefault +from rest_framework.serializers import CharField from rest_framework.serializers import HiddenField +from rest_framework.serializers import ValidationError from rest_framework.serializers import SerializerMethodField +from rest_framework.views import PermissionDenied +from rest_framework.exceptions import NotAuthenticated +from django_filters.rest_framework import ChoiceFilter +from django_filters.rest_framework import FilterSet from patchwork.api.base import BaseHyperlinkedModelSerializer +from patchwork.api.base import CurrentCoverDefault +from patchwork.api.base import CurrentPersonDefault from patchwork.api.base import NestedHyperlinkedIdentityField from patchwork.api.base import MultipleFieldLookupMixin from patchwork.api.base import PatchworkPermission -from patchwork.api.base import CurrentCoverDefault from patchwork.api.base import CurrentPatchDefault from patchwork.api.embedded import PersonSerializer from patchwork.models import Cover @@ -99,7 +108,7 @@ class CoverCommentSerializer(BaseCommentListSerializer): 'comment_id': 'id', }, ) - cover = HiddenField(default=CurrentCoverDefault()) + cover = HiddenField(default=CreateOnlyDefault(CurrentCoverDefault())) class Meta: model = CoverComment @@ -111,6 +120,38 @@ class Meta: extra_kwargs = {'url': {'view_name': 'api-cover-comment-detail'}} +class CoverMaintainerNoteSerializer(CoverCommentSerializer): + content = CharField(required=True) + submitter = PersonSerializer( + read_only=True, default=CreateOnlyDefault(CurrentPersonDefault()) + ) + + def validate(self, attrs): + is_create = self.instance is None + if is_create: + # ReadOnly fields are ignored in create/update operations + submitter_field = self.fields.get('submitter') + attrs['submitter'] = submitter_field.default(submitter_field) + + if self.Meta.model.objects.filter( + cover=attrs['cover'], msgid='' + ).first(): + raise ValidationError( + 'Maintaner note already exists for cover' + ) + + return super().validate(attrs) + + class Meta: + model = CoverComment + fields = CoverCommentSerializer.Meta.fields + read_only_fields = tuple( + f + for f in CoverCommentSerializer.Meta.read_only_fields + if f not in ('content',) + ) + + class CoverCommentMixin(object): permission_classes = (PatchworkPermission,) serializer_class = CoverCommentSerializer @@ -139,7 +180,7 @@ class PatchCommentSerializer(BaseCommentListSerializer): 'comment_id': 'id', }, ) - patch = HiddenField(default=CurrentPatchDefault()) + patch = HiddenField(default=CreateOnlyDefault(CurrentPatchDefault())) class Meta: model = PatchComment @@ -151,6 +192,38 @@ class Meta: extra_kwargs = {'url': {'view_name': 'api-patch-comment-detail'}} +class PatchMaintainerNoteSerializer(PatchCommentSerializer): + content = CharField(required=True) + submitter = PersonSerializer( + read_only=True, default=CreateOnlyDefault(CurrentPersonDefault()) + ) + + def validate(self, attrs): + is_create = self.instance is None + if is_create: + # ReadOnly fields are ignored in create/update operations + submitter_field = self.fields.get('submitter') + attrs['submitter'] = submitter_field.default(submitter_field) + + if self.Meta.model.objects.filter( + patch=attrs['patch'], msgid='' + ).first(): + raise ValidationError( + 'Maintaner note already exists for patch' + ) + + return super().validate(attrs) + + class Meta: + model = PatchComment + fields = PatchCommentSerializer.Meta.fields + read_only_fields = tuple( + f + for f in PatchCommentSerializer.Meta.read_only_fields + if f not in ('content',) + ) + + class PatchCommentMixin(object): permission_classes = (PatchworkPermission,) serializer_class = PatchCommentSerializer @@ -171,55 +244,191 @@ def get_queryset(self): ) -class CoverCommentList(CoverCommentMixin, ListAPIView): - """List cover comments""" +COMMENT_TYPES = ( + ('comments', 'Comments'), + ('notes', 'Maintainer Notes'), +) + + +class CommentsFilter(FilterSet): + type = ChoiceFilter( + label='Type', + field_name='msgid', + choices=COMMENT_TYPES, + method='filter_choice', + ) + + def filter_choice(self, queryset, name, value): + if name == 'msgid' and value == 'notes': + return queryset.filter(msgid='') + + return queryset.exclude(msgid='') + + class Meta: + fields = ['msgid'] + + +class MaintainerNoteMixin(object): + """ + Only maintainers can create objects and when creating or updating a note + use the correct serializer. + + Override `maintainer_note_serializer_class` with the serializer that must + be used and `project_lookup_attr` with the validated data key that holds a + relationship with the object project. + """ + + maintainer_note_serializer_class = None + project_lookup_attr = '' + filterset_class = CommentsFilter + _ERROR_MSG_MAP = {'DELETE': 'delete', 'POST': 'create'} + + def get_queryset(self): + """ + We always remove maintainer notes from the queryset, unless specified + by the filter type. + """ + qs = super(MaintainerNoteMixin, self).get_queryset() + if ( + self.request.method in SAFE_METHODS + and not self.request.query_params.get('type') + ): + return qs.exclude(msgid='') + return qs + + def get_serializer_class(self): + assert self.maintainer_note_serializer_class is not None + + if self.request.method in ('PUT', 'PATCH'): + obj = self.get_object() + if obj and obj.is_maintainer_note: + return self.maintainer_note_serializer_class + + if self.request.method == 'POST': + return self.maintainer_note_serializer_class + + return super(MaintainerNoteMixin, self).get_serializer_class() + + def _check_notes_permission(self, instance, user): + assert self.project_lookup_attr != '' + try: + project = instance[self.project_lookup_attr].project + msgid = instance.get('msgid', '') + except (TypeError, KeyError): + project = getattr(instance, self.project_lookup_attr).project + msgid = instance.msgid + + action_msg = self._ERROR_MSG_MAP[self.request.method] + if not user or not user.is_authenticated: + raise NotAuthenticated( + f'You must be authenticated to {action_msg} a maintainer note' + ) + + if msgid: + raise PermissionDenied( + f'Only maintainer notes can be {action_msg}d' + ) + if not project.is_editable(user): + raise PermissionDenied( + f'You must be a maintainer to {action_msg} a note' + ) + + def perform_create(self, serializer): + self._check_notes_permission( + serializer.validated_data, self.request.user + ) + return super(MaintainerNoteMixin, self).perform_create(serializer) + + def perform_destroy(self, instance): + self._check_notes_permission(instance, self.request.user) + return super(MaintainerNoteMixin, self).perform_destroy(instance) + + +class CoverCommentList( + MaintainerNoteMixin, CoverCommentMixin, ListCreateAPIView +): + """ + get: + List cover comments + + post: + Create a maintainer note in the cover + """ search_fields = ('subject',) ordering_fields = ('id', 'subject', 'date', 'submitter') ordering = 'id' lookup_url_kwarg = 'cover_id' + maintainer_note_serializer_class = CoverMaintainerNoteSerializer + project_lookup_attr = 'cover' class CoverCommentDetail( - CoverCommentMixin, MultipleFieldLookupMixin, RetrieveUpdateAPIView + MaintainerNoteMixin, + CoverCommentMixin, + MultipleFieldLookupMixin, + RetrieveUpdateDestroyAPIView, ): """ get: Show a cover comment. patch: - Update a cover comment. + Update a cover comment or maintainer note. put: - Update a cover comment. + Update a cover comment or maintainer note. + + delete: + Delete a maintainer note. """ lookup_url_kwargs = ('cover_id', 'comment_id') lookup_fields = ('cover_id', 'id') + maintainer_note_serializer_class = CoverMaintainerNoteSerializer + project_lookup_attr = 'cover' -class PatchCommentList(PatchCommentMixin, ListAPIView): - """List patch comments""" +class PatchCommentList( + MaintainerNoteMixin, PatchCommentMixin, ListCreateAPIView +): + """ + get: + List patch comments + + post: + Create a maintainer note in the patch + """ search_fields = ('subject',) ordering_fields = ('id', 'subject', 'date', 'submitter') ordering = 'id' lookup_url_kwarg = 'patch_id' + maintainer_note_serializer_class = PatchMaintainerNoteSerializer + project_lookup_attr = 'patch' class PatchCommentDetail( - PatchCommentMixin, MultipleFieldLookupMixin, RetrieveUpdateAPIView + MaintainerNoteMixin, + PatchCommentMixin, + MultipleFieldLookupMixin, + RetrieveUpdateDestroyAPIView, ): """ get: Show a patch comment. patch: - Update a patch comment. + Update a patch comment or maintainer note. put: - Update a patch comment. + Update a patch comment or maintainer note. + + delete: + Delete a maintainer note. """ lookup_url_kwargs = ('patch_id', 'comment_id') lookup_fields = ('patch_id', 'id') + maintainer_note_serializer_class = PatchMaintainerNoteSerializer + project_lookup_attr = 'patch' diff --git a/patchwork/forms.py b/patchwork/forms.py index cf77bdcc..8b943a8c 100644 --- a/patchwork/forms.py +++ b/patchwork/forms.py @@ -12,6 +12,7 @@ from django.template.backends import django as django_template_backend from patchwork.models import Bundle +from patchwork.models import PatchComment from patchwork.models import Patch from patchwork.models import State from patchwork.models import UserProfile @@ -106,6 +107,19 @@ class DeleteBundleForm(forms.Form): bundle_id = forms.IntegerField(widget=forms.HiddenInput) +class PatchMaintainerNoteForm(forms.ModelForm): + name = 'patchnoteform' + form_name = forms.CharField(initial=name, widget=forms.HiddenInput) + content = forms.CharField(label='Content', widget=forms.Textarea) + addressed = forms.BooleanField( + label='Addressed', widget=forms.CheckboxInput, required=False + ) + + class Meta: + model = PatchComment + fields = ['content', 'addressed'] + + class EmailForm(forms.Form): email = forms.EmailField(max_length=200) diff --git a/patchwork/models.py b/patchwork/models.py index ae2f4a6d..bb7da160 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -360,6 +360,10 @@ class EmailMixin(models.Model): re.M | re.I, ) + @property + def is_maintainer_note(self): + return self.msgid == '' + @property def patch_responses(self): if not self.content: diff --git a/patchwork/templates/patchwork/partials/comment.html b/patchwork/templates/patchwork/partials/comment.html new file mode 100644 index 00000000..ed01122a --- /dev/null +++ b/patchwork/templates/patchwork/partials/comment.html @@ -0,0 +1,66 @@ +{% load person %} +{% load utils %} +{% load syntax %} + +{% is_editable submission user as editable %} +{% is_editable item user as comment_is_editable %} + +
+
+ {{ item.submitter|personify:project }} + {{ item.date }} UTC | + {% if not item.msgid %} + # Maintainer Note + {% else %} + #{{ forloop.counter}} + {% endif %} + +{% if item.addressed == None %} +
+{% else %} + +{% if item.addressed == True %} +
+{% else %} + +{% if item.addressed == False %} +
+{% else %} + +
+
+{{ item|commentsyntax }}
+  
+
diff --git a/patchwork/templates/patchwork/partials/patch-forms.html b/patchwork/templates/patchwork/partials/patch-forms.html index 80f82815..44b6f1ae 100644 --- a/patchwork/templates/patchwork/partials/patch-forms.html +++ b/patchwork/templates/patchwork/partials/patch-forms.html @@ -44,6 +44,22 @@
{% endif %} + {% if is_maintainer %} +
+ {% if not note %} + + {% else %} + + + {% endif %} +
+ {% endif %}
{% endif %}
diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html index cd74491c..1aa78181 100644 --- a/patchwork/templates/patchwork/submission.html +++ b/patchwork/templates/patchwork/submission.html @@ -136,6 +136,25 @@

{{ submission.name }}

{% csrf_token %} {% include "patchwork/partials/patch-forms.html" %} +{% if create_note_form %} +
+

New Note

+ {% csrf_token %} + {{ create_note_form.as_div }} + +
+{% endif %} +{% if edit_note_form %} +
+

Edit Note

+ {% csrf_token %} + {{ edit_note_form.as_div }} +
+ + +
+
+{% endif %} {% if submission.pull_url %}

Pull-request

@@ -187,68 +206,15 @@

Message

-{% for item in comments %} -{% if forloop.first %} +{% if note or comments %}

Comments

+{% if note %} + {% include "patchwork/partials/comment.html" with item=note %} {% endif %} -{% is_editable item user as comment_is_editable %} - -
-
- {{ item.submitter|personify:project }} - {{ item.date }} UTC | - #{{ forloop.counter }} - -{% if item.addressed == None %} -
-{% else %} - -{% if item.addressed == True %} -
-{% else %} - -{% if item.addressed == False %} -
-{% else %} - -
-
-{{ item|commentsyntax }}
-  
-
+{% for item in comments %} +{% include "patchwork/partials/comment.html" %} {% endfor %} +{% endif %} {% if submission.diff %}
diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py index 3487bf4a..27f3a2b3 100644 --- a/patchwork/tests/api/test_comment.py +++ b/patchwork/tests/api/test_comment.py @@ -241,19 +241,6 @@ def test_update_invalid_addressed(self): getattr(CoverComment.objects.all().first(), 'addressed') ) - def test_create_delete(self): - """Ensure creates and deletes aren't allowed""" - comment = create_cover_comment(cover=self.cover) - self.user.is_superuser = True - self.user.save() - self.client.authenticate(user=self.user) - - resp = self.client.post(self.api_url(self.cover, item=comment)) - self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - - resp = self.client.delete(self.api_url(self.cover, item=comment)) - self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - @override_settings(ENABLE_REST_API=True) class TestPatchComments(utils.APITestCase): @@ -480,16 +467,3 @@ def test_update_invalid_addressed(self): self.assertFalse( getattr(PatchComment.objects.all().first(), 'addressed') ) - - def test_create_delete(self): - """Ensure creates and deletes aren't allowed""" - comment = create_patch_comment(patch=self.patch) - self.user.is_superuser = True - self.user.save() - self.client.authenticate(user=self.user) - - resp = self.client.post(self.api_url(self.patch, item=comment)) - self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - - resp = self.client.delete(self.api_url(self.patch, item=comment)) - self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) diff --git a/patchwork/tests/api/test_maintainer_notes.py b/patchwork/tests/api/test_maintainer_notes.py new file mode 100644 index 00000000..e144ed97 --- /dev/null +++ b/patchwork/tests/api/test_maintainer_notes.py @@ -0,0 +1,786 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2025 ProFUSION +# +# SPDX-License-Identifier: GPL-2.0-or-later + +from django.test import override_settings +from django.urls import reverse +from rest_framework import status + +from patchwork.models import PatchComment +from patchwork.models import CoverComment +from patchwork.tests.api import utils +from patchwork.tests.utils import create_cover +from patchwork.tests.utils import create_cover_comment +from patchwork.tests.utils import create_patch +from patchwork.tests.utils import create_patch_comment +from patchwork.tests.utils import create_maintainer +from patchwork.tests.utils import create_project +from patchwork.tests.utils import create_user + + +@override_settings(ENABLE_REST_API=True) +class TestPatchMaintainerNotes(utils.APITestCase): + @staticmethod + def api_url(patch, version=None, item=None): + kwargs = {'patch_id': patch.id} + if version: + kwargs['version'] = version + if item is None: + return reverse('api-patch-comment-list', kwargs=kwargs) + kwargs['comment_id'] = item.id + return reverse('api-patch-comment-detail', kwargs=kwargs) + + def setUp(self): + super(TestPatchMaintainerNotes, self).setUp() + self.project = create_project() + self.maintainer = create_maintainer(self.project) + self.maintainer_person = self.maintainer.person_set.first() + self.patch = create_patch(project=self.project) + + # Another maintainer for testing editing other's notes + self.other_maintainer = create_maintainer( + self.project, + username='other_maintainer', + email='other@example.com', + ) + self.other_person = self.other_maintainer.person_set.first() + + def assertMaintainerNote(self, note_obj, note_json): + """Assert a maintainer note is correctly serialized.""" + self.assertEqual(note_obj.id, note_json['id']) + self.assertEqual(note_obj.submitter.id, note_json['submitter']['id']) + self.assertEqual('', note_obj.msgid) + self.assertIn(note_obj.content, note_json['content']) + + def test_create_maintainer_note(self): + """Test creating a maintainer note for a patch.""" + self.client.authenticate(user=self.maintainer) + + data = {'content': 'This is a maintainer note', 'addressed': None} + resp = self.client.post(self.api_url(self.patch), data) + + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + self.assertEqual(1, PatchComment.objects.filter(msgid='').count()) + + note = PatchComment.objects.get(msgid='') + self.assertEqual(data['content'], note.content) + self.assertEqual(self.maintainer_person, note.submitter) + self.assertMaintainerNote(note, resp.data) + + def test_create_note_non_maintainer(self): + """Test that a non-maintainer cannot create a maintainer note.""" + user = create_user() + self.client.authenticate(user=user) + + resp = self.client.post( + self.api_url(self.patch), {'content': 'This should fail'} + ) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertEqual(0, PatchComment.objects.filter(msgid='').count()) + + def test_duplicate_maintainer_note(self): + """Test that we can't create duplicate maintainer notes for a patch.""" + self.client.authenticate(user=self.maintainer) + + resp = self.client.post( + self.api_url(self.patch), {'content': 'First maintainer note'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + + resp = self.client.post( + self.api_url(self.patch), + {'content': 'Second maintainer note - should fail'}, + ) + + self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + self.assertIn('Maintaner note already exists', str(resp.data)) + + def test_update_own_maintainer_note(self): + """Test updating a maintainer note as the original creator.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'Original content'} + ) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + updated_content = 'Updated content' + resp = self.client.patch( + self.api_url(self.patch, item=note), {'content': updated_content} + ) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + note.refresh_from_db() + self.assertEqual(updated_content, note.content) + self.assertEqual(updated_content, resp.data['content']) + + def test_update_others_maintainer_note(self): + """Test that a maintainer can update another maintainer's note.""" + # First maintainer creates a note + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), + {'content': 'Original content from first maintainer'}, + ) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + # Second maintainer updates it + self.client.authenticate(user=self.other_maintainer) + updated_content = 'Updated by second maintainer' + resp = self.client.patch( + self.api_url(self.patch, item=note), {'content': updated_content} + ) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + note.refresh_from_db() + self.assertEqual(updated_content, note.content) + self.assertEqual(updated_content, resp.data['content']) + + # Submitter should not change + self.assertEqual( + self.maintainer_person.id, resp.data['submitter']['id'] + ) + self.assertEqual(self.maintainer_person, note.submitter) + + def test_update_note_non_maintainer(self): + """Test that a non-maintainer cannot update a maintainer note.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'Original content'} + ) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + user = create_user() + self.client.authenticate(user=user) + resp = self.client.patch( + self.api_url(self.patch, item=note), + {'content': 'This should fail'}, + ) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + note.refresh_from_db() + self.assertEqual('Original content', note.content) + + def test_list_includes_maintainer_notes_with_filter(self): + """Test that maintainer notes are included when using the type filter.""" + create_patch_comment(patch=self.patch) + + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'Maintainer note'} + ) + note_id = resp.data['id'] + + # Get list of comments with type=notes filter + resp = self.client.get(self.api_url(self.patch), {'type': 'notes'}) + + # Should include only the note + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(note_id, resp.data[0]['id']) + self.assertEqual('Maintainer note', resp.data[0]['content']) + + def test_delete_maintainer_note(self): + """Test deleting a maintainer note as a maintainer.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'Note to be deleted'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + resp = self.client.delete(self.api_url(self.patch, item=note)) + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) + self.assertEqual(0, PatchComment.objects.filter(id=note_id).count()) + + def test_delete_maintainer_note_by_other_maintainer(self): + """Test that another maintainer can delete a maintainer note.""" + # First maintainer creates a note + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), + {'content': 'Note created by first maintainer'}, + ) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + # Second maintainer deletes it + self.client.authenticate(user=self.other_maintainer) + resp = self.client.delete(self.api_url(self.patch, item=note)) + + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) + self.assertEqual(0, PatchComment.objects.filter(id=note_id).count()) + + def test_delete_maintainer_note_by_non_maintainer(self): + """Test that a non-maintainer cannot delete a maintainer note.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), + {'content': 'Note that should not be deletable'}, + ) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + user = create_user() + self.client.authenticate(user=user) + resp = self.client.delete(self.api_url(self.patch, item=note)) + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertTrue(PatchComment.objects.filter(id=note_id).exists()) + + def test_delete_regular_comment_fails(self): + """Test that regular comments cannot be deleted via the API.""" + comment = create_patch_comment( + patch=self.patch, content='Regular comment' + ) + + self.client.authenticate(user=self.maintainer) + resp = self.client.delete(self.api_url(self.patch, item=comment)) + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertIn('Only maintainer notes can be deleted', str(resp.data)) + + def test_filter_by_type_notes(self): + """Test filtering comments to show only maintainer notes.""" + create_patch_comment(patch=self.patch, content='Regular comment') + + self.client.authenticate(user=self.maintainer) + note_resp = self.client.post( + self.api_url(self.patch), + {'content': 'Maintainer note for filtering'}, + ) + note_id = note_resp.data['id'] + + # Filter to only show notes + resp = self.client.get( + self.api_url(self.patch), + {'type': 'notes'}, + ) + + # Should include only the note, not the regular comment + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(note_id, resp.data[0]['id']) + self.assertEqual( + 'Maintainer note for filtering', resp.data[0]['content'] + ) + self.assertEqual('', resp.data[0]['msgid']) + + def test_filter_by_type_comments(self): + """Test filtering to show only regular comments (exclude notes).""" + regular_comment = create_patch_comment( + patch=self.patch, content='Regular comment for filtering' + ) + + self.client.authenticate(user=self.maintainer) + self.client.post( + self.api_url(self.patch), + {'content': 'Maintainer note'}, + ) + + # Filter to only show regular comments + resp = self.client.get(self.api_url(self.patch), {'type': 'comments'}) + + # Should include only the regular comment, not the note + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(regular_comment.id, resp.data[0]['id']) + self.assertEqual( + 'Regular comment for filtering', resp.data[0]['content'] + ) + self.assertNotEqual('', resp.data[0]['msgid']) + + def test_authentication_required_for_delete(self): + """Test that authentication is required for DELETE operations.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'Note to be deleted'} + ) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + self.client.authenticate(user=None) + resp = self.client.delete(self.api_url(self.patch, item=note)) + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertIn('permission_denied', str(resp.data).lower()) + + def test_get_list_default_excludes_notes(self): + """Test that the default GET behavior excludes maintainer notes.""" + regular_comment = create_patch_comment( + patch=self.patch, content='Regular comment' + ) + + self.client.authenticate(user=self.maintainer) + self.client.post( + self.api_url(self.patch), + {'content': 'Hidden maintainer note'}, + ) + + self.client.authenticate(user=None) # No auth needed for GET + resp = self.client.get(self.api_url(self.patch)) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(regular_comment.id, resp.data[0]['id']) + + def test_non_maintainer_can_access_filtered_notes(self): + """Test that non-maintainers can access notes with the filter.""" + self.client.authenticate(user=self.maintainer) + note_resp = self.client.post( + self.api_url(self.patch), {'content': 'Maintainer note'} + ) + note_id = note_resp.data['id'] + + user = create_user() + self.client.authenticate(user=user) + resp = self.client.get(self.api_url(self.patch), {'type': 'notes'}) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(note_id, resp.data[0]['id']) + + def test_no_modification_of_msgid_field(self): + """Test that msgid cannot be explicitly set or modified.""" + self.client.authenticate(user=self.maintainer) + + # Try to create a maintainer note with a non-empty msgid + resp = self.client.post( + self.api_url(self.patch), + {'content': 'Note with msgid', 'msgid': ''}, + ) + + # Should succeed, but msgid should be empty (MessageID ignored in request) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + self.assertEqual('', resp.data['msgid']) + note = PatchComment.objects.get(id=resp.data['id']) + self.assertEqual('', note.msgid) + + def test_maintainer_serializer_used(self): + """Test that the correct serializer is used based on method/model.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'Maintainer note'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + note_id = resp.data['id'] + note = PatchComment.objects.get(id=note_id) + + other_user = create_user(username='test_other') + self.client.authenticate(user=self.maintainer) + + # Try to update submitter field (should be ignored as it's read-only) + resp = self.client.patch( + self.api_url(self.patch, item=note), + {'content': 'Updated note', 'submitter': {'id': other_user.id}}, + ) + + # Should succeed but submitter shouldn't change + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual('Updated note', resp.data['content']) + self.assertEqual( + self.maintainer_person.id, resp.data['submitter']['id'] + ) + + def test_validation_error_has_proper_message(self): + """Test that validation errors have the correct message format.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.patch), {'content': 'First note'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + + resp = self.client.post( + self.api_url(self.patch), {'content': 'Second note - should fail'} + ) + + self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + self.assertIn('Maintaner note already exists', str(resp.data)) + self.assertIn('patch', str(resp.data).lower()) + + +@override_settings(ENABLE_REST_API=True) +class TestCoverMaintainerNotes(utils.APITestCase): + @staticmethod + def api_url(cover, version=None, item=None): + kwargs = {'cover_id': cover.id} + if version: + kwargs['version'] = version + if item is None: + return reverse('api-cover-comment-list', kwargs=kwargs) + kwargs['comment_id'] = item.id + return reverse('api-cover-comment-detail', kwargs=kwargs) + + def setUp(self): + super(TestCoverMaintainerNotes, self).setUp() + self.project = create_project() + self.maintainer = create_maintainer(self.project) + self.maintainer_person = self.maintainer.person_set.first() + self.cover = create_cover(project=self.project) + + # Another maintainer for testing editing other's notes + self.other_maintainer = create_maintainer( + self.project, + username='other_maintainer', + email='other@example.com', + ) + self.other_person = self.other_maintainer.person_set.first() + + def assertMaintainerNote(self, note_obj, note_json): + """Assert a maintainer note is correctly serialized.""" + self.assertEqual(note_obj.id, note_json['id']) + self.assertEqual(note_obj.submitter.id, note_json['submitter']['id']) + self.assertEqual('', note_obj.msgid) + self.assertIn(note_obj.content, note_json['content']) + + def test_create_maintainer_note(self): + """Test creating a maintainer note for a cover letter.""" + self.client.authenticate(user=self.maintainer) + + data = {'content': 'This is a cover maintainer note'} + resp = self.client.post(self.api_url(self.cover), data) + + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + self.assertEqual(1, CoverComment.objects.filter(msgid='').count()) + + note = CoverComment.objects.get(msgid='') + self.assertEqual(data['content'], note.content) + self.assertEqual(self.maintainer_person, note.submitter) + self.assertMaintainerNote(note, resp.data) + + def test_create_note_non_maintainer(self): + """Test that a non-maintainer cannot create a maintainer note.""" + user = create_user() + self.client.authenticate(user=user) + resp = self.client.post( + self.api_url(self.cover), {'content': 'This should fail'} + ) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertEqual(0, CoverComment.objects.filter(msgid='').count()) + + def test_duplicate_maintainer_note(self): + """Test that we can't create duplicate maintainer notes for a cover.""" + self.client.authenticate(user=self.maintainer) + + resp = self.client.post( + self.api_url(self.cover), {'content': 'First maintainer note'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + resp = self.client.post( + self.api_url(self.cover), + {'content': 'Second maintainer note - should fail'}, + ) + + self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + self.assertIn('Maintaner note already exists', str(resp.data)) + self.assertEqual(1, CoverComment.objects.filter(msgid='').count()) + + def test_update_own_maintainer_note(self): + """Test updating a maintainer note as the original creator.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'Original content'} + ) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + updated_content = 'Updated content' + resp = self.client.patch( + self.api_url(self.cover, item=note), {'content': updated_content} + ) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + note.refresh_from_db() + self.assertEqual(updated_content, note.content) + self.assertEqual(updated_content, resp.data['content']) + + def test_update_others_maintainer_note(self): + """Test that a maintainer can update another maintainer's note.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), + {'content': 'Original content from first maintainer'}, + ) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + # Second maintainer updates it + self.client.authenticate(user=self.other_maintainer) + updated_content = 'Updated by second maintainer' + resp = self.client.patch( + self.api_url(self.cover, item=note), {'content': updated_content} + ) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + note.refresh_from_db() + self.assertEqual(updated_content, note.content) + self.assertEqual(updated_content, resp.data['content']) + self.assertEqual( + self.maintainer_person.id, resp.data['submitter']['id'] + ) + self.assertEqual(self.maintainer_person, note.submitter) + + def test_update_note_non_maintainer(self): + """Test that a non-maintainer cannot update a maintainer note.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'Original content'} + ) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + # Try to update as non-maintainer + user = create_user() + self.client.authenticate(user=user) + resp = self.client.patch( + self.api_url(self.cover, item=note), + {'content': 'This should fail'}, + ) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + note.refresh_from_db() + self.assertEqual('Original content', note.content) + + def test_list_includes_maintainer_notes_with_filter(self): + """Test that maintainer notes are included when using the type filter.""" + create_cover_comment(cover=self.cover) + + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'Maintainer note'} + ) + note_id = resp.data['id'] + + resp = self.client.get(self.api_url(self.cover), {'type': 'notes'}) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(note_id, resp.data[0]['id']) + self.assertEqual('Maintainer note', resp.data[0]['content']) + + def test_delete_maintainer_note(self): + """Test deleting a maintainer note as a maintainer.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'Note to be deleted'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + resp = self.client.delete(self.api_url(self.cover, item=note)) + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) + self.assertEqual(0, CoverComment.objects.filter(id=note_id).count()) + + def test_delete_maintainer_note_by_other_maintainer(self): + """Test that another maintainer can delete a maintainer note.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), + {'content': 'Note created by first maintainer'}, + ) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + # Second maintainer deletes it + self.client.authenticate(user=self.other_maintainer) + resp = self.client.delete(self.api_url(self.cover, item=note)) + + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) + self.assertEqual(0, CoverComment.objects.filter(id=note_id).count()) + + def test_delete_maintainer_note_by_non_maintainer(self): + """Test that a non-maintainer cannot delete a maintainer note.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), + {'content': 'Note that should not be deletable'}, + ) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + # Try to delete as non-maintainer + user = create_user() + self.client.authenticate(user=user) + resp = self.client.delete(self.api_url(self.cover, item=note)) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertTrue(CoverComment.objects.filter(id=note_id).exists()) + + def test_delete_regular_comment_fails(self): + """Test that regular comments cannot be deleted via the API.""" + comment = create_cover_comment( + cover=self.cover, content='Regular comment' + ) + + self.client.authenticate(user=self.maintainer) + resp = self.client.delete(self.api_url(self.cover, item=comment)) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertIn('Only maintainer notes can be deleted', str(resp.data)) + + def test_filter_by_type_notes(self): + """Test filtering comments to show only maintainer notes.""" + create_cover_comment(cover=self.cover, content='Regular comment') + + self.client.authenticate(user=self.maintainer) + note_resp = self.client.post( + self.api_url(self.cover), + {'content': 'Maintainer note for filtering'}, + ) + note_id = note_resp.data['id'] + + # Filter to only show notes + resp = self.client.get(self.api_url(self.cover), {'type': 'notes'}) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(note_id, resp.data[0]['id']) + self.assertEqual( + 'Maintainer note for filtering', resp.data[0]['content'] + ) + self.assertEqual('', resp.data[0]['msgid']) + + def test_filter_by_type_comments(self): + """Test filtering to show only regular comments (exclude notes).""" + + regular_comment = create_cover_comment( + cover=self.cover, content='Regular comment for filtering' + ) + + self.client.authenticate(user=self.maintainer) + self.client.post( + self.api_url(self.cover), + {'content': 'Maintainer note'}, + ) + + # Filter to only show regular comments + resp = self.client.get(self.api_url(self.cover), {'type': 'comments'}) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(regular_comment.id, resp.data[0]['id']) + self.assertEqual( + 'Regular comment for filtering', resp.data[0]['content'] + ) + self.assertNotEqual('', resp.data[0]['msgid']) + + def test_authentication_required_for_delete(self): + """Test that authentication is required for DELETE operations.""" + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'Note to be deleted'} + ) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + self.client.authenticate(user=None) + resp = self.client.delete(self.api_url(self.cover, item=note)) + + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + self.assertIn('permission_denied', str(resp.data).lower()) + + def test_get_list_default_excludes_notes(self): + """Test that the default GET behavior excludes maintainer notes.""" + regular_comment = create_cover_comment( + cover=self.cover, content='Regular comment' + ) + + self.client.authenticate(user=self.maintainer) + self.client.post( + self.api_url(self.cover), {'content': 'Hidden maintainer note'} + ) + + self.client.authenticate(user=None) # No auth needed for GET + resp = self.client.get(self.api_url(self.cover)) + + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(regular_comment.id, resp.data[0]['id']) + + def test_non_maintainer_can_access_filtered_notes(self): + """Test that non-maintainers can access notes with the filter.""" + self.client.authenticate(user=self.maintainer) + note_resp = self.client.post( + self.api_url(self.cover), {'content': 'Maintainer note'} + ) + note_id = note_resp.data['id'] + + # Non-maintainer user should still be able to see notes with filter + user = create_user() + self.client.authenticate(user=user) + resp = self.client.get(self.api_url(self.cover), {'type': 'notes'}) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertEqual(note_id, resp.data[0]['id']) + + def test_no_modification_of_msgid_field(self): + """Test that msgid cannot be explicitly set or modified.""" + self.client.authenticate(user=self.maintainer) + + # Try to create a maintainer note with a non-empty msgid + resp = self.client.post( + self.api_url(self.cover), + {'content': 'Note with msgid', 'msgid': ''}, + ) + + # Should succeed, but msgid should be empty (MessageID ignored in request) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + self.assertEqual('', resp.data['msgid']) + + # Verify in database + note = CoverComment.objects.get(id=resp.data['id']) + self.assertEqual('', note.msgid) + + def test_maintainer_serializer_used(self): + """Test that the correct serializer is used based on method/model.""" + # Create a note first + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'Maintainer note'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + note_id = resp.data['id'] + note = CoverComment.objects.get(id=note_id) + + # Check that read-only fields are respected - try to change submitter + other_user = create_user(username='test_other') + self.client.authenticate(user=self.maintainer) + + # Try to update submitter field (should be ignored as it's read-only) + resp = self.client.patch( + self.api_url(self.cover, item=note), + {'content': 'Updated note', 'submitter': {'id': other_user.id}}, + ) + + # Should succeed but submitter shouldn't change + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual('Updated note', resp.data['content']) + self.assertEqual( + self.maintainer_person.id, resp.data['submitter']['id'] + ) + + def test_validation_error_has_proper_message(self): + """Test that validation errors have the correct message format.""" + # Create a maintainer note + self.client.authenticate(user=self.maintainer) + resp = self.client.post( + self.api_url(self.cover), {'content': 'First note'} + ) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + + # Try to create another one + resp = self.client.post( + self.api_url(self.cover), {'content': 'Second note - should fail'} + ) + + # Should fail with proper error message + self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) + self.assertIn('Maintaner note already exists', str(resp.data)) + self.assertIn( + 'cover', str(resp.data).lower() + ) # Mentions it's for a cover diff --git a/patchwork/tests/views/test_patch.py b/patchwork/tests/views/test_patch.py index 3de558f0..01a328a2 100644 --- a/patchwork/tests/views/test_patch.py +++ b/patchwork/tests/views/test_patch.py @@ -247,6 +247,70 @@ def test_comment_redirect(self): response = self.client.get(requested_url) self.assertRedirects(response, redirect_url) + def test_show_maintainer_note(self): + project = create_project() + user_default = create_user(project) + user_maintainer = create_maintainer(project) + patch = create_patch(project=project) + note = create_patch_comment(patch=patch, msgid='') + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.encoded_msgid, + }, + ) + + # No authentication + response = self.client.get(requested_url) + self.assertNotIn('# Maintainer Note'.encode('utf-8'), response.content) + self.assertNotIn(note.content.encode('utf-8'), response.content) + + # Auth with default user + self.client.login(username=user_default.username, password=user_default.username) + response = self.client.get(requested_url) + self.assertNotIn('# Maintainer Note'.encode('utf-8'), response.content) + self.assertNotIn(note.content.encode('utf-8'), response.content) + + # Auth with maintainer user + self.client.login(username=user_maintainer.username, password=user_maintainer.username) + response = self.client.get(requested_url) + self.assertIn('# Maintainer Note'.encode('utf-8'), response.content) + self.assertIn(note.content.encode('utf-8'), response.content) + + def test_maintainer_notes_controls(self): + project = create_project() + user = create_maintainer(project) + patch = create_patch(project=project) + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.encoded_msgid, + }, + ) + + # No authentication + response = self.client.get(requested_url) + self.assertNotIn('Add Note'.encode('utf-8'), response.content) + self.assertNotIn('Edit Note'.encode('utf-8'), response.content) + self.assertNotIn('Remove Note'.encode('utf-8'), response.content) + + # Auth with no note + self.client.login(username=user.username, password=user.username) + response = self.client.get(requested_url) + self.assertIn('Add Note'.encode('utf-8'), response.content) + self.assertNotIn('Edit Note'.encode('utf-8'), response.content) + self.assertNotIn('Remove Note'.encode('utf-8'), response.content) + + # Auth with note + note = create_patch_comment(patch=patch, msgid='') + response = self.client.get(requested_url) + self.assertNotIn('Add Note'.encode('utf-8'), response.content) + self.assertIn('Edit Note'.encode('utf-8'), response.content) + self.assertIn('Remove Note'.encode('utf-8'), response.content) + self.assertIn(note.content.encode('utf-8'), response.content) + def test_old_detail_url(self): patch = create_patch() diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py index 15013a89..3fc5c8a8 100644 --- a/patchwork/views/cover.py +++ b/patchwork/views/cover.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: GPL-2.0-or-later +from django.contrib import messages from django.http import Http404 from django.http import HttpResponse from django.http import HttpResponseRedirect @@ -10,7 +11,8 @@ from django.shortcuts import render from django.urls import reverse -from patchwork.models import Cover +from patchwork.forms import PatchMaintainerNoteForm +from patchwork.models import Cover, CoverComment from patchwork.models import Patch from patchwork.models import Project from patchwork.views.utils import cover_to_mbox @@ -42,14 +44,94 @@ def cover_detail(request, project_id, msgid): 'project': cover.project, } - comments = cover.comments.all() + comments = cover.comments.exclude(msgid='').all() comments = comments.select_related('submitter') comments = comments.only('submitter', 'date', 'id', 'content', 'cover') + is_maintainer = ( + request.user.is_superuser + or request.user.is_authenticated + and request.user.person_set.all().first() + and cover.project in request.user.profile.maintainer_projects.all() + ) + + note, create_note_form, edit_note_form, errors = ( + handle_post_maintainer_note(cover, request) + ) + + if is_maintainer: + context['note'] = note + context['comments'] = comments + context['is_maintainer'] = is_maintainer + context['create_note_form'] = create_note_form + context['edit_note_form'] = edit_note_form + if errors: + context['errors'] = errors return render(request, 'patchwork/submission.html', context) +def handle_post_maintainer_note(cover, request): + note = cover.comments.filter(msgid='').all().first() + create_note_form = None + edit_note_form = None + errors = None + form_name = request.POST.get('form_name', '') + is_maintainer = ( + request.user.is_superuser + or request.user.is_authenticated + and request.user.person_set.all().first() + and cover.project in request.user.profile.maintainer_projects.all() + ) + + if request.method != 'POST' or not is_maintainer: + return note, create_note_form, edit_note_form, errors + + if form_name == PatchMaintainerNoteForm.name: + edit_cancel = bool(request.POST.get('cancel', False)) + if edit_cancel: + edit_note_form = None + elif note: + edit_note_form = PatchMaintainerNoteForm( + request.POST, instance=note + ) + if edit_note_form.is_valid(): + note = edit_note_form.save() + messages.success(request, 'Note updated') + edit_note_form = None + else: + errors = edit_note_form.err + + else: + create_note = PatchMaintainerNoteForm( + request.POST, + instance=CoverComment( + cover=cover, + submitter=request.user.person_set.all().first(), + ), + ) + if create_note.is_valid(): + note = create_note.save() + messages.success(request, 'Note created') + else: + errors = create_note.err + create_note_form = None + + action = request.POST.get('action', None) + if action: + action = action.lower() + if action == 'note:add': + create_note_form = PatchMaintainerNoteForm() + elif action == 'note:edit': + edit_note_form = PatchMaintainerNoteForm(instance=note) + elif action == 'note:remove': + note.delete() + messages.success(request, 'Note removed') + note = None + + return note, create_note_form, edit_note_form, errors + + def cover_mbox(request, project_id, msgid): db_msgid = '<%s>' % msgid project = get_object_or_404(Project, linkname=project_id) diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index efe94f17..6ed09f4e 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -13,8 +13,10 @@ from django.urls import reverse from patchwork.forms import CreateBundleForm +from patchwork.forms import PatchMaintainerNoteForm from patchwork.forms import PatchForm from patchwork.models import Cover +from patchwork.models import PatchComment from patchwork.models import Patch from patchwork.models import Project from patchwork.views import generic_list @@ -62,9 +64,18 @@ def patch_detail(request, project_id, msgid): editable = patch.is_editable(request.user) context = {'project': patch.project} + note = patch.comments.filter(msgid='').all().first() form = None create_bundle_form = None + create_note_form = None + edit_note_form = None errors = None + is_maintainer = ( + request.user.is_superuser + or request.user.is_authenticated + and request.user.person_set.all().first() + and patch.project in request.user.profile.maintainer_projects.all() + ) if editable: form = PatchForm(instance=patch) @@ -72,28 +83,69 @@ def patch_detail(request, project_id, msgid): create_bundle_form = CreateBundleForm() if request.method == 'POST': - action = request.POST.get('action', None) - if action: - action = action.lower() - - if action in ['create', 'add']: - errors = set_bundle( - request, project, action, request.POST, [patch] - ) + form_name = request.POST.get('form_name', '') + if is_maintainer and form_name == PatchMaintainerNoteForm.name: + edit_cancel = bool(request.POST.get('cancel', False)) + if edit_cancel: + edit_note_form = None + elif note: + edit_note_form = PatchMaintainerNoteForm( + request.POST, instance=note + ) + if edit_note_form.is_valid(): + edit_note_form.save() + messages.success(request, 'Note updated') + edit_note_form = None + else: + errors = edit_note_form.err + + else: + create_note = PatchMaintainerNoteForm( + request.POST, + instance=PatchComment( + patch=patch, + submitter=request.user.person_set.all().first(), + ), + ) + if create_note.is_valid(): + note = create_note.save() + messages.success(request, 'Note created') + else: + errors = create_note.err + create_note_form = None + + else: + action = request.POST.get('action', None) + if action: + action = action.lower() + + if is_maintainer and action == 'note:add': + create_note_form = PatchMaintainerNoteForm() + elif is_maintainer and action == 'note:edit': + edit_note_form = PatchMaintainerNoteForm(instance=note) + elif is_maintainer and action == 'note:remove': + note.delete() + messages.success(request, 'Note removed') + note = None + + elif action in ['create', 'add']: + errors = set_bundle( + request, project, action, request.POST, [patch] + ) - elif not editable: - return HttpResponseForbidden() + elif not editable: + return HttpResponseForbidden() - elif action == 'update': - form = PatchForm(data=request.POST, instance=patch) - if form.is_valid(): - form.save() - messages.success(request, 'Patch updated') + elif action == 'update': + form = PatchForm(data=request.POST, instance=patch) + if form.is_valid(): + form.save() + messages.success(request, 'Patch updated') if request.user.is_authenticated: context['bundles'] = request.user.bundles.all() - comments = patch.comments.all() + comments = patch.comments.exclude(msgid='').all() comments = comments.select_related('submitter') comments = comments.only( 'submitter', 'date', 'id', 'content', 'patch', 'addressed' @@ -113,12 +165,18 @@ def patch_detail(request, project_id, msgid): related_same_project = [] related_different_project = [] + if is_maintainer: + context['note'] = note + context['comments'] = comments context['checks'] = Patch.filter_unique_checks( patch.check_set.all().select_related('user'), ) context['submission'] = patch context['editable'] = editable + context['is_maintainer'] = is_maintainer + context['create_note_form'] = create_note_form + context['edit_note_form'] = edit_note_form context['patch_form'] = form context['create_bundle_form'] = create_bundle_form context['project'] = patch.project