Merge pull request #46582 from code-dot-org/dtl_candidate_04af819b

apps/src/javalab/JavalabView.jsx

@@ -72,6 +72,8 @@ class JavalabView extends React.Component {
 
   componentDidUpdate(prevProps) {
     if (prevProps.isReadOnlyWorkspace !== this.props.isReadOnlyWorkspace) {
+      // Whether the finish button is disabled in studioApp is dependent on whether the workspace
+      // is readonly, so if the readonly state changes, the disabled finish button state changes
       const disableFinishButton =
         (!!this.props.isReadOnlyWorkspace && !this.props.isSubmittable) ||
         !!this.props.isCodeReviewing;

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

@@ -14,17 +14,6 @@ export default class CodeReviewDataApi {
     this.locale = locale;
   }
 
-  // Temp method only used to set this.token
-  getCodeReviewCommentsForProject(onDone) {
-    return $.ajax({
-      url: `/code_review_comments/project_comments`,
-      method: 'GET',
-      data: {channel_id: this.channelId}
-    }).done((data, _, request) => {
-      this.token = request.getResponseHeader('csrf-token');
-    });
-  }
-
   getReviewablePeers() {
     return $.ajax({
       url: `/code_reviews/peers_with_open_reviews`,
@@ -73,17 +62,21 @@ export default class CodeReviewDataApi {
   };
 
   getCodeReviews() {
-    // TODO: csrf token get rid of this:
-    this.getCodeReviewCommentsForProject();
-
-    return $.ajax({
-      url: `/code_reviews`,
-      type: 'GET',
-      data: {
-        channelId: this.channelId,
-        levelId: this.levelId,
-        scriptId: this.scriptId
-      }
+    return new Promise((resolve, reject) => {
+      $.ajax({
+        url: `/code_reviews`,
+        type: 'GET',
+        data: {
+          channelId: this.channelId,
+          levelId: this.levelId,
+          scriptId: this.scriptId
+        }
+      })
+        .done((codeReviews, _, request) => {
+          this.token = request.getResponseHeader('csrf-token');
+          resolve(codeReviews);
+        })
+        .fail(result => reject(result));
     });
   }
 
@@ -136,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);
@@ -50,7 +51,7 @@ const CodeReviewTimelineReview = ({
               {javalabMsg.openedDate({date: formattedDate})}
             </div>
           </div>
-          {isOpen && (
+          {isOpen && !viewAsCodeReviewer && (
             <div>
               <Button
                 icon="close"
@@ -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
 };
 
@@ -119,6 +120,12 @@ describe('CodeReviewTimelineReview', () => {
     expect(wrapper.find('Button')).to.have.length(0);
   });
 
+  it('hides the close button if not viewAsCodeReviewer', () => {
+    const review = {...DEFAULT_REVIEW, isOpen: true};
+    const wrapper = setUp({review: review, viewAsCodeReviewer: true});
+    expect(wrapper.find('Button')).to.have.length(0);
+  });
+
   it('displays Comments for each comment', () => {
     const wrapper = setUp();
     expect(wrapper.find(Comment)).to.have.length(2);

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

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

aws/cloudformation/cloud_formation_stack.yml.erb

@@ -16,6 +16,9 @@ Parameters:
   DBInstanceType:
     Type: String
     Default: db.r5.2xlarge
+  DBBackupRetention:
+    Type: Number
+    Default: 5
 <% end -%>
 <% if frontends -%>
   DaemonInstanceType:

aws/cloudformation/components/database.yml.erb

@@ -4,13 +4,22 @@
       DBClusterIdentifier: !Sub "${AWS::StackName}-cluster"
       DBClusterParameterGroupName: !Ref AuroraClusterDBParameters # Resource name generated from db_parameters.yml.erb.
       Engine: aurora-mysql
-      # We will usually do engine version updates manually, since updating this requires replacement, so this value may be out of sync with cluster.
-      EngineVersion: 5.7.12
+      # Updating a Stack with a change to EngineVersion causes "Some Interruption".
+      # Each Database Instance in the cluster is restarted.
+      # https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html#cfn-rds-dbcluster-engineversion
+      EngineVersion: 5.7.mysql_aurora.2.10.2
       MasterUsername: !Sub "{{resolve:secretsmanager:${DatabaseSecret}:SecretString:username}}"
       MasterUserPassword: !Sub "{{resolve:secretsmanager:${DatabaseSecret}:SecretString:password}}"
+      EnableIAMDatabaseAuthentication: true
       VpcSecurityGroupIds: [!ImportValue VPC-DBSecurityGroup]
       DBSubnetGroupName: !ImportValue VPC-DBSubnetGroup
-
+      EnableCloudwatchLogsExports:
+        - general
+        - audit
+        - error
+        - slowquery
+      DeletionProtection: true
+      BackupRetentionPeriod: !Ref DBBackupRetention
 <%
   # Integer range identifying each of the DB Instances to provision in the cluster.
   DB_INSTANCE_RANGE=(0..1)

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/controllers/code_reviews_controller.rb

@@ -16,6 +16,9 @@ def index
     # (Note that this ability is defined on Project.)
     authorize! :index_code_reviews, project
 
+    # Setting custom header here allows us to access the csrf-token and manually use for create
+    headers['csrf-token'] = form_authenticity_token
+
     code_reviews = CodeReview.where(project_id: project.id)
     render json: code_reviews.map(&:summarize_with_comments)
   end

dashboard/app/controllers/project_versions_controller.rb

@@ -24,7 +24,7 @@ def project_commits
     user_storage_id, project_id = storage_decrypt_channel_id(params[:channel_id])
     project_owner = User.find_by(id: user_id_for_storage_id(user_storage_id))
     return render :not_acceptable unless project_owner
-    return render :forbidden unless can?(:project_commits, ProjectVersion.new, project_owner, project_id)
+    return render :forbidden unless can?(:view_project_commits, project_owner)
     commits = ProjectVersion.where(project_id: project_id).where.not(comment: '').order(created_at: :asc)
     commits = commits.map do |commit|
       {