diff --git a/reviewboard/attachments/models.py b/reviewboard/attachments/models.py index 54a6056e138abe24f0fb91dc8d92c391f8d57c01..bbc9c229089ac63ede545cd39d3ab0f80c599d2c 100644 --- a/reviewboard/attachments/models.py +++ b/reviewboard/attachments/models.py @@ -1,10 +1,14 @@ +from __future__ import annotations + import logging import os +from typing import Optional, TYPE_CHECKING from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.db.models import Max +from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from djblets.db.fields import JSONField, RelationCounterField @@ -15,6 +19,9 @@ from reviewboard.diffviewer.models import FileDiff from reviewboard.scmtools.models import Repository from reviewboard.site.models import LocalSite +if TYPE_CHECKING: + from reviewboard.reviews.models import ReviewRequest, ReviewRequestDraft + logger = logging.getLogger(__name__) @@ -215,6 +222,37 @@ class FileAttachment(models.Model): return (self.repository_id is not None or self.added_in_filediff_id is not None) + # @cached_property + # def is_draft(self) -> bool: + # """Whether the file attachment is a draft. + + # Version Added: + # 6.0 + + # Type: + # bool + # """ + # if hasattr(self, '_is_draft'): + # print('in is_draft: returning self._is_draft which is', self._is_draft) + # return self._is_draft + + # self._is_draft = (not self.user and (self.drafts.exists() or + # self.inactive_drafts.exists())) + # print('in is_draft: returning the query which is', self._is_draft) + # return self._is_draft + + @cached_property + def is_draft(self) -> bool: + """Whether the file attachment is a draft. + + Version Added: + 6.0 + + Type: + bool + """ + return bool(not self.user and self.get_review_request_draft()) + @property def num_revisions(self): """Return the number of revisions of this attachment.""" @@ -225,24 +263,78 @@ class FileAttachment(models.Model): """Return a string representation of this file for the admin list.""" return self.caption - def get_review_request(self): + # def get_review_request(self) -> ReviewRequest: + # """Return the ReviewRequest that this file is attached to.""" + # if hasattr(self, '_review_request'): + # return self._review_request + + # review_request = None + + # try: + # review_request = self.review_request.all()[0] + # self._is_draft = False + # except IndexError: + # try: + # review_request = self.inactive_review_request.all()[0] + # self._is_draft = False + # except IndexError: + # # Maybe it's on a draft. + # try: + # draft = self.drafts.get() + # except ObjectDoesNotExist: + # draft = self.inactive_drafts.get() + + # self._is_draft = True + # review_request = draft.review_request + # finally: + # assert review_request is not None + # return review_request + + def get_review_request(self) -> ReviewRequest: """Return the ReviewRequest that this file is attached to.""" if hasattr(self, '_review_request'): return self._review_request + review_request = None + try: - return self.review_request.all()[0] + review_request = self.review_request.all()[0] + self._review_request_draft = None except IndexError: try: - return self.inactive_review_request.all()[0] + review_request = self.inactive_review_request.all()[0] + self._review_request_draft = None except IndexError: # Maybe it's on a draft. - try: - draft = self.drafts.get() - except ObjectDoesNotExist: - draft = self.inactive_drafts.get() + draft = self.get_review_request_draft() + review_request = draft.review_request + finally: + assert review_request is not None + return review_request + + def get_review_request_draft(self) -> Optional[ReviewRequestDraft]: + """Return the ReviewRequestDraft that this file is attached to. + + If the file attachment is not part of a review request draft, this + will return ``None``. - return draft.review_request + Returns: + reviewboard.reviews.models.ReviewRequestDraft: + The ReviewRequestDraft that this file is attached to. + """ + if hasattr(self, '_review_request_draft'): + print('RETURNING self._review_request_draft') + return self._review_request_draft + + draft = None + + try: + draft = self.drafts.get() + except ObjectDoesNotExist: + draft = self.inactive_drafts.get() + finally: + self._review_request_draft = draft + return draft def get_comments(self): """Return all the comments made on this file attachment.""" diff --git a/reviewboard/attachments/tests.py b/reviewboard/attachments/tests.py index 7a598ad5ef047a8f82aa0ec34999d5e54d6090e9..2721b4b5f3532a0d2c110589f11ff4a3f86c8aff 100644 --- a/reviewboard/attachments/tests.py +++ b/reviewboard/attachments/tests.py @@ -7,6 +7,7 @@ from django.conf import settings from django.contrib.auth.models import AnonymousUser, User from django.core.cache import cache from django.core.files.uploadedfile import SimpleUploadedFile +from django.db.models import Q from django.utils.safestring import SafeText from djblets.testing.decorators import add_fixtures from kgb import SpyAgency @@ -19,6 +20,7 @@ from reviewboard.attachments.mimetypes import (MimetypeHandler, from reviewboard.attachments.models import (FileAttachment, FileAttachmentHistory) from reviewboard.diffviewer.models import DiffSet, DiffSetHistory, FileDiff +from reviewboard.reviews.models import ReviewRequestDraft from reviewboard.scmtools.core import PRE_CREATION from reviewboard.site.models import LocalSite from reviewboard.testing import TestCase @@ -72,6 +74,200 @@ class BaseFileAttachmentTestCase(TestCase): class FileAttachmentTests(BaseFileAttachmentTestCase): """Tests for the FileAttachment model.""" + @add_fixtures(['test_users', 'test_scmtools']) + def test_is_draft_with_draft(self) -> None: + """Testing FileAttachment.is_draft with a draft file attachment""" + review_request = self.create_review_request(publish=True) + file_attachment = self.create_file_attachment( + review_request=review_request, + draft=True) + + # 1 query: + # + # 1. Fetch review request draft + queries = [ + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_file_attachments', + }, + 'where': ( + Q(file_attachments__id=file_attachment.pk) + ), + }, + ] + + with self.assertQueries(queries): + self.assertTrue(file_attachment.is_draft) + + # The property should be cached now. + with self.assertNumQueries(0): + self.assertTrue(file_attachment.is_draft) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_is_draft_with_draft_inactive(self) -> None: + """Testing FileAttachment.is_draft with a draft file attachment""" + review_request = self.create_review_request(publish=True) + file_attachment = self.create_file_attachment( + review_request=review_request, + draft=True, + active=False) + + # 2 queries: + # + # 1. Fetch review request draft + # 2. Fetch inactive review request draft + queries = [ + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_file_attachments', + }, + 'where': ( + Q(file_attachments__id=file_attachment.pk) + ), + }, + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_inactive_file_attachments', + }, + 'where': ( + Q(inactive_file_attachments__id=file_attachment.pk) + ), + }, + ] + + with self.assertQueries(queries): + self.assertTrue(file_attachment.is_draft) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_is_draft_with_draft_and_review_request_loaded(self) -> None: + """Testing FileAttachment.is_draft with a draft file attachment + when the review request has been loaded""" + review_request = self.create_review_request(publish=True) + file_attachment = self.create_file_attachment( + review_request=review_request, + draft=True) + + file_attachment.get_review_request() + + # file_attachment._review_request_draft will be set after calling + # get_review_request(), so no queries. + with self.assertNumQueries(0): + self.assertTrue(file_attachment.is_draft) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_is_draft_with_published(self) -> None: + """Testing FileAttachment.is_draft with a published file attachment""" + review_request = self.create_review_request(publish=True) + file_attachment = self.create_file_attachment( + review_request=review_request) + + # 2 queries: + # + # 1. Fetch review request draft + # 2. Fetch inactive review request draft + queries = [ + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_file_attachments', + }, + 'where': ( + Q(file_attachments__id=file_attachment.pk) + ), + }, + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_inactive_file_attachments', + }, + 'where': ( + Q(inactive_file_attachments__id=file_attachment.pk) + ), + }, + ] + + with self.assertQueries(queries): + self.assertFalse(file_attachment.is_draft) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_is_draft_with_published_and_review_request_loaded(self) -> None: + """Testing FileAttachment.is_draft with a published file attachment + when the review request has been loaded""" + review_request = self.create_review_request(publish=True) + file_attachment = self.create_file_attachment( + review_request=review_request) + + file_attachment.get_review_request() + + # file_attachment._review_request_draft will be set after calling + # get_review_request(), so no queries. + with self.assertNumQueries(0): + self.assertFalse(file_attachment.is_draft) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_is_draft_with_published_inactive(self) -> None: + """Testing FileAttachment.is_draft with a published inactive file + attachment + """ + review_request = self.create_review_request(publish=True) + file_attachment = self.create_file_attachment( + review_request=review_request, + active=False) + + # 2 queries: + # + # 1. Fetch review request + # 2. Fetch inactive review request + queries = [ + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_file_attachments', + }, + 'where': ( + Q(file_attachments__id=file_attachment.pk) + ), + }, + { + 'model': ReviewRequestDraft, + 'num_joins': 1, + 'tables': { + 'reviews_reviewrequestdraft', + 'reviews_reviewrequestdraft_inactive_file_attachments', + }, + 'where': ( + Q(inactive_file_attachments__id=file_attachment.pk) + ), + }, + ] + + with self.assertQueries(queries): + self.assertFalse(file_attachment.is_draft) + + @add_fixtures(['test_users']) + def test_is_draft_with_user_file_attachment(self) -> None: + """Testing FileAttachment.is_draft with a user file attachment""" + user = self.create_user() + file_attachment = self.create_user_file_attachment(user=user) + + with self.assertNumQueries(0): + self.assertFalse(file_attachment.is_draft) + @add_fixtures(['test_users', 'test_scmtools']) def test_upload_file(self): """Testing uploading a file attachment""" diff --git a/reviewboard/reviews/builtin_fields.py b/reviewboard/reviews/builtin_fields.py index 847d308f2a297aa779e87b6fc0fc50033ade5fb4..8c7bff608842ca9dfe50a894c21828eea60c13f7 100644 --- a/reviewboard/reviews/builtin_fields.py +++ b/reviewboard/reviews/builtin_fields.py @@ -1217,10 +1217,11 @@ class FileAttachmentsField(ReviewRequestPageDataMixin, BuiltinFieldMixin, review_request = self.review_request_details.get_review_request() model_attrs = { - 'id': attachment.pk, - 'loaded': True, 'downloadURL': attachment.get_absolute_url(), 'filename': attachment.filename, + 'id': attachment.pk, + 'isDraft': attachment.is_draft, + 'loaded': True, 'revision': attachment.attachment_revision, 'thumbnailHTML': attachment.thumbnail, } diff --git a/reviewboard/static/rb/js/common/resources/models/fileAttachmentModel.ts b/reviewboard/static/rb/js/common/resources/models/fileAttachmentModel.ts index c80e833a75ae89ab80ba263761695deb7369a2a3..30e5c102ad4cc846809123f849eb06bceb0c15b0 100644 --- a/reviewboard/static/rb/js/common/resources/models/fileAttachmentModel.ts +++ b/reviewboard/static/rb/js/common/resources/models/fileAttachmentModel.ts @@ -28,6 +28,9 @@ export interface FileAttachmentAttrs extends BaseResourceAttrs { /** The name of the file, for existing file attachments. */ filename: string; + /** Whether the file attachment is a draft. */ + isDraft: boolean; + /** The URL to the review UI for this file attachment. */ reviewURL: string; @@ -68,6 +71,7 @@ export class FileAttachment extends BaseResource { 'downloadURL': null, 'file': null, 'filename': null, + 'isDraft': true, 'reviewURL': null, 'revision': null, 'thumbnailHTML': null, @@ -82,6 +86,7 @@ export class FileAttachment extends BaseResource { attachmentHistoryID: 'attachment_history_id', downloadURL: 'url', file: 'path', + isDraft: 'is_draft', reviewURL: 'review_url', thumbnailHTML: 'thumbnail', }; @@ -97,6 +102,7 @@ export class FileAttachment extends BaseResource { 'caption', 'downloadURL', 'filename', + 'isDraft', 'reviewURL', 'revision', 'thumbnailHTML', diff --git a/reviewboard/static/rb/js/common/resources/models/tests/fileAttachmentModelTests.ts b/reviewboard/static/rb/js/common/resources/models/tests/fileAttachmentModelTests.ts index 5f3d1537288376fb927a14de56c64883bb93915d..bcb7ac7e970d3e19102162e3f0de534d65807150 100644 --- a/reviewboard/static/rb/js/common/resources/models/tests/fileAttachmentModelTests.ts +++ b/reviewboard/static/rb/js/common/resources/models/tests/fileAttachmentModelTests.ts @@ -62,6 +62,7 @@ suite('rb/resources/models/FileAttachment', function() { caption: 'caption', filename: 'filename', id: 42, + is_draft: true, review_url: 'reviewURL', revision: 123, thumbnail: 'thumbnailHTML', @@ -75,6 +76,7 @@ suite('rb/resources/models/FileAttachment', function() { expect(data.downloadURL).toBe('downloadURL'); expect(data.filename).toBe('filename'); expect(data.id).toBe(42); + expect(data.isDraft).toBe(true); expect(data.reviewURL).toBe('reviewURL'); expect(data.revision).toBe(123); expect(data.thumbnailHTML).toBe('thumbnailHTML'); diff --git a/reviewboard/webapi/resources/base_file_attachment.py b/reviewboard/webapi/resources/base_file_attachment.py index bc6501723943c5890301c452da0955f143547fe6..8d8c7693eb5c0ed4256c1fd15082f9c1c1dff6fd 100644 --- a/reviewboard/webapi/resources/base_file_attachment.py +++ b/reviewboard/webapi/resources/base_file_attachment.py @@ -1,4 +1,7 @@ -from djblets.webapi.fields import IntFieldType, DictFieldType, StringFieldType +from djblets.webapi.fields import (BooleanFieldType, + DictFieldType, + IntFieldType, + StringFieldType) from reviewboard.attachments.models import FileAttachment from reviewboard.webapi.base import WebAPIResource @@ -42,6 +45,11 @@ class BaseFileAttachmentResource(WebAPIResource): 'type': IntFieldType, 'description': 'The numeric ID of the file.', }, + 'is_draft': { + 'type': BooleanFieldType, + 'description': 'Whether the file attachment is a draft.', + 'added_in': '6.0', + }, 'mimetype': { 'type': StringFieldType, 'description': 'The mimetype for the file.', diff --git a/reviewboard/webapi/resources/base_review_request_file_attachment.py b/reviewboard/webapi/resources/base_review_request_file_attachment.py index 3ce27d7629febb6f3091f585b88c1747fd040de5..5e6084c577730622bc790a1f977d25bdcd694214 100644 --- a/reviewboard/webapi/resources/base_review_request_file_attachment.py +++ b/reviewboard/webapi/resources/base_review_request_file_attachment.py @@ -256,9 +256,10 @@ class BaseReviewRequestFileAttachmentResource(BaseFileAttachmentResource): return DOES_NOT_EXIST try: - resources.review_request_draft.prepare_draft( + draft = resources.review_request_draft.prepare_draft( request, review_request) + file._review_request_draft = draft # no effect except PermissionDenied: return self.get_no_access_error(request) diff --git a/reviewboard/webapi/tests/test_file_attachment.py b/reviewboard/webapi/tests/test_file_attachment.py index b751b9029d7bb39ffe64eef25d4cd8b6bd6c7e5e..af398cbd44f64369903bd46508f6db69e8b9dc64 100644 --- a/reviewboard/webapi/tests/test_file_attachment.py +++ b/reviewboard/webapi/tests/test_file_attachment.py @@ -29,11 +29,49 @@ class ResourceListTests(ReviewRequestChildListMixin, return (get_file_attachment_list_url(review_request), file_attachment_list_mimetype) - def compare_item(self, item_rsp, attachment): - self.assertEqual(item_rsp['id'], attachment.pk) + def compare_item( + self, + item_rsp: dict, + attachment: FileAttachment, + use_draft_caption: bool = False, + ) -> None: + """Compare an item in the results to the stored item. + + Args: + item_rsp (dict): + The item dict in the results. + + attachment (reviewboard.attachments.models.FileAttachment): + The stored item. + + use_draft_caption (bool): + Whether to compare the caption from the results to the + ``draft_caption`` attribute or the actual ``caption``. + + Raises: + AssertionError: + The item in the results does not match the stored item. + """ + self.assertEqual(item_rsp['absolute_url'], + attachment.get_absolute_url()) + self.assertEqual(item_rsp['attachment_history_id'], + attachment.attachment_history_id) + + if use_draft_caption: + self.assertEqual(item_rsp['caption'], attachment.draft_caption) + else: + self.assertEqual(item_rsp['caption'], attachment.caption) + self.assertEqual(item_rsp['extra_data'], attachment.extra_data) self.assertEqual(item_rsp['filename'], attachment.filename) + self.assertEqual(item_rsp['icon_url'], attachment.icon_url) + self.assertEqual(item_rsp['id'], attachment.pk) + self.assertEqual(item_rsp['is_draft'], attachment.is_draft) + self.assertEqual(item_rsp['mimetype'], attachment.mimetype) + self.assertEqual(item_rsp['review_url'], + self.resource.serialize_review_url_field(attachment)) self.assertEqual(item_rsp['revision'], attachment.attachment_revision) + self.assertEqual(item_rsp['thumbnail'], attachment.thumbnail) # # HTTP GET tests @@ -98,14 +136,15 @@ class ResourceListTests(ReviewRequestChildListMixin, def check_post_result(self, user, rsp, review_request): draft = review_request.get_draft() self.assertIsNotNone(draft) - self.assertIn('file_attachment', rsp) - item_rsp = rsp['file_attachment'] + item_rsp = rsp['file_attachment'] attachment = FileAttachment.objects.get(pk=item_rsp['id']) + + self.assertTrue(attachment.is_draft) self.assertIn(attachment, draft.file_attachments.all()) self.assertNotIn(attachment, review_request.file_attachments.all()) - self.compare_item(item_rsp, attachment) + self.compare_item(item_rsp, attachment, use_draft_caption=True) def test_post_not_owner(self): """Testing the POST review-requests//file-attachments/ API @@ -220,13 +259,49 @@ class ResourceItemTests(ReviewRequestChildItemMixin, return (get_file_attachment_item_url(file_attachment), file_attachment_item_mimetype) - def compare_item(self, item_rsp, attachment): - self.assertEqual(item_rsp['id'], attachment.pk) + def compare_item( + self, + item_rsp: dict, + attachment: FileAttachment, + use_draft_caption: bool = False, + ) -> None: + """Compare an item in the results to the stored item. + + Args: + item_rsp (dict): + The item dict in the results. + + attachment (reviewboard.attachments.models.FileAttachment): + The stored item. + + use_draft_caption (bool): + Whether to compare the caption from the results to the + ``draft_caption`` attribute or the actual ``caption``. + + Raises: + AssertionError: + The item in the results does not match the stored item. + """ + self.assertEqual(item_rsp['absolute_url'], + attachment.get_absolute_url()) + self.assertEqual(item_rsp['attachment_history_id'], + attachment.attachment_history_id) + + if use_draft_caption: + self.assertEqual(item_rsp['caption'], attachment.draft_caption) + else: + self.assertEqual(item_rsp['caption'], attachment.caption) + self.assertEqual(item_rsp['extra_data'], attachment.extra_data) self.assertEqual(item_rsp['filename'], attachment.filename) + self.assertEqual(item_rsp['icon_url'], attachment.icon_url) + self.assertEqual(item_rsp['id'], attachment.pk) + self.assertEqual(item_rsp['is_draft'], attachment.is_draft) + self.assertEqual(item_rsp['mimetype'], attachment.mimetype) + self.assertEqual(item_rsp['review_url'], + self.resource.serialize_review_url_field(attachment)) self.assertEqual(item_rsp['revision'], attachment.attachment_revision) - self.assertEqual(item_rsp['absolute_url'], - attachment.get_absolute_url()) + self.assertEqual(item_rsp['thumbnail'], attachment.thumbnail) # # HTTP DELETE tests @@ -244,6 +319,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, def check_delete_result(self, user, review_request, file_attachment): draft = review_request.get_draft() self.assertIsNotNone(draft) + self.assertTrue(file_attachment.is_draft) self.assertIn(file_attachment, draft.inactive_file_attachments.all()) self.assertNotIn(file_attachment, draft.file_attachments.all()) self.assertIn(file_attachment, review_request.file_attachments.all()) @@ -297,6 +373,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, draft = review_request.get_draft() self.assertIsNotNone(draft) + self.assertTrue(file_attachment.is_draft) self.assertIn(file_attachment, draft.file_attachments.all()) self.assertIn(file_attachment, review_request.file_attachments.all()) diff --git a/reviewboard/webapi/tests/test_file_attachment_draft.py b/reviewboard/webapi/tests/test_file_attachment_draft.py index 0ab7d8a07008091cf9fe342b3fb3a972b0d7dc02..67b28d4e1a0db1a2d342a9a18d4e07782b3ff7fc 100644 --- a/reviewboard/webapi/tests/test_file_attachment_draft.py +++ b/reviewboard/webapi/tests/test_file_attachment_draft.py @@ -18,9 +18,50 @@ class ResourceListTests(BaseWebAPITestCase, metaclass=BasicTestsMetaclass): sample_api_url = 'review-requests//draft/file-attachments/' resource = resources.draft_file_attachment - def compare_item(self, item_rsp, attachment): - self.assertEqual(item_rsp['id'], attachment.pk) + def compare_item( + self, + item_rsp: dict, + attachment: FileAttachment, + use_draft_caption: bool = False, + ) -> None: + """Compare an item in the results to the stored item. + + Args: + item_rsp (dict): + The item dict in the results. + + attachment (reviewboard.attachments.models.FileAttachment): + The stored item. + + use_draft_caption (bool): + Whether to compare the caption from the results to the + ``draft_caption`` attribute or the actual ``caption``. + + Raises: + AssertionError: + The item in the results does not match the stored item. + """ + self.assertTrue(attachment.is_draft) + self.assertEqual(item_rsp['absolute_url'], + attachment.get_absolute_url()) + self.assertEqual(item_rsp['attachment_history_id'], + attachment.attachment_history_id) + + if use_draft_caption: + self.assertEqual(item_rsp['caption'], attachment.draft_caption) + else: + self.assertEqual(item_rsp['caption'], attachment.caption) + + self.assertEqual(item_rsp['extra_data'], attachment.extra_data) self.assertEqual(item_rsp['filename'], attachment.filename) + self.assertEqual(item_rsp['icon_url'], attachment.icon_url) + self.assertEqual(item_rsp['id'], attachment.pk) + self.assertEqual(item_rsp['is_draft'], attachment.is_draft) + self.assertEqual(item_rsp['mimetype'], attachment.mimetype) + self.assertEqual(item_rsp['review_url'], + self.resource.serialize_review_url_field(attachment)) + self.assertEqual(item_rsp['revision'], attachment.attachment_revision) + self.assertEqual(item_rsp['thumbnail'], attachment.thumbnail) # # HTTP GET tests @@ -120,7 +161,7 @@ class ResourceListTests(BaseWebAPITestCase, metaclass=BasicTestsMetaclass): attachment = FileAttachment.objects.get(pk=item_rsp['id']) self.assertIn(attachment, draft.file_attachments.all()) self.assertNotIn(attachment, review_request.file_attachments.all()) - self.compare_item(item_rsp, attachment) + self.compare_item(item_rsp, attachment, use_draft_caption=True) def test_post_with_permission_denied_error(self): """Testing the POST review-requests//draft/file-attachments/ API @@ -148,9 +189,49 @@ class ResourceItemTests(BaseWebAPITestCase, metaclass=BasicTestsMetaclass): sample_api_url = 'review-requests//draft/file-attachments//' resource = resources.draft_file_attachment - def compare_item(self, item_rsp, attachment): - self.assertEqual(item_rsp['id'], attachment.pk) + def compare_item( + self, + item_rsp: dict, + attachment: FileAttachment, + use_draft_caption: bool = False, + ) -> None: + """Compare an item in the results to the stored item. + + Args: + item_rsp (dict): + The item dict in the results. + + attachment (reviewboard.attachments.models.FileAttachment): + The stored item. + + use_draft_caption (bool): + Whether to compare the caption from the results to the + ``draft_caption`` attribute or the actual ``caption``. + + Raises: + AssertionError: + The item in the results does not match the stored item. + """ + self.assertEqual(item_rsp['absolute_url'], + attachment.get_absolute_url()) + self.assertEqual(item_rsp['attachment_history_id'], + attachment.attachment_history_id) + + if use_draft_caption: + self.assertEqual(item_rsp['caption'], attachment.draft_caption) + else: + self.assertEqual(item_rsp['caption'], attachment.caption) + + self.assertEqual(item_rsp['extra_data'], attachment.extra_data) self.assertEqual(item_rsp['filename'], attachment.filename) + self.assertEqual(item_rsp['icon_url'], attachment.icon_url) + self.assertEqual(item_rsp['id'], attachment.pk) + self.assertEqual(item_rsp['is_draft'], attachment.is_draft) + self.assertEqual(item_rsp['mimetype'], attachment.mimetype) + self.assertEqual(item_rsp['review_url'], + self.resource.serialize_review_url_field(attachment)) + self.assertEqual(item_rsp['revision'], attachment.attachment_revision) + self.assertEqual(item_rsp['thumbnail'], attachment.thumbnail) # # HTTP DELETE tests @@ -276,9 +357,7 @@ class ResourceItemTests(BaseWebAPITestCase, metaclass=BasicTestsMetaclass): def check_put_result(self, user, item_rsp, file_attachment): file_attachment = FileAttachment.objects.get(pk=file_attachment.pk) - self.assertEqual(item_rsp['id'], file_attachment.pk) - self.assertEqual(item_rsp['caption'], 'My new caption') - self.assertEqual(file_attachment.draft_caption, 'My new caption') + self.compare_item(item_rsp, file_attachment, use_draft_caption=True) def test_put_with_non_owner_superuser(self): """Testing the PUT review-requests//draft/file-attachments//