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

Timed Assessment: Timer starts only when user click "Start" while initiating Attempt #7548

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bivanalhar
Copy link
Contributor

No description provided.

@bivanalhar bivanalhar force-pushed the bivan/timed-assessment-fix branch 8 times, most recently from 6c79d3b to 181cb3b Compare September 12, 2024 09:58
return <div />;
};

export default RemainingTimeTranslations;
Copy link
Contributor

Choose a reason for hiding this comment

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

Check filename RemainingTimeTranslation or RemainingTimeTranslations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -22,7 +22,7 @@ def perform_tracked(assessment, submission_id, submitter)
def force_submit(submission, submitter)
User.with_stamper(submitter) do
ActiveRecord::Base.transaction do
submission.update!('finalise' => 'true')
submission.update!('finalise' => 'true') if submission.attempting?
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I find it cleaner to check for the workflow state attempting during the query in line 13, than the check here within the transaction itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (hours > 0) {
return (
<FormattedMessage
{...translations.hoursMinutesSeconds}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can reduce the number of translations by using =0, you won't have so many if-cases as well.

See example:

{secs, plural, =0 {} one {# second} other {# seconds}}',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -306,6 +306,13 @@ const SubmissionsTableRow = (props) => {
: null}
</TableCell>
) : null}
{assessment.hasTimeLimit ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of : null, why not

{assessment.hasTimeLimit && (
  <TableCell style={tableCenterCellStyle}>
    {submission.timerStartedAt && formatDate(submission.timerStartedAt)}
  </TableCell>
)}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -471,12 +477,20 @@ const translations = defineMessages({
'{stillSomeTimeRemaining, select, true {Once the time is up, \
the assessment will be automatically finalised.} other {Finalising the submission now!}}',
},
timedAssessmentStartDialogMessage: {
id: 'course.assessment.submission.timedExamStartDialogMessage',
defaultMessage: 'Click to start the assessment and the timer',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can simplify to: Start the timed assessment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

startTimedExamAssessmentFailed: {
id: 'course.assessment.submission.startTimedExamAssessmentFailed',
defaultMessage:
'There is an error in starting the exam / assessment. Please check your \
Copy link
Contributor

Choose a reason for hiding this comment

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

missing trailing full stop, and don't need to overload with both terminology "exam / assessment", can keep it short.

There was an error starting the assessment. Please check your internet connection and try again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

timedExamDialogTitle: {
id: 'course.assessment.submission.timedExamDialogTitle',
defaultMessage:
'{stillSomeTimeRemaining, select, true {{remainingTime} {isNewSubmission, select, true {} other {remaining}} to \
complete this exam.} other {The exam has ended!}}',
},
timedExamStartDialogMessage: {
id: 'course.assessment.submission.timedExamStartDialogMessage',
defaultMessage: 'Click to start the exam and the timer',
Copy link
Contributor

Choose a reason for hiding this comment

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

Start the timed exam

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

- timer starts only after student click "Start" when start attempting the exam
- if submission already has timer started at, we won't allow to re-configure it
- add test cases regarding this API
- refactor the TimeLimitBanner to accommodate when timer not started yet
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.

2 participants