Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DTL (Test > Levelbuilder): 04af819b #46582

Merged
merged 31 commits into from
May 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3d41ff5
Reinforce Custom JSON Parsing Logic
Hamms May 19, 2022
a572902
update cdo secrets to support parsing JSON arrays
Hamms May 24, 2022
36d58b5
work on delete accounts helper
molly-moen May 24, 2022
e91302f
Merge branch 'molly/code_review_notes_migration' into molly/deleted-u…
molly-moen May 25, 2022
e438625
update tests
molly-moen May 25, 2022
76631b6
update ability for get commits for code review v2
maureensturgeon May 25, 2022
f6a7fc4
remove accidental schema changes
maureensturgeon May 25, 2022
9e9ad40
return csrf token with code review initial request
maureensturgeon May 26, 2022
dde09ec
add comment requested on a previous code review
maureensturgeon May 26, 2022
9713ba6
Upgrade non-production Aurora Engine version to latest Patch release.
sureshc May 26, 2022
ffe139e
Merge remote-tracking branch 'origin/staging' into molly/deleted-user…
molly-moen May 26, 2022
fff8984
address comments
molly-moen May 26, 2022
0878885
update test name
molly-moen May 26, 2022
336bea4
Configure non-production database clusters to match production cluster.
sureshc May 26, 2022
bc210b2
Create DELETE /code_review_notes/:id
sanchitmalhotra126 May 26, 2022
e0bf93e
enable peer review mode for code review v2 separately
maureensturgeon May 26, 2022
f535f35
don't show close review to peer, refactor get commits ability
maureensturgeon May 26, 2022
78c12ac
Store deleted state on comment and fix tests
sanchitmalhotra126 May 27, 2022
05ad0bb
staging content changes (-robo-commit)
deploy-code-org May 27, 2022
612b6a8
fix tests and code review ability logic
maureensturgeon May 27, 2022
3b61397
Address comments
sanchitmalhotra126 May 27, 2022
f76fc90
Merge pull request #46549 from code-dot-org/maureen/LP-2357-fix-commi…
maureensturgeon May 27, 2022
d4c488d
Merge pull request #46520 from code-dot-org/molly/deleted-user-code-r…
molly-moen May 27, 2022
12ec475
Merge pull request #46444 from code-dot-org/harden-secrets-json-logic
Hamms May 27, 2022
b59ee96
Merge pull request #46571 from code-dot-org/staging
deploy-code-org May 27, 2022
db7bba8
Merge pull request #46561 from code-dot-org/sanchit/code-review-delete
sanchitmalhotra126 May 27, 2022
5159ee5
Merge pull request #46572 from code-dot-org/staging
deploy-code-org May 27, 2022
62f4d5e
Merge pull request #46551 from code-dot-org/upgrade-non-production-au…
sureshc May 27, 2022
6829252
Merge pull request #46560 from code-dot-org/update-database-configura…
sureshc May 27, 2022
04af819
Merge pull request #46577 from code-dot-org/staging
deploy-code-org May 27, 2022
eaa201c
Merge commit '04af819b' into dtl_candidate_04af819b
deploy-code-org May 30, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/src/javalab/JavalabView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions apps/src/templates/instructions/CommitsAndReviewTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -189,6 +199,7 @@ const CommitsAndReviewTab = props => {
addCodeReviewComment={addCodeReviewComment}
closeReview={handleCloseReview}
toggleResolveComment={toggleResolveComment}
deleteCodeReviewComment={deleteCodeReviewComment}
/>
{!openReviewData && !isReadOnlyWorkspace && (
<div style={styles.timelineAligned}>
Expand Down
45 changes: 23 additions & 22 deletions apps/src/templates/instructions/codeReviewV2/CodeReviewDataApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const CodeReviewTimeline = props => {
timelineData,
addCodeReviewComment,
closeReview,
toggleResolveComment
toggleResolveComment,
deleteCodeReviewComment
} = props;

const timelineEndRef = useRef(null);
Expand Down Expand Up @@ -60,6 +61,7 @@ const CodeReviewTimeline = props => {
addCodeReviewComment={addCodeReviewComment}
closeReview={closeReview}
toggleResolveComment={toggleResolveComment}
deleteCodeReviewComment={deleteCodeReviewComment}
/>
);
}
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -50,7 +51,7 @@ const CodeReviewTimelineReview = ({
{javalabMsg.openedDate({date: formattedDate})}
</div>
</div>
{isOpen && (
{isOpen && !viewAsCodeReviewer && (
<div>
<Button
icon="close"
Expand Down Expand Up @@ -85,7 +86,7 @@ const CodeReviewTimelineReview = ({
comment={convertDataForComponent}
key={`code-review-comment-${comment.id}`}
onResolveStateToggle={toggleResolveComment}
onDelete={() => {}}
onDelete={deleteCodeReviewComment}
viewAsCodeReviewer={viewAsCodeReviewer}
/>
);
Expand Down Expand Up @@ -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 = {
Expand Down
43 changes: 33 additions & 10 deletions apps/src/templates/instructions/codeReviewV2/Comment.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -26,7 +28,9 @@ class Comment extends Component {
this.state = {
isCommentResolved: props.comment.isResolved,
hideResolved: true,
isUpdating: false
isUpdating: false,
isDeleted: false,
displayError: false
};
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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'
});
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -234,7 +257,7 @@ class Comment extends Component {
{commentText}
</div>
)}
{hasError && this.renderErrorMessage()}
{displayError && this.renderErrorMessage()}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const DEFAULT_PROPS = {
addCodeReviewComment: () => {},
closeReview: () => {},
toggleResolveComment: () => {},
deleteCodeReviewComment: () => {},
viewAsCodeReviewer: false
};

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ const DEFAULT_PROPS = {
],
addCodeReviewComment: () => {},
closeReview: () => {},
toggleResolveComment: () => {}
toggleResolveComment: () => {},
deleteCodeReviewComment: () => {}
};

const setUp = (overrideProps = {}) => {
Expand Down
3 changes: 3 additions & 0 deletions aws/cloudformation/cloud_formation_stack.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ Parameters:
DBInstanceType:
Type: String
Default: db.r5.2xlarge
DBBackupRetention:
Type: Number
Default: 5
<% end -%>
<% if frontends -%>
DaemonInstanceType:
Expand Down
15 changes: 12 additions & 3 deletions aws/cloudformation/components/database.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 10 additions & 1 deletion dashboard/app/controllers/code_review_notes_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions dashboard/app/controllers/code_reviews_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/controllers/project_versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
{
Expand Down