From 406fedc4d1229f4462818035b7d907ac3d4fcbe3 Mon Sep 17 00:00:00 2001 From: dzhuang Date: Sun, 5 Feb 2017 00:36:07 +0800 Subject: [PATCH 1/2] Allow optional pages by adding page_desc.is_optional_page attribute. --- course/flow.py | 127 ++++++++++---- course/models.py | 2 +- course/page/base.py | 19 +++ course/page/choice.py | 12 ++ course/page/code.py | 4 + course/page/inline.py | 4 + course/page/text.py | 16 ++ course/page/upload.py | 4 + .../course/flow-completion-grade.html | 161 ++++++++++-------- .../course/flow-confirm-completion.html | 12 +- course/templates/course/flow-page.html | 4 + course/views.py | 16 +- doc/flow.rst | 6 + 13 files changed, 276 insertions(+), 111 deletions(-) diff --git a/course/flow.py b/course/flow.py index e57807ce..f8d4aae2 100644 --- a/course/flow.py +++ b/course/flow.py @@ -617,11 +617,12 @@ def get_session_answered_page_data( flow_session, # type: FlowSession answer_visits # type: List[Optional[FlowPageVisit]] ): - # type: (...) -> Tuple[List[FlowPageData], List[FlowPageData]] + # type: (...) -> Tuple[List[FlowPageData], List[FlowPageData], bool] all_page_data = get_all_page_data(flow_session) answered_page_data_list = [] # type: List[FlowPageData] unanswered_page_data_list = [] # type: List[FlowPageData] + is_interactive_flow = False # type: bool for i, page_data in enumerate(all_page_data): assert i == page_data.ordinal @@ -634,12 +635,14 @@ def get_session_answered_page_data( page = instantiate_flow_page_with_ctx(fctx, page_data) if page.expects_answer(): - if answer_data is None: - unanswered_page_data_list.append(page_data) - else: - answered_page_data_list.append(page_data) + is_interactive_flow = True + if not page.is_optional_page: + if answer_data is None: + unanswered_page_data_list.append(page_data) + else: + answered_page_data_list.append(page_data) - return (answered_page_data_list, unanswered_page_data_list) + return (answered_page_data_list, unanswered_page_data_list, is_interactive_flow) class GradeInfo(object): @@ -661,6 +664,10 @@ class GradeInfo(object): partially_correct_count, # type: int incorrect_count, # type: int unknown_count, # type: int + optional_fully_correct_count=0, # type: int + optional_partially_correct_count=0, # type: int + optional_incorrect_count=0, # type: int + optional_unknown_count=0, # type: int ): # type: (...) -> None self.points = points @@ -671,6 +678,10 @@ class GradeInfo(object): self.partially_correct_count = partially_correct_count self.incorrect_count = incorrect_count self.unknown_count = unknown_count + self.optional_fully_correct_count = optional_fully_correct_count + self.optional_partially_correct_count = optional_partially_correct_count + self.optional_incorrect_count = optional_incorrect_count + self.optional_unknown_count = optional_unknown_count # Rounding to larger than 100% will break the percent bars on the # flow results page. @@ -739,6 +750,32 @@ class GradeInfo(object): """Only to be used for visualization purposes.""" return self.FULL_PERCENT*self.unknown_count/self.total_count() + def optional_total_count(self): + return (self.optional_fully_correct_count + + self.optional_partially_correct_count + + self.optional_incorrect_count + + self.optional_unknown_count) + + def optional_fully_correct_percent(self): + """Only to be used for visualization purposes.""" + return self.FULL_PERCENT * self.optional_fully_correct_count\ + / self.optional_total_count() + + def optional_partially_correct_percent(self): + """Only to be used for visualization purposes.""" + return self.FULL_PERCENT * self.optional_partially_correct_count\ + / self.optional_total_count() + + def optional_incorrect_percent(self): + """Only to be used for visualization purposes.""" + return self.FULL_PERCENT * self.optional_incorrect_count\ + / self.optional_total_count() + + def optional_unknown_percent(self): + """Only to be used for visualization purposes.""" + return self.FULL_PERCENT * self.optional_unknown_count\ + / self.optional_total_count() + # }}} @@ -766,6 +803,11 @@ def gather_grade_info( incorrect_count = 0 unknown_count = 0 + optional_fully_correct_count = 0 + optional_partially_correct_count = 0 + optional_incorrect_count = 0 + optional_unknown_count = 0 + for i, page_data in enumerate(all_page_data): page = instantiate_flow_page_with_ctx(fctx, page_data) @@ -789,29 +831,42 @@ def gather_grade_info( feedback = get_feedback_for_grade(grade) - max_points += grade.max_points + if page.is_optional_page: + if feedback is None or feedback.correctness is None: + optional_unknown_count += 1 + continue - if feedback is None or feedback.correctness is None: - unknown_count += 1 - points = None - continue + if feedback.correctness == 1: + optional_fully_correct_count += 1 + elif feedback.correctness == 0: + optional_incorrect_count += 1 + else: + optional_partially_correct_count += 1 - max_reachable_points += grade.max_points + else: + max_points += grade.max_points - page_points = grade.max_points*feedback.correctness + if feedback is None or feedback.correctness is None: + unknown_count += 1 + points = None + continue - if points is not None: - points += page_points + max_reachable_points += grade.max_points - provisional_points += page_points + page_points = grade.max_points*feedback.correctness - if grade.max_points > 0: - if feedback.correctness == 1: - fully_correct_count += 1 - elif feedback.correctness == 0: - incorrect_count += 1 - else: - partially_correct_count += 1 + if points is not None: + points += page_points + + provisional_points += page_points + + if grade.max_points > 0: + if feedback.correctness == 1: + fully_correct_count += 1 + elif feedback.correctness == 0: + incorrect_count += 1 + else: + partially_correct_count += 1 # {{{ adjust max_points if requested @@ -841,7 +896,12 @@ def gather_grade_info( fully_correct_count=fully_correct_count, partially_correct_count=partially_correct_count, incorrect_count=incorrect_count, - unknown_count=unknown_count) + unknown_count=unknown_count, + + optional_fully_correct_count=optional_fully_correct_count, + optional_partially_correct_count=optional_partially_correct_count, + optional_incorrect_count=optional_incorrect_count, + optional_unknown_count=optional_unknown_count) @transaction.atomic @@ -1959,6 +2019,10 @@ def view_flow_page(pctx, flow_session_id, ordinal): args["max_points"] = fpctx.page.max_points(fpctx.page_data) args["page_expect_answer_and_gradable"] = True + if fpctx.page.is_optional_page: + assert not getattr(args, "max_points", None) + args["is_optional_page"] = True + return render_course_page( pctx, "course/flow-page.html", args, allow_instant_flow_requests=False) @@ -2419,15 +2483,10 @@ def finish_flow_session_view(pctx, flow_session_id): answer_visits = assemble_answer_visits(flow_session) # type: List[Optional[FlowPageVisit]] # noqa - (answered_page_data_list, unanswered_page_data_list) =\ + (answered_page_data_list, unanswered_page_data_list, is_interactive_flow) =\ get_session_answered_page_data( fctx, flow_session, answer_visits) - answered_count = len(answered_page_data_list) - unanswered_count = len(unanswered_page_data_list) - - is_interactive_flow = bool(answered_count + unanswered_count) # type: bool - if flow_permission.view not in access_rule.permissions: raise PermissionDenied() @@ -2557,6 +2616,11 @@ def finish_flow_session_view(pctx, flow_session_id): else: # confirm ending flow + answered_count = len(answered_page_data_list) + unanswered_count = len(unanswered_page_data_list) + required_count = answered_count + unanswered_count + session_may_generate_grade = ( + grading_rule.generates_grade and required_count) return render_finish_response( "course/flow-confirm-completion.html", last_page_nr=flow_session.page_count-1, @@ -2564,7 +2628,8 @@ def finish_flow_session_view(pctx, flow_session_id): answered_count=answered_count, unanswered_count=unanswered_count, unanswered_page_data_list=unanswered_page_data_list, - total_count=answered_count+unanswered_count) + required_count=required_count, + session_may_generate_grade=session_may_generate_grade) # }}} diff --git a/course/models.py b/course/models.py index 6f0277df..b0256e2a 100644 --- a/course/models.py +++ b/course/models.py @@ -586,6 +586,7 @@ def add_default_roles_and_permissions(course, rpm(role=role, permission=pp.view_gradebook).save() rpm(role=role, permission=pp.assign_grade).save() rpm(role=role, permission=pp.view_grader_stats).save() + rpm(role=role, permission=pp.batch_download_submission).save() rpm(role=role, permission=pp.impose_flow_session_deadline).save() rpm(role=role, permission=pp.end_flow_session).save() @@ -622,7 +623,6 @@ def add_default_roles_and_permissions(course, rpm(role=role, permission=pp.edit_grading_opportunity).save() rpm(role=role, permission=pp.batch_import_grade).save() rpm(role=role, permission=pp.batch_export_grade).save() - rpm(role=role, permission=pp.batch_download_submission).save() rpm(role=role, permission=pp.batch_impose_flow_session_deadline).save() rpm(role=role, permission=pp.batch_end_flow_session).save() diff --git a/course/page/base.py b/course/page/base.py index a5742323..8c66cb23 100644 --- a/course/page/base.py +++ b/course/page/base.py @@ -337,6 +337,7 @@ class PageBase(object): # }}} self.page_desc = page_desc + self.is_optional_page = getattr(page_desc, "is_optional_page", False) else: from warnings import warn @@ -366,6 +367,7 @@ class PageBase(object): return ( ("access_rules", Struct), + ("is_optional_page", bool), ) def get_modified_permissions_for_page(self, permissions): @@ -731,6 +733,21 @@ class PageBaseWithTitle(PageBase): class PageBaseWithValue(PageBase): + def __init__(self, vctx, location, page_desc): + super(PageBaseWithValue, self).__init__(vctx, location, page_desc) + + if vctx is not None: + if not hasattr(page_desc, "value"): + vctx.add_warning( + location, + _("Attribute 'value' is not set, default " + "value '1' is used.")) + if hasattr(page_desc, "value") and self.is_optional_page: + vctx.add_warning( + location, + _("Attribute 'value' is ignored when " + "'is_optional_page' is True.")) + def allowed_attrs(self): return super(PageBaseWithValue, self).allowed_attrs() + ( ("value", (int, float)), @@ -740,6 +757,8 @@ class PageBaseWithValue(PageBase): return True def max_points(self, page_data): + if self.is_optional_page: + return 0 return getattr(self.page_desc, "value", 1) diff --git a/course/page/choice.py b/course/page/choice.py index b25ddaf4..7db289b2 100644 --- a/course/page/choice.py +++ b/course/page/choice.py @@ -193,6 +193,10 @@ class ChoiceQuestion(ChoiceQuestionBase): ``ChoiceQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| @@ -328,6 +332,10 @@ class MultipleChoiceQuestion(ChoiceQuestionBase): ``MultipleChoiceQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| @@ -568,6 +576,10 @@ class SurveyChoiceQuestion(PageBaseWithTitle): ``ChoiceQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| diff --git a/course/page/code.py b/course/page/code.py index 44c21731..b00355ff 100644 --- a/course/page/code.py +++ b/course/page/code.py @@ -312,6 +312,10 @@ class PythonCodeQuestion(PageBaseWithTitle, PageBaseWithValue): ``PythonCodeQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| diff --git a/course/page/inline.py b/course/page/inline.py index d1fb92b3..d912f752 100644 --- a/course/page/inline.py +++ b/course/page/inline.py @@ -521,6 +521,10 @@ class InlineMultiQuestion(TextQuestionBase, PageBaseWithValue): ``InlineMultiQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| diff --git a/course/page/text.py b/course/page/text.py index 33b964d6..cb08c1b8 100644 --- a/course/page/text.py +++ b/course/page/text.py @@ -632,6 +632,10 @@ class TextQuestionBase(PageBaseWithTitle): ``TextQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| @@ -751,6 +755,10 @@ class SurveyTextQuestion(TextQuestionBase): ``TextQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| @@ -810,6 +818,10 @@ class TextQuestion(TextQuestionBase, PageBaseWithValue): ``TextQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| @@ -986,6 +998,10 @@ class HumanGradedTextQuestion(TextQuestionBase, PageBaseWithValue, ``HumanGradedTextQuestion`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| diff --git a/course/page/upload.py b/course/page/upload.py index d4c066ae..22e018d2 100644 --- a/course/page/upload.py +++ b/course/page/upload.py @@ -97,6 +97,10 @@ class FileUploadQuestion(PageBaseWithTitle, PageBaseWithValue, ``Page`` + .. attribute:: is_optional_page + + |is-optional-page-attr| + .. attribute:: access_rules |access-rules-page-attr| diff --git a/course/templates/course/flow-completion-grade.html b/course/templates/course/flow-completion-grade.html index 2cd4734b..84a131a5 100644 --- a/course/templates/course/flow-completion-grade.html +++ b/course/templates/course/flow-completion-grade.html @@ -8,74 +8,99 @@ {% block content %} - {% if grade_info != None and grade_info.total_count %} -

{% trans "Results" %}: {{flow_desc.title}}

-
-

- {# Translators: the following 5 blocks of literals make a sentence. PartI #} - {% trans "You have" %} - {% if grade_info.max_points != grade_info.max_reachable_points %} - {# Translators: PartII #} - {% trans "(so far)" %} - {% endif %} - {# Translators: PartIII #} - {% blocktrans trimmed with provisional_points=grade_info.provisional_points|floatformat max_points=grade_info.max_points|floatformat %} - achieved {{ provisional_points }} out of - {{ max_points }} - points. - {% endblocktrans %} - {% if grade_info.max_points != grade_info.max_reachable_points %} - {# Translators: PartIV #} - {% blocktrans trimmed %} - (Some questions are not graded yet, so your grade will likely - change.) - {% endblocktrans %} - {% else %} - {# Translators: PartV #} - {% blocktrans trimmed with points_percent=grade_info.points_percent|floatformat %} - (That's {{ points_percent }}%.) - {% endblocktrans %} - {% endif %} -

- - {% if grade_info.total_points_percent < 100.001 %} - {# otherwise we'll have trouble drawing the bar #} - -
-
-
-
-
- {% endif %} - -

- {% blocktrans trimmed with fully_correct_count=grade_info.fully_correct_count partially_correct_count=grade_info.partially_correct_count incorrect_count=grade_info.incorrect_count %} - You have answered {{ fully_correct_count }} questions - correctly, {{ partially_correct_count }} questions - partially correctly, and {{ incorrect_count }} questions - incorrectly. - {% endblocktrans %} - {% if grade_info.unknown_count %} - {% blocktrans trimmed with unknown_count=grade_info.unknown_count %} - The grade for {{ unknown_count }} questions is not yet known. - {% endblocktrans %} - {% endif %} -

- -
-
-
-
-
-
-
+ {% if grade_info != None %} + {% if grade_info.total_count or grade_info.optional_total_count %} +

{% trans "Results" %}: {{flow_desc.title}}

+
+ {% if grade_info.total_count %} +

+ {# Translators: the following 5 blocks of literals make a sentence. PartI #} + {% trans "You have" %} + {% if grade_info.max_points != grade_info.max_reachable_points %} + {# Translators: PartII #} + {% trans "(so far)" %} + {% endif %} + {# Translators: PartIII #} + {% blocktrans trimmed with provisional_points=grade_info.provisional_points|floatformat max_points=grade_info.max_points|floatformat %} + achieved {{ provisional_points }} out of + {{ max_points }} + points. + {% endblocktrans %} + {% if grade_info.max_points != grade_info.max_reachable_points %} + {# Translators: PartIV #} + {% blocktrans trimmed %} + (Some questions are not graded yet, so your grade will likely + change.) + {% endblocktrans %} + {% else %} + {# Translators: PartV #} + {% blocktrans trimmed with points_percent=grade_info.points_percent|floatformat %} + (That's {{ points_percent }}%.) + {% endblocktrans %} + {% endif %} +

+ {% if grade_info.total_points_percent < 100.001 %} + {# otherwise we'll have trouble drawing the bar #} +
+
+
+
+
+ {% endif %} +

+ {% blocktrans trimmed with fully_correct_count=grade_info.fully_correct_count partially_correct_count=grade_info.partially_correct_count incorrect_count=grade_info.incorrect_count %} + You have answered {{ fully_correct_count }} grading questions + correctly, {{ partially_correct_count }} questions + partially correctly, and {{ incorrect_count }} questions + incorrectly. + {% endblocktrans %} + {% if grade_info.unknown_count %} + {% blocktrans trimmed with unknown_count=grade_info.unknown_count %} + The grade for {{ unknown_count }} questions is not yet known. + {% endblocktrans %} + {% endif %} +

+
+
+
+
+
+
+ {% endif %} + {% if grade_info.optional_total_count %} +

+ {% blocktrans trimmed with total_count=grade_info.optional_total_count fully_correct_count=grade_info.optional_fully_correct_count partially_correct_count=grade_info.optional_partially_correct_count incorrect_count=grade_info.optional_incorrect_count %} + There are {{ total_count }} optional questions (not for grading). + You have answered {{ fully_correct_count }} correctly, + {{ partially_correct_count }} partially correctly, + and {{ incorrect_count }} incorrectly. + {% endblocktrans %} + {% if grade_info.optional_unknown_count %} + {% blocktrans trimmed with unknown_count=grade_info.optional_unknown_count %} + The correctness for {{ unknown_count }} optional questions is not yet known. + {% endblocktrans %} + {% endif %} +

+
+
+
+
+
+
+ {% endif %} +
+ {% endif %} {% endif %} {{ completion_text|safe }} diff --git a/course/templates/course/flow-confirm-completion.html b/course/templates/course/flow-confirm-completion.html index 44c34912..f2913372 100644 --- a/course/templates/course/flow-confirm-completion.html +++ b/course/templates/course/flow-confirm-completion.html @@ -28,12 +28,12 @@ {% endif %} - {% if total_count %} + {% if required_count %}

{% blocktrans trimmed %} - There were {{total_count}} questions. + There were {{required_count}} grading questions. {% endblocktrans %} - {% if answered_count == total_count %} + {% if answered_count == required_count %} {% blocktrans trimmed %} You have provided an answer for all of them. {% endblocktrans %} @@ -48,8 +48,10 @@ {% trans "If you choose to end your session, the following things will happen:" %}

diff --git a/course/templates/course/flow-page.html b/course/templates/course/flow-page.html index 3c78a656..1e4e5ba5 100644 --- a/course/templates/course/flow-page.html +++ b/course/templates/course/flow-page.html @@ -317,6 +317,10 @@ {{ max_points }} points {% endblocktrans %}
+ {% elif is_optional_page %} +
+ {% trans "Optional question" %} +
{% endif %} {# }}} #} diff --git a/course/views.py b/course/views.py index 770843b7..f1edc651 100644 --- a/course/views.py +++ b/course/views.py @@ -1006,6 +1006,9 @@ class ExceptionStage3Form(StyledForm): initial=default_data.get("due"), label=_("Due time")) + self.fields["generates_grade"] = forms.BooleanField(required=False, + initial=default_data.get("generates_grade", True), + label=_("Generates grade")) self.fields["credit_percent"] = forms.FloatField(required=False, initial=default_data.get("credit_percent"), label=_("Credit percent")) @@ -1021,6 +1024,7 @@ class ExceptionStage3Form(StyledForm): layout.append(Div("create_grading_exception", "due_same_as_access_expiration", "due", + "generates_grade", "credit_percent", "bonus_points", "max_points", "max_points_enforced_cap", css_class="well")) @@ -1163,7 +1167,7 @@ def grant_exception_stage_3(pctx, participation_id, flow_id, session_id): new_grading_rule["if_completed_before"] = due_local_naive for attr_name in ["credit_percent", "bonus_points", - "max_points", "max_points_enforced_cap"]: + "max_points", "max_points_enforced_cap", "generates_grade"]: if form.cleaned_data[attr_name] is not None: new_grading_rule[attr_name] = form.cleaned_data[attr_name] @@ -1171,10 +1175,6 @@ def grant_exception_stage_3(pctx, participation_id, flow_id, session_id): and session.access_rules_tag is not None): new_grading_rule["if_has_tag"] = session.access_rules_tag - if hasattr(grading_rule, "generates_grade"): - new_grading_rule["generates_grade"] = \ - grading_rule.generates_grade - validate_session_grading_rule( vctx, ugettext("newly created exception"), dict_to_struct(new_grading_rule), tags, @@ -1205,9 +1205,13 @@ def grant_exception_stage_3(pctx, participation_id, flow_id, session_id): else: data = { "restrict_to_same_tag": session.access_rules_tag is not None, - "credit_percent": grading_rule.credit_percent, #"due_same_as_access_expiration": True, "due": grading_rule.due, + "generates_grade": grading_rule.generates_grade, + "credit_percent": grading_rule.credit_percent, + "bonus_points": grading_rule.bonus_points, + "max_points": grading_rule.max_points, + "max_points_enforced_cap": grading_rule.max_points_enforced_cap, } for perm in access_rule.permissions: data[perm] = True diff --git a/doc/flow.rst b/doc/flow.rst index fa8aecf0..3b6f0569 100644 --- a/doc/flow.rst +++ b/doc/flow.rst @@ -668,6 +668,12 @@ The following page types are predefined: An integer or a floating point number, representing the point value of the question. +.. |is-optional-page-attr| replace:: + + Optional. A Boolean value indicating whether the page is an optional page + which does not require answer for fully completion of the flow. + If `true`, :attr:`value` will be igored. Defaults to `false` if not present. + .. |text-widget-page-attr| replace:: Optional. -- GitLab From ff4e2922353f294ec426689831ea6add05e27e8f Mon Sep 17 00:00:00 2001 From: dzhuang Date: Mon, 13 Feb 2017 10:00:06 +0800 Subject: [PATCH 2/2] No warning for PageBaseWithValue if value is absent and is_optional_page is True --- course/page/base.py | 9 ++------- doc/flow.rst | 6 +++++- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/course/page/base.py b/course/page/base.py index 8c66cb23..d813dbc8 100644 --- a/course/page/base.py +++ b/course/page/base.py @@ -737,15 +737,10 @@ class PageBaseWithValue(PageBase): super(PageBaseWithValue, self).__init__(vctx, location, page_desc) if vctx is not None: - if not hasattr(page_desc, "value"): - vctx.add_warning( - location, - _("Attribute 'value' is not set, default " - "value '1' is used.")) if hasattr(page_desc, "value") and self.is_optional_page: - vctx.add_warning( + raise ValidationError( location, - _("Attribute 'value' is ignored when " + _("Attribute 'value' should be removed when " "'is_optional_page' is True.")) def allowed_attrs(self): diff --git a/doc/flow.rst b/doc/flow.rst index 3b6f0569..5a5b25cd 100644 --- a/doc/flow.rst +++ b/doc/flow.rst @@ -672,7 +672,11 @@ The following page types are predefined: Optional. A Boolean value indicating whether the page is an optional page which does not require answer for fully completion of the flow. - If `true`, :attr:`value` will be igored. Defaults to `false` if not present. + If ``true``, :attr:`value` should not present. Defaults to ``false`` if not present. + Note that ``is_optional_page: true`` differs from ``value: 0`` in that finishing flows + with unanswered page(s) with the latter will be warned of "unanswered question(s)", + while with the former won't. When using not-for-grading page(s) to collect + answers from students, it's to better use ``value: 0``. .. |text-widget-page-attr| replace:: -- GitLab