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

Javalab: show appropriate options in comment options menu #41715

Merged
merged 10 commits into from
Jul 28, 2021

Conversation

bencodeorg
Copy link
Contributor

@bencodeorg bencodeorg commented Jul 23, 2021

The "comment options" menu is an ellipsis that appear on each comment to show the option to delete a comment (only for teachers) and to resolve/re-open a comment (only for project owner). This PR:

  • adds a new redux property isPeerReviewing that is passed from the back end when one student is viewing another student's code.
  • hides the ellipsis entirely if a student is in this mode.
  • if the ellipsis is being shown (ie, not isPeerReviewing), shows the delete option in the comment options menu if the user is a teacher
  • if the ellipsis is being shown (ie, not isPeerReviewing) shows the resolve/re-open option (one or the other, depending on the state of the comment) if the user is a student

Student reviewing another student's work

image

Student looking at their own project

image

Teacher looking at student work

image

Testing story

Tested manually with two students in the same section and a teacher. Also added unit tests.

@bencodeorg bencodeorg changed the base branch from staging to ben/javalab-peer-review-read-only July 23, 2021 19:01
Base automatically changed from ben/javalab-peer-review-read-only to staging July 26, 2021 18:46
@@ -45,7 +46,7 @@ export default class Comment extends Component {
};

renderFormattedTimestamp = timestampString =>
moment.utc(timestampString).format('M/D/YYYY [at] h:mm A');
moment(timestampString).format('M/D/YYYY [at] h:mm A');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated fix to show timestamps in local time instead of UTC.

@bencodeorg bencodeorg requested a review from a team July 28, 2021 16:54
Copy link
Contributor

@sanchitmalhotra126 sanchitmalhotra126 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -136,6 +136,7 @@ Javalab.prototype.init = function(config) {
channelId: config.channel,
isProjectLevel: !!config.level.isProjectLevel,
isEditingStartSources: this.isStartMode,
isCodeReviewing: !!config.isCodeReviewing,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious how this gets set - will config.isCodeReviewing be updated every time a user switches to another student's project via the review dropdown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gets passed in from the controller here and is set as a page constant (I believe such that it cannot be changed once the page is loaded):

view_options(is_code_reviewing: true)

As far as I know, we'll be fully reloading the page for one student to view another student's work, so we'd be setting isCodeReviewing to true on page load after a student selects another student from the dropdown.

@@ -8,7 +10,9 @@ import * as codeReviewDataApi from './codeReview/codeReviewDataApi';

const FLASH_ERROR_TIME_MS = 5000;

export default class ReviewTab extends Component {
class ReviewTab extends Component {
static propTypes = {viewAsCodeReviewer: PropTypes.bool.isRequired};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add // populated by redux?

@bencodeorg bencodeorg merged commit fe76f16 into staging Jul 28, 2021
@bencodeorg bencodeorg deleted the ben/javalab-explicit-peer-review-mode branch July 28, 2021 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants