Skip to content

Fix Resurrect Achievement Items #2938

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.

Sign up for GitHub

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

Open
dlglin wants to merge 4 commits into
base: WeBWorK-2.21
Choose a base branch
from
+138 −45
Open

Fix Resurrect Achievement Items #2938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7d7fd79
Fix ressurect achievement items
dlglin Feb 13, 2026
68f98e2
Fix resurrect achievement items
dlglin Mar 26, 2026
2a4996a
fix resurrect achievement items
dlglin Mar 26, 2026
1ccb538
Fix maketext call
dlglin Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 75 additions & 20 deletions lib/WeBWorK/AchievementItems/ResurrectGW.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,69 @@ sub new ($class) {
id => 'ResurrectGW',
name => x('Necromancers Charm'),
description => x(
'Reopens any test for an additional 24 hours. This allows you to take a test even if the '
. 'close date has past. This item does not allow you to take additional versions of the test.'
'Reopens any test for an additional 24 hours. If you are allowed to start new versions of the test, '
. 'then this allows you to start a new test even if the close date has past. '
. 'If you were not allowed to start a new version of the test, '
. 'then this item will not allow you to take additional versions of the test. '
. 'This item will not extend the time limit for any tests that you have already started.'
)
}, $class;
}

sub can_use($self, $set, $records) {
sub can_use ($self, $set, $records) {
return $set->assignment_type =~ /gateway/
&& (after($set->due_date) || ($set->reduced_scoring_date && after($set->reduced_scoring_date)));
&& (after($set->due_date) || ($set->enable_reduced_scoring && after($set->reduced_scoring_date)));
# TODO: Check if a new version can be created, and only allow using this reward in that case.
}

sub print_form ($self, $set, $records, $c) {
return $c->tag(
'p',
$c->maketext(
'Reopen this test for the next 24 hours. This item does not allow you to take any additional '
. 'versions of the test.'
)
);
if (after($set->due_date)) {
return $c->tag(
'p',
$c->maketext(
'Reopen this test for the next 24 hours. If you were allowed to start new versions of the test, '
. 'then this will allow you to start a new test. '
. 'If you have already started all of the versions of the test that you are allowed to start, '
. 'then you should not use this item. '
. 'This item will not extend the time limit for any tests that you have already started.'
)
);
} else {
if (after($set->due_date - ONE_DAY)) {
return $c->tag(
'p',
$c->maketext(
'Reopen this test for full credit for the next 24 hours. If you are allowed to start new versions '
. 'of the test, then this will allow you to start a new test. '
. 'If you have already started all of the versions of the test that you are allowed to start, '
. 'then you should not use this item. '
. 'This item will not extend the time limit for any tests that you have already started.'
)
);
} else {
return $c->c(
$c->tag(
'p',
$c->maketext(
'Reopen this test for full credit for the next 24 hours. After 24 hours any tests will revert '
. 'to counting for [_1]% of their value until [_2].',
$c->ce->{pg}{ansEvalDefaults}{reducedScoringValue} * 100,
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
)
),
$c->tag(
'p',
$c->maketext(
' If you are allowed to start new versions of the test, '
. 'then this will allow you to start a new test. '
. 'If you have already started all of the versions of the test that you are allowed to start, '
. 'then you should not use this item. '
. 'This item will not extend the time limit for any tests that you have already started.'
)
)
)->join('');
}
}
}

sub use_item ($self, $set, $records, $c) {
Expand All @@ -44,18 +87,30 @@ sub use_item ($self, $set, $records, $c) {
$set->reduced_scoring_date(time + ONE_DAY);
$userSet->reduced_scoring_date($set->reduced_scoring_date);
}
$set->due_date(time + ONE_DAY);
$userSet->due_date($set->due_date);
if ($set->due_date > $set->answer_date) {
$set->answer_date(time + ONE_DAY);
$userSet->answer_date($set->answer_date);
if (after($set->due_date - ONE_DAY)) {
$set->due_date(time + ONE_DAY);
$userSet->due_date($set->due_date);
if ($set->due_date > $set->answer_date) {
$set->answer_date($set->due_date);
$userSet->answer_date($set->answer_date);
}
}
$db->putUserSet($userSet);

return $c->maketext(
'This assignment has been reopened and will now close on [_1].',
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
);
if ($set->enable_reduced_scoring && ($set->reduced_scoring_date != $set->due_date)) {
return $c->maketext(
'This assignment has been reopened and is due on [_1]. After that date any work '
. 'completed will count for [_2]% of its value until [_3].',
$c->formatDateTime($set->reduced_scoring_date, $c->ce->{studentDateDisplayFormat}),
$c->ce->{pg}{ansEvalDefaults}{reducedScoringValue} * 100,
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
);
} else {
return $c->maketext(
'This assignment has been reopened and will now close on [_1].',
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
);
}
}

1;
88 changes: 63 additions & 25 deletions lib/WeBWorK/AchievementItems/ResurrectHW.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,48 +16,86 @@ sub new ($class) {
}, $class;
}

sub can_use($self, $set, $records) {
sub can_use ($self, $set, $records) {
return $set->assignment_type eq 'default'
&& (after($set->due_date) || ($set->reduced_scoring_date && after($set->reduced_scoring_date)));
&& (after($set->due_date) || ($set->enable_reduced_scoring && after($set->reduced_scoring_date)));
}

sub print_form ($self, $set, $records, $c) {
return $c->tag('p',
$c->maketext('Reopen this homework assignment for the next 24 hours. All problems will be rerandomized.'));
if (after($set->due_date)) {
return $c->tag(
'p',
$c->maketext(
'Reopen this homework assignment for the next 24 hours. All problems will be rerandomized.')
);
} else {
if (after($set->due_date - ONE_DAY)) {
return $c->tag('p',
$c->maketext('Reopen this homework assignment for full credit for the next 24 hours. '));
} else {
return $c->tag(
'p',
$c->maketext(
'Reopen this homework assignment for full credit for the next 24 hours. After 24 hours '
. 'any progress will revert to counting for [_1]% of the value until [_2].',
$c->ce->{pg}{ansEvalDefaults}{reducedScoringValue} * 100,
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
)
);
}
}
}

sub use_item ($self, $set, $records, $c) {
my $db = $c->db;
my $userSet = $db->getUserSet($set->user_id, $set->set_id);

# Change the seed for all of the problems since the set is currently closed.
my %userProblems =
map { $_->problem_id => $_ } $db->getUserProblemsWhere({ user_id => $set->user_id, set_id => $set->set_id });
for my $problem (@$records) {
my $userProblem = $userProblems{ $problem->problem_id };
$userProblem->problem_seed($userProblem->problem_seed % 2**31 + 1);
$problem->problem_seed($userProblem->problem_seed);
$db->putUserProblem($userProblem);
my $db = $c->db;
my $userSet = $db->getUserSet($set->user_id, $set->set_id);
my $rerandomizeMessage = '';

# Change the seed for all of the problems if the set is currently closed.
if (after($set->due_date)) {
my %userProblems =
map { $_->problem_id => $_ }
$db->getUserProblemsWhere({ user_id => $set->user_id, set_id => $set->set_id });
for my $problem (@$records) {
my $userProblem = $userProblems{ $problem->problem_id };
$userProblem->problem_seed($userProblem->problem_seed % 2**31 + 1);
$problem->problem_seed($userProblem->problem_seed);
$db->putUserProblem($userProblem);
}
$rerandomizeMessage = $c->maketext('Problems have been rerandomized.');
}

# Add time to the reduced scoring date if it was defined in the first place
if ($set->reduced_scoring_date) {
$set->reduced_scoring_date(time + ONE_DAY);
$userSet->reduced_scoring_date($set->reduced_scoring_date);
}
# Add time to the close date
$set->due_date(time + ONE_DAY);
$userSet->due_date($set->due_date);
# This may require also extending the answer date.
if ($set->due_date > $set->answer_date) {
$set->answer_date($set->due_date);
$userSet->answer_date($set->answer_date);
# Add time to the close date if necessary
if (after($set->due_date - ONE_DAY)) {
$set->due_date(time + ONE_DAY);
$userSet->due_date($set->due_date);
# This may require also extending the answer date.
if ($set->due_date > $set->answer_date) {
$set->answer_date($set->due_date);
$userSet->answer_date($set->answer_date);
}
}
$db->putUserSet($userSet);

return $c->maketext(
'This assignment has been reopened and will now close on [_1]. Problems have been rerandomized.',
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat}));
if ($set->enable_reduced_scoring && ($set->reduced_scoring_date != $set->due_date)) {
return $c->maketext(
'This assignment has been reopened and is due on [_1]. After that date any work '
. 'completed will count for [_2]% of its value until [_3]. ',
$c->formatDateTime($set->reduced_scoring_date, $c->ce->{studentDateDisplayFormat}),
$c->ce->{pg}{ansEvalDefaults}{reducedScoringValue} * 100,
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
) . $rerandomizeMessage;
} else {
return $c->maketext(
'This assignment has been reopened and will now close on [_1]. ',
$c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat})
) . $rerandomizeMessage;
}
}
Comment on lines +85 to 99
Copy link
Copy Markdown
Contributor

@somiaj somiaj Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

$rerandomizeMessage should also be a positional argument inside of the maketext call.

Copy link
Copy Markdown
Contributor

@somiaj somiaj Apr 1, 2026

Choose a reason for hiding this comment

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

Unsure if that gets tricky or there should be a conditional to check if it should be added or not so the translated strings don't have extra space in them in the case rerandomizeMessage is empty.

Copy link
Copy Markdown
Member Author

@dlglin dlglin Apr 1, 2026

Choose a reason for hiding this comment

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

$rerandomizeMessage is the result of a maketext call, so that string will already be translated. I don't see how having it as a positional argument is necessary.

Is it a problem to have an extra space at the end of a string to be translated?

Copy link
Copy Markdown
Contributor

@somiaj somiaj Apr 1, 2026

Choose a reason for hiding this comment

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

My understanding is not all languages are right to left, so just appending the string to the end may not be appropriate for some translations.

In terms of the html, an extra space won't matter, but what happens if a translation doesn't understand that space is important and leaves it off. Putting the translated string as a positional argument inside the maketext call makes the use of the space clearer and allows positioning the statement in the string appropriately for the translation.

There are various places thought out the code that a string is translated and then added as a positional argument in future translations to join things together. I think this should probably do the same. I think the space thing is very minor and probably won't matter if the positional argument is empty.

Copy link
Copy Markdown
Contributor

@somiaj somiaj Apr 1, 2026

Choose a reason for hiding this comment

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

Others may have a better knowledge, what you did may be okay, I just feel it isn't. I think a positional argument is better than string concatenation. I'm unsure if there is a better approach to avoid having to write lots of cases like you have been doing in most of this.

Copy link
Copy Markdown
Member

@drgrice1 drgrice1 Apr 1, 2026

Choose a reason for hiding this comment

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

It is always best to give as much as possible together for translations. However, generally concatenations of two sentences can be done. Usually the bidirectional algorithm will make it work even for right to left languages. However, the space should be inserted between the translations and not inside of one of them. The translator may not add that space in the translation, and even if they do may not put it in the correct place, not knowing that it is going to be concatenated with another translation.

Perhaps making it a positional argument inside another translation is better though.


1;
Loading