DTT (Staging > Test) [robo-dtt]

apps/src/templates/instructions/CommitsAndReviewTab.jsx

@@ -105,6 +105,16 @@ const CommitsAndReviewTab = props => {
     }
   };
 
+  const deleteCodeReviewComment = async (commentId, onSuccess, onFailure) => {
+    try {
+      await dataApi.deleteCodeReviewComment(commentId);
+      onSuccess();
+    } catch (err) {
+      console.log(err);
+      onFailure();
+    }
+  };
+
   const handleCloseReview = async (onSuccess, onFailure) => {
     try {
       const closedReview = await dataApi.closeReview(openReviewData.id);
@@ -189,6 +199,7 @@ const CommitsAndReviewTab = props => {
             addCodeReviewComment={addCodeReviewComment}
             closeReview={handleCloseReview}
             toggleResolveComment={toggleResolveComment}
+            deleteCodeReviewComment={deleteCodeReviewComment}
           />
           {!openReviewData && !isReadOnlyWorkspace && (
             <div style={styles.timelineAligned}>

apps/src/templates/instructions/codeReviewV2/CodeReviewDataApi.js

@@ -129,6 +129,14 @@ export default class CodeReviewDataApi {
     });
   }
 
+  deleteCodeReviewComment(commentId) {
+    return $.ajax({
+      url: `/code_review_notes/${commentId}`,
+      type: 'DELETE',
+      headers: {'X-CSRF-Token': this.token}
+    });
+  }
+
   closeReview(reviewId) {
     return new Promise((resolve, reject) => {
       $.ajax({

apps/src/templates/instructions/codeReviewV2/CodeReviewTimeline.jsx

@@ -20,7 +20,8 @@ const CodeReviewTimeline = props => {
     timelineData,
     addCodeReviewComment,
     closeReview,
-    toggleResolveComment
+    toggleResolveComment,
+    deleteCodeReviewComment
   } = props;
 
   const timelineEndRef = useRef(null);
@@ -60,6 +61,7 @@ const CodeReviewTimeline = props => {
               addCodeReviewComment={addCodeReviewComment}
               closeReview={closeReview}
               toggleResolveComment={toggleResolveComment}
+              deleteCodeReviewComment={deleteCodeReviewComment}
             />
           );
         }
@@ -75,7 +77,8 @@ CodeReviewTimeline.propTypes = {
   ),
   addCodeReviewComment: PropTypes.func.isRequired,
   closeReview: PropTypes.func.isRequired,
-  toggleResolveComment: PropTypes.func.isRequired
+  toggleResolveComment: PropTypes.func.isRequired,
+  deleteCodeReviewComment: PropTypes.func.isRequired
 };
 
 export default CodeReviewTimeline;

apps/src/templates/instructions/codeReviewV2/CodeReviewTimelineReview.jsx

@@ -19,7 +19,8 @@ const CodeReviewTimelineReview = ({
   addCodeReviewComment,
   closeReview,
   toggleResolveComment,
-  viewAsCodeReviewer
+  viewAsCodeReviewer,
+  deleteCodeReviewComment
 }) => {
   const {id, createdAt, isOpen, version, isVersionExpired, comments} = review;
   const [displayCloseError, setDisplayCloseError] = useState(false);
@@ -85,7 +86,7 @@ const CodeReviewTimelineReview = ({
                 comment={convertDataForComponent}
                 key={`code-review-comment-${comment.id}`}
                 onResolveStateToggle={toggleResolveComment}
-                onDelete={() => {}}
+                onDelete={deleteCodeReviewComment}
                 viewAsCodeReviewer={viewAsCodeReviewer}
               />
             );
@@ -114,7 +115,8 @@ CodeReviewTimelineReview.propTypes = {
   addCodeReviewComment: PropTypes.func.isRequired,
   closeReview: PropTypes.func.isRequired,
   toggleResolveComment: PropTypes.func.isRequired,
-  viewAsCodeReviewer: PropTypes.bool
+  viewAsCodeReviewer: PropTypes.bool,
+  deleteCodeReviewComment: PropTypes.func.isRequired
 };
 
 const styles = {

apps/src/templates/instructions/codeReviewV2/Comment.jsx

@@ -11,6 +11,8 @@ import Tooltip from '@cdo/apps/templates/Tooltip';
 import {ViewType} from '@cdo/apps/code-studio/viewAsRedux';
 import Spinner from '@cdo/apps/code-studio/pd/components/spinner';
 
+const FLASH_ERROR_TIME_MS = 5000;
+
 class Comment extends Component {
   static propTypes = {
     comment: commentShape.isRequired,
@@ -26,7 +28,9 @@ class Comment extends Component {
     this.state = {
       isCommentResolved: props.comment.isResolved,
       hideResolved: true,
-      isUpdating: false
+      isUpdating: false,
+      isDeleted: false,
+      displayError: false
     };
   }
 
@@ -104,14 +108,20 @@ class Comment extends Component {
       this.props.comment.id,
       newIsResolvedStatus,
       () => this.setState({isCommentResolved: newIsResolvedStatus}),
-      () => {
-        // TODO: handle set resolve failure
-      }
+      this.flashErrorMessage
+    );
+  };
+
+  deleteCodeReviewComment = () => {
+    this.props.onDelete(
+      this.props.comment.id,
+      () => this.setState({isDeleted: true}),
+      this.flashErrorMessage
     );
   };
 
   getMenuItems = () => {
-    const {viewAsCodeReviewer, viewAsTeacher, onDelete} = this.props;
+    const {viewAsCodeReviewer, viewAsTeacher} = this.props;
     const {hideResolved, isCommentResolved} = this.state;
     let menuItems = [];
     if (isCommentResolved) {
@@ -136,7 +146,7 @@ class Comment extends Component {
     if (viewAsTeacher) {
       // Instructors can delete comments
       menuItems.push({
-        onClick: onDelete,
+        onClick: this.deleteCodeReviewComment,
         text: javalabMsg.delete(),
         iconClass: 'trash'
       });
@@ -169,16 +179,29 @@ class Comment extends Component {
     });
   };
 
+  flashErrorMessage = () => {
+    this.setState({displayError: true});
+    setTimeout(() => this.setState({displayError: false}), FLASH_ERROR_TIME_MS);
+  };
+
   render() {
+    if (this.state.isDeleted) {
+      return null;
+    }
+
     const {
       commentText,
       timestampString,
       isFromTeacher,
-      isFromOlderVersionOfProject,
-      hasError
+      isFromOlderVersionOfProject
     } = this.props.comment;
 
-    const {hideResolved, isUpdating, isCommentResolved} = this.state;
+    const {
+      hideResolved,
+      isUpdating,
+      isCommentResolved,
+      displayError
+    } = this.state;
 
     return (
       <div
@@ -234,7 +257,7 @@ class Comment extends Component {
             {commentText}
           </div>
         )}
-        {hasError && this.renderErrorMessage()}
+        {displayError && this.renderErrorMessage()}
       </div>
     );
   }

apps/test/unit/templates/instructions/codeReviewV2/CodeReviewTimelineReviewTest.jsx

@@ -41,6 +41,7 @@ const DEFAULT_PROPS = {
   addCodeReviewComment: () => {},
   closeReview: () => {},
   toggleResolveComment: () => {},
+  deleteCodeReviewComment: () => {},
   viewAsCodeReviewer: false
 };
 

apps/test/unit/templates/instructions/codeReviewV2/CodeReviewTimelineTest.jsx

@@ -54,7 +54,8 @@ const DEFAULT_PROPS = {
   ],
   addCodeReviewComment: () => {},
   closeReview: () => {},
-  toggleResolveComment: () => {}
+  toggleResolveComment: () => {},
+  deleteCodeReviewComment: () => {}
 };
 
 const setUp = (overrideProps = {}) => {

dashboard/app/controllers/code_review_notes_controller.rb

@@ -1,7 +1,7 @@
 class CodeReviewNotesController < ApplicationController
   before_action :authenticate_user!
 
-  load_and_authorize_resource :code_review_note, only: [:update]
+  load_and_authorize_resource :code_review_note, only: [:update, :destroy]
 
   # POST /code_review_notes
   def create
@@ -34,4 +34,13 @@ def update
 
     render json: @code_review_note.summarize
   end
+
+  # DELETE /code_review_notes/:id
+  def destroy
+    if @code_review_note.delete
+      return head :ok
+    else
+      return head :bad_request
+    end
+  end
 end

dashboard/app/models/ability.rb

@@ -156,6 +156,14 @@ def initialize(user)
         code_review_note.code_review.user_id == user.id
       end
 
+      can :destroy, CodeReviewNote do |code_review_note|
+        # Teachers can delete comments on their student's projects,
+        # their own comments anywhere, and comments on their projects.
+        code_review_note.code_review.owner&.student_of?(user) ||
+          (user.teacher? && user.id == code_review_note.commenter_id) ||
+          (user.teacher? && user.id == code_review_note.code_review.user_id)
+      end
+
       can :create, Pd::RegionalPartnerProgramRegistration, user_id: user.id
       can :read, Pd::Session
       can :manage, Pd::Enrollment, user_id: user.id

dashboard/config/routes.rb

@@ -976,7 +976,7 @@
     get :peers_with_open_reviews, on: :collection
   end
 
-  resources :code_review_notes, only: [:create, :update]
+  resources :code_review_notes, only: [:create, :update, :destroy]
 
   resources :code_review_comments, only: [:create, :destroy] do
     patch :toggle_resolved, on: :member

dashboard/test/controllers/code_review_notes_controller_test.rb

@@ -6,7 +6,8 @@ class CodeReviewNotesControllerTest < ActionController::TestCase
   setup_all do
     @project_owner = create :student
     @peer = create :student
-    section = create :section, code_review_expires_at: Time.now.utc + 1.day
+    @teacher = create :teacher
+    section = create :section, code_review_expires_at: Time.now.utc + 1.day, teacher: @teacher
     student_follower = create :follower, section: section, student_user: @project_owner
     peer_follower = create :follower, section: section, student_user: @peer
 
@@ -54,4 +55,28 @@ class CodeReviewNotesControllerTest < ActionController::TestCase
     assert_not_nil response_json['id']
     assert_equal true, response_json['isResolved']
   end
+
+  test 'delete code review comment' do
+    review_note = create :code_review_note, code_review: @code_review, commenter: @peer
+
+    sign_in @teacher
+
+    delete :destroy, params: {
+      id: review_note.id
+    }
+
+    assert_response :success
+  end
+
+  test 'students cannot delete code review comment' do
+    review_note = create :code_review_note, code_review: @code_review, commenter: @peer
+
+    sign_in @project_owner
+
+    delete :destroy, params: {
+      id: review_note.id
+    }
+
+    assert_response :forbidden
+  end
 end