Note edit and delete #507

RichGriff · 2024-06-23T17:37:57Z

API now has a new endpoint for fetching a single comment as well as update and delete
Reworked the /comments/{assurance_case_id}/ endpoint to make this work with the above
Ability for a user to toggle whether they would like to edit the comment or not
Ability for a user to remove the comment

coveralls · 2024-06-23T17:40:48Z

Pull Request Test Coverage Report for Build 9635296974

Details

2 of 23 (8.7%) changed or added relevant lines in 1 file are covered.
7 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.9%) to 82.059%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
eap_backend/eap_api/views.py	2	23	8.7%

Files with Coverage Reduction	New Missed Lines	%
eap_backend/eap_api/views.py	7	61.9%

Totals
Change from base Build 9614951569:	-0.9%
Covered Lines:	1363
Relevant Lines:	1661

💛 - Coveralls

coveralls · 2024-06-23T17:41:03Z

Pull Request Test Coverage Report for Build 9635298553

Details

2 of 22 (9.09%) changed or added relevant lines in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.9%) to 82.059%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
eap_backend/eap_api/views.py	2	22	9.09%

Files with Coverage Reduction	New Missed Lines	%
eap_backend/eap_api/views.py	2	61.9%

Totals
Change from base Build 9614951569:	-0.9%
Covered Lines:	1363
Relevant Lines:	1661

💛 - Coveralls

cptanalatriste · 2024-06-24T07:43:32Z

eap_backend/eap_api/urls.py

@@ -21,11 +21,17 @@
        views.property_claim_list,
        name="property_claim_list",
    ),
+    # path(
+    #     "comments/<int:assurance_case_id>/",
+    #     views.comment_list,


Please remove commented code

cptanalatriste · 2024-06-24T07:49:50Z

eap_backend/eap_api/urls.py

    path(
-        "comments/<int:assurance_case_id>/",
+        "assurance_case/<int:assurance_case_id>/comments/",


Although I might disagree with the initial API design, I see it is important to keep in consistent with all the other resources: case_element_in_plural/ for listing/creation deletion where you can send the assurance_case_id via request parameters instead of via slug. Please adjust accordingly.

@cptanalatriste do you mean changing assurance_case to assurance_cases ?

I was thinking doing something similar to goals, another top-level case item attached to assurance case (see goal_list and goal_detail). In that style, we would have two resources: comments/ to handle case-level retrieve and creation and comments/<int:pk> for update, deletion and single-comment retrieval. Just to keep the same design over the whole API

cptanalatriste · 2024-06-24T07:53:03Z

next_frontend/.env.local

@@ -1,7 +1,7 @@
 # TODO(cgavidia): This should be handled via environment variables.
-# NEXT_PUBLIC_API_URL=http://localhost:8000
+NEXT_PUBLIC_API_URL=http://localhost:8000


Please do not merge this code into develop

@cptanalatriste good catch! Will switch this out, forgot that we had this in the repo.

cptanalatriste · 2024-06-24T07:55:12Z

next_frontend/components/cases/NotesFeed.tsx


  //@ts-ignore
  assuranceCase.comments.sort((a, b) => new Date(b.created_at) - new Date(a.created_at));

  useEffect(() => {
    //@ts-ignore
    assuranceCase.comments.sort((a, b) => new Date(b.created_at) - new Date(a.created_at));
+    console.log('Comments', assuranceCase.comments)


Please remove unnecessary logs

cptanalatriste · 2024-06-24T07:55:33Z

next_frontend/components/cases/NotesFeed.tsx

+      }
+
+      const updatedComments = assuranceCase.comments.filter((comment:any) => comment.id !== id)
+      console.log('Updated Comments', updatedComments)


cptanalatriste · 2024-06-24T07:56:48Z

eap_backend/eap_api/views.py

+
+
+@api_view(["GET", "PUT", "DELETE"])
+def comment_detail(request, pk):


Given that we're out of "crunch-mode", please at least at some unit tests for the new backend code. At least at the test_view level

@cptanalatriste this is where I was hoping you'd be able to help

@RichGriff of course! I can add a couple of tests to the branch

cptanalatriste

Only some minor requests

RichGriff · 2024-06-24T08:31:03Z

@cptanalatriste will work on what I can today on the same branch.

coveralls · 2024-06-24T13:59:06Z

Pull Request Test Coverage Report for Build 9646635937

Details

2 of 22 (9.09%) changed or added relevant lines in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.9%) to 82.059%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
eap_backend/eap_api/views.py	2	22	9.09%

Files with Coverage Reduction	New Missed Lines	%
eap_backend/eap_api/views.py	2	61.9%

Totals
Change from base Build 9614951569:	-0.9%
Covered Lines:	1363
Relevant Lines:	1661

💛 - Coveralls

coveralls · 2024-06-24T13:59:27Z

Pull Request Test Coverage Report for Build 9646622294

Details

2 of 22 (9.09%) changed or added relevant lines in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.9%) to 82.059%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
eap_backend/eap_api/views.py	2	22	9.09%

Files with Coverage Reduction	New Missed Lines	%
eap_backend/eap_api/views.py	2	61.9%

Totals
Change from base Build 9614951569:	-0.9%
Covered Lines:	1363
Relevant Lines:	1661

💛 - Coveralls

RichGriff

Changes made

coveralls · 2024-06-24T16:03:00Z

Pull Request Test Coverage Report for Build 9648535842

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

2 of 22 (9.09%) changed or added relevant lines in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.2%) to 82.746%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
eap_backend/eap_api/views.py	2	22	9.09%

Files with Coverage Reduction	New Missed Lines	%
eap_backend/eap_api/views.py	2	63.62%

Totals
Change from base Build 9614951569:	-0.2%
Covered Lines:	1386
Relevant Lines:	1675

💛 - Coveralls

cptanalatriste · 2024-06-24T16:19:23Z

eap_backend/eap_api/views.py

-        serializer = CommentSerializer(data=request.data)
+        data = request.data.copy()
+        data["assurance_case_id"] = (
+            assurance_case_id  # Ensure assurance_case_id is set in the data


Also, this shouldn't be needed. Aren't you already sending the assurance case ID in the payload body?

Note edit and delete

ac54e5a

RichGriff requested a review from cptanalatriste June 23, 2024 17:38

style: pre-commit fixes

c04b14f

RichGriff mentioned this pull request Jun 23, 2024

[User Story]: Edit and Remove Notes in Assurance Case Editor #467

Closed

8 tasks

cptanalatriste reviewed Jun 24, 2024

View reviewed changes

cptanalatriste requested changes Jun 24, 2024

View reviewed changes

RichGriff added 2 commits June 24, 2024 14:52

updated assurance_case to cases

6d284a9

Updated frontend

9017d31

removed console logs

f138e87

RichGriff commented Jun 24, 2024

View reviewed changes

RichGriff merged commit 6e5da00 into develop Jun 24, 2024
8 checks passed

RichGriff deleted the #467-updating-removing-notes branch June 24, 2024 16:04

cptanalatriste reviewed Jun 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Note edit and delete #507

Note edit and delete #507

RichGriff commented Jun 23, 2024

coveralls commented Jun 23, 2024 •

edited

Loading

coveralls commented Jun 23, 2024 •

edited

Loading

cptanalatriste Jun 24, 2024

cptanalatriste Jun 24, 2024

RichGriff Jun 24, 2024

cptanalatriste Jun 24, 2024

cptanalatriste Jun 24, 2024

RichGriff Jun 24, 2024

cptanalatriste Jun 24, 2024

cptanalatriste Jun 24, 2024

cptanalatriste Jun 24, 2024

RichGriff Jun 24, 2024

cptanalatriste Jun 24, 2024

cptanalatriste left a comment

RichGriff commented Jun 24, 2024

coveralls commented Jun 24, 2024 •

edited

Loading

coveralls commented Jun 24, 2024 •

edited

Loading

RichGriff left a comment

coveralls commented Jun 24, 2024 •

edited

Loading

cptanalatriste Jun 24, 2024



		@api_view(["GET", "PUT", "DELETE"])
		def comment_detail(request, pk):

Note edit and delete #507

Note edit and delete #507

Conversation

RichGriff commented Jun 23, 2024

coveralls commented Jun 23, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9635296974

Details

💛 - Coveralls

coveralls commented Jun 23, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9635298553

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cptanalatriste left a comment

Choose a reason for hiding this comment

RichGriff commented Jun 24, 2024

coveralls commented Jun 24, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9646635937

Details

💛 - Coveralls

coveralls commented Jun 24, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9646622294

Details

💛 - Coveralls

RichGriff left a comment

Choose a reason for hiding this comment

coveralls commented Jun 24, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9648535842

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Choose a reason for hiding this comment

coveralls commented Jun 23, 2024 •

edited

Loading

coveralls commented Jun 23, 2024 •

edited

Loading

coveralls commented Jun 24, 2024 •

edited

Loading

coveralls commented Jun 24, 2024 •

edited

Loading

coveralls commented Jun 24, 2024 •

edited

Loading