Merge pull request #1649 from doccano/enhancement/removeCommentAPI

[Enhancement] Remove unnecessary CommentListDoc API
2 years ago · 9c11882181
8 changed files with 47 additions and 60 deletions
--- a/backend/api/tests/api/test_comment.py
+++ b/backend/api/tests/api/test_comment.py
@ -11,15 +11,18 @@ class TestCommentListDocAPI(CRUDMixin):
    def setUpTestData(cls):
        cls.project = prepare_project()
        cls.non_member = make_user()
-        doc = make_doc(cls.project.item)
-        make_comment(doc, cls.project.users[0])
+        doc1 = make_doc(cls.project.item)
+        doc2 = make_doc(cls.project.item)
+        make_comment(doc1, cls.project.users[0])
+        make_comment(doc2, cls.project.users[0])
        cls.data = {'text': 'example'}
-        cls.url = reverse(viewname='comment_list_doc', args=[cls.project.item.id, doc.id])
+        cls.url = reverse(viewname='comment_list', args=[cls.project.item.id])
+        cls.url += f'?example={doc1.id}'

    def test_allows_project_member_to_list_comments(self):
        for member in self.project.users:
            response = self.assert_fetch(member, status.HTTP_200_OK)
-            self.assertEqual(len(response.data), 1)
+            self.assertEqual(response.data['count'], 1)

    def test_denies_non_project_member_to_list_comments(self):
        self.assert_fetch(self.non_member, status.HTTP_403_FORBIDDEN)
@ -45,7 +48,7 @@ class TestCommentListProjectAPI(CRUDMixin):
        self.non_member = make_user()
        self.doc = make_doc(self.project.item)
        make_comment(self.doc, self.project.users[0])
-        self.url = reverse(viewname='comment_list_project', args=[self.project.item.id])
+        self.url = reverse(viewname='comment_list', args=[self.project.item.id])

    def test_allows_project_member_to_list_comments(self):
        for member in self.project.users:
@ -87,7 +90,7 @@ class TestCommentDetailAPI(CRUDMixin):
        doc = make_doc(self.project.item)
        comment = make_comment(doc, self.project.users[0])
        self.data = {'text': 'example'}
-        self.url = reverse(viewname='comment_detail', args=[self.project.item.id, doc.id, comment.id])
+        self.url = reverse(viewname='comment_detail', args=[self.project.item.id, comment.id])

    def test_allows_comment_owner_to_get_comment(self):
        # Todo: Allows project member to get comment.
--- a/backend/api/urls.py
+++ b/backend/api/urls.py
@ -115,18 +115,13 @@ urlpatterns_project = [
        view=tag.TagDetail.as_view(),
        name='tag_detail'
    ),
-    path(
-        route='examples/<int:example_id>/comments',
-        view=comment.CommentListDoc.as_view(),
-        name='comment_list_doc'
-    ),
    path(
        route='comments',
-        view=comment.CommentListProject.as_view(),
-        name='comment_list_project'
+        view=comment.CommentList.as_view(),
+        name='comment_list'
    ),
    path(
-        route='examples/<int:example_id>/comments/<int:comment_id>',
+        route='comments/<int:comment_id>',
        view=comment.CommentDetail.as_view(),
        name='comment_detail'
    ),
--- a/backend/api/views/comment.py
+++ b/backend/api/views/comment.py
@ -10,39 +10,28 @@ from ..permissions import IsOwnComment
 from ..serializers import CommentSerializer


-class CommentListDoc(generics.ListCreateAPIView):
-    pagination_class = None
-    permission_classes = [IsAuthenticated & IsInProjectOrAdmin]
-    serializer_class = CommentSerializer
-    model = Comment
-
-    def get_queryset(self):
-        queryset = self.model.objects.filter(
-            example__project_id=self.kwargs['project_id'],
-            example=self.kwargs['example_id']
-        )
-        return queryset
-
-    def perform_create(self, serializer):
-        serializer.save(example_id=self.kwargs['example_id'], user=self.request.user)
-
-
-class CommentListProject(generics.ListAPIView):
+class CommentList(generics.ListCreateAPIView):
    permission_classes = [IsAuthenticated & IsInProjectOrAdmin]
    serializer_class = CommentSerializer
    filter_backends = (DjangoFilterBackend, filters.SearchFilter)
+    filterset_fields = ['example']
    search_fields = ('text',)
-    model = Comment

    def get_queryset(self):
-        queryset = self.model.objects.filter(
+        queryset = Comment.objects.filter(
            example__project_id=self.kwargs['project_id']
        )
        return queryset

+    def perform_create(self, serializer):
+        serializer.save(
+            example_id=self.request.query_params.get('example'),
+            user=self.request.user
+        )
+
    def delete(self, request, *args, **kwargs):
        delete_ids = request.data['ids']
-        self.model.objects.filter(user=request.user, pk__in=delete_ids).delete()
+        Comment.objects.filter(user=request.user, pk__in=delete_ids).delete()
        return Response(status=status.HTTP_204_NO_CONTENT)


--- a/frontend/components/tasks/toolbar/ToolbarLaptop.vue
+++ b/frontend/components/tasks/toolbar/ToolbarLaptop.vue
@ -31,7 +31,7 @@
        />
        <v-dialog v-model="dialogComment">
          <form-comment
-            :doc-id="docId"
+            :example-id="docId"
            @click:cancel="dialogComment=false"
          />
        </v-dialog>
--- a/frontend/components/tasks/toolbar/forms/FormComment.vue
+++ b/frontend/components/tasks/toolbar/forms/FormComment.vue
@ -4,7 +4,7 @@
    :cancel-text="$t('generic.close')"
    @cancel="$emit('click:cancel')"
  >
-    <template #content>
+    <template v-if="user.id" #content>
      <form-create
        @add-comment="add"
      />
@ -35,7 +35,7 @@ export default Vue.extend({
  },

  props: {
-    docId: {
+    exampleId: {
      type: Number,
      required: true
    }
@ -48,12 +48,8 @@ export default Vue.extend({
    }
  },

-  async fetch() {
-    this.user = await this.$services.user.getMyProfile()
-  },
-
  watch: {
-    currentDoc: {
+    exampleId: {
      handler(val) {
        if (val !== undefined) {
          this.list()
@ -64,20 +60,24 @@ export default Vue.extend({
    }
  },

+  async created() {
+    this.user = await this.$services.user.getMyProfile()
+  },
+
  methods: {
    async list() {
-      this.comments = await this.$services.comment.list(this.$route.params.id, this.docId)
+      this.comments = await this.$services.comment.list(this.$route.params.id, this.exampleId)
    },
    async add(message: string) {
-      await this.$services.comment.create(this.$route.params.id, this.docId, message)
+      await this.$services.comment.create(this.$route.params.id, this.exampleId, message)
      this.list()
    },
    async remove(item: CommentReadDTO) {
-      await this.$services.comment.delete(this.$route.params.id, this.docId, item)
+      await this.$services.comment.delete(this.$route.params.id, item)
      this.list()
    },
    async maybeUpdate(item: CommentReadDTO) {
-      await this.$services.comment.update(this.$route.params.id, this.docId, item)
+      await this.$services.comment.update(this.$route.params.id, item)
      this.list()
    }
  }
--- a/frontend/domain/models/comment/commentRepository.ts
+++ b/frontend/domain/models/comment/commentRepository.ts
@ -9,9 +9,9 @@ export interface CommentRepository {

  create(projectId: string, docId: number, text: string): Promise<CommentItem>

-  update(projectId: string, docId: number, item: CommentItem): Promise<CommentItem>
+  update(projectId: string, item: CommentItem): Promise<CommentItem>

-  delete(projectId: string, docId: number, commentId: number): Promise<void>
+  delete(projectId: string, commentId: number): Promise<void>

  deleteBulk(projectId: string, items: number[]): Promise<void>
 }
--- a/frontend/repositories/comment/apiCommentRepository.ts
+++ b/frontend/repositories/comment/apiCommentRepository.ts
@ -16,25 +16,25 @@ export class APICommentRepository implements CommentRepository {
  }

  async list(projectId: string, exampleId: number): Promise<CommentItem[]> {
-    const url = `/projects/${projectId}/examples/${exampleId}/comments`
+    const url = `/projects/${projectId}/comments?example=${exampleId}&limit=100`
    const response = await this.request.get(url)
-    return response.data.map((item: any) => plainToInstance(CommentItem, item))
+    return response.data.results.map((item: any) => plainToInstance(CommentItem, item))
  }

  async create(projectId: string, exampleId: number, text: string): Promise<CommentItem> {
-    const url = `/projects/${projectId}/examples/${exampleId}/comments`
+    const url = `/projects/${projectId}/comments?example=${exampleId}`
    const response = await this.request.post(url, { projectId, exampleId, text })
    return plainToInstance(CommentItem, response.data)
  }

-  async update(projectId: string, exampleId: number, item: CommentItem): Promise<CommentItem> {
-    const url = `/projects/${projectId}/examples/${exampleId}/comments/${item.id}`
+  async update(projectId: string, item: CommentItem): Promise<CommentItem> {
+    const url = `/projects/${projectId}/comments/${item.id}`
    const response = await this.request.put(url, item.toObject())
    return plainToInstance(CommentItem, response.data)
  }

-  async delete(projectId: string, exampleId: number, commentId: number): Promise<void> {
-    const url = `/projects/${projectId}/examples/${exampleId}/comments/${commentId}`
+  async delete(projectId: string, commentId: number): Promise<void> {
+    const url = `/projects/${projectId}/comments/${commentId}`
    const response = await this.request.delete(url)
  }

--- a/frontend/services/application/comment/commentApplicationService.ts
+++ b/frontend/services/application/comment/commentApplicationService.ts
@ -22,13 +22,13 @@ export class CommentApplicationService {
    return this.repository.create(projectId, docId, text)
  }

-  public update(projectId: string, docId: number, item: CommentReadDTO): Promise<CommentItem> {
+  public update(projectId: string, item: CommentReadDTO): Promise<CommentItem> {
    const comment = plainToInstance(CommentItem, item)
-    return this.repository.update(projectId, docId, comment)
+    return this.repository.update(projectId, comment)
  }

-  public delete(projectId: string, docId: number, item: CommentReadDTO): Promise<void> {
-    return this.repository.delete(projectId, docId, item.id)
+  public delete(projectId: string, item: CommentReadDTO): Promise<void> {
+    return this.repository.delete(projectId, item.id)
  }

  public deleteBulk(projectId: string, items: CommentReadDTO[]): Promise<void> {