Fix capa report generation issue #264

HamzaIbnFarooq · 2021-05-28T11:00:58Z

Description

Fixes Capa questions report generation issues by providing a default value for the problems having no text values for answer options.

Related Issues

#262
#239

How to test?

Test 1:

Generate the issue without this branch's fix:

Create a Checkbox Problem on studio
Don't put any text against one of the options
Through LMS select that option with no text
Go to Data Download under Instructor tab
Under the Reports section, select the Problem and press Create a report of problem responses
The error will be generated.

Now use this branch and try downloading the report again, it should be fixed now.

Test 2:

Generate the issue without this branch's fix:

Create a checkboxes problem
Add N options under that problem (where N is a number)
Go to LMS, select and submit Nth option
Go back to the studio and update the question by removing the Nth option (now the total options will be N-1)
Try generating a response report for that problem, it will give Error: There was an error generating your report. error.

Now use this branch and try downloading the report again, it should be fixed now.

In the current state if it is unable to find the answer text for a given response (maybe because it was skipped?) then the report will fail to generate. Adding in a conditional with placeholder text allows the report to generate rather than completely failing to provide this data to the course instructor.

ziafazal

@HamzaIbnFarooq completed first round of review. I'd recommend you add units tests too so we can have a single upstream PR.

ziafazal · 2021-05-31T10:54:21Z

common/lib/capa/capa/capa_problem.py

@@ -657,7 +657,8 @@ def find_answer_text(self, answer_id, current_answer):
        if isinstance(current_answer, list):
            # Multiple answers. This case happens e.g. in multiple choice problems
            answer_text = ", ".join(
-                self.find_answer_text(answer_id, answer) for answer in current_answer
+                self.find_answer_text(answer_id, answer) or 'No Answer Recorded'


we should add logic in find_answer_text to check if there is no text for answered choice it should return Answer Text Missing.

ziafazal · 2021-05-31T11:04:51Z

common/lib/capa/capa/capa_problem.py

-            input_cls = inputtypes.registry.get_class_for_tag(choicegroup.tag)
-            choices_map = dict(input_cls.extract_choices(choicegroup, self.capa_system.i18n, text_only=True))
-            answer_text = choices_map[current_answer]
+            if len(elems) == 1:


we need to check all cases

we have not choice found

we have 1 choice

we have more than 1 choices found

if len(elems) == 0: log.warning("Answer Text Missing for answer id: %s and choice number: %s", answer_id, current_answer) answer_text = 'Answer Text Missing' elif len(elems) == 0: {current implementation} else log.warning("Multiple answers found for answer id: %s and choice number: %s", answer_id, current_answer) answer_text = 'Multiple answers found'

HamzaIbnFarooq · 2021-05-31T13:03:36Z

I'd recommend you add units tests too so we can have a single upstream PR.

@ziafazal we are not planning this PR to be an upstream one, we will be using this locally right now. We will be creating an upstream of #265 only, once that gets reviewed.

HamzaIbnFarooq · 2021-06-01T11:37:50Z

@pdpinch Do you think we should create an upstream PR of fixing old data for capa reports generation and add markdown validation in capa problems combined
OR
stick with the original plan as mentioned here saying that only add markdown validation in capa problems should be upstream.

pdpinch · 2021-06-01T13:12:46Z

My first priority is to get the Residential MITx teams access to their data, but yes we should plan on upstreaming both of these fixes to edx master.

…

@pdpinch <https://github.com/pdpinch> Do you think we should create an upstream PR of fixing old data for capa reports generation <#264> and add markdown validation in capa problems <#265> combined OR stick with the original plan as mentioned here <#239 (comment)> saying that only add markdown validation in capa problems <#265> should be upstream.

ziafazal

👍

HamzaIbnFarooq · 2021-06-01T13:36:06Z

My first priority is to get the Residential MITx teams access to their data, but yes we should plan on upstreaming both of these fixes to edx master.

@pdpinch you can have a look at the current PR, if it seems good to you then we can merge it right away. I can cherry-pick the commits to create an upstream PR and add some tests afterwards.

pdpinch · 2021-06-02T13:15:11Z

@mitodl/devops can we try deploying this change to lms.mitx.mit.edu to see if it helps address ZenDesk 96001

blarghmatey · 2021-06-02T13:37:06Z

Once it's merged we can run a build to get this through to production. Code looks good to me 👍

HamzaIbnFarooq · 2021-06-04T11:06:31Z

@blarghmatey the current branch's fix was removed from mitx/koa most probably due to a force push (maybe yesterday), so whenever it's feasible for you, can you push the commit back to mitx/koa to reflect the changes?

In the current state if CAPA problem response report generation is unable to find the answer text for a given response then the report will fail to generate. Adding a placeholder text allows the report to generate rather than completely failing to provide this data to the course instructor. (cherry picked from commit e94801661a3b964276f7dcac331c39057a095797)

This PR adds MFE API. This is part of the work that is being done to obtain the MFE Runtime Configurations and that has been discussed in the BTR WG. Discussion: https://discuss.openedx.org/t/how-to-use-microfrontend-in-a-multitenant-instance/6936/14?u=mafermazu MFE Runtime configuration - eduNEXT: https://docs.google.com/document/d/1-FHIQmyeQZu3311x8eYUNMru4JX7Yb3UlqjmJxvM8do/edit?usp=sharing feat: add lms setting to set mfe config cache (#262) Co-authored-by: María Fernanda Magallanes Z <maria.magallanes@edunext.co> feat: make mfe config api disabled by default (#263) * feat: make mfe config api disabled by default * fix: simple is better than complex test: add mfe config tests (#264) * test: add mfe config tests * test: fix it and simplify it * test: correct pylint issues fix: correct pep 8 violations fix: add mfe api unit test in github workflow fix: correct unit tests refactor: move mfe api to lms fix: try mfe api urls without regex fix: add app_namespace in lms urls fix: try url without conditional Revert "fix: try url without conditional" This reverts commit 694aab546134b4bd9ad2642e24927b42cac24459. fix: set enable_mfe_config_api feature to true in the tests test: try to add failed test case Revert "test: try to add failed test case" This reverts commit cee6bf656ab1b96492b0b6199ddff32a6d6a65bd. docs: improve explanation and documentation fix: ensure the response is a json object refactor: be consistent with the variable names fix: allow overriding mfe api config cache timeout in production fix: handle 404 response in view refactor: use a guard instead if-else feat: add the possibility to show mfe specific config

blarghmatey and others added 2 commits May 28, 2021 12:44

fix: add default value to capa problem answers

5f53517

HamzaIbnFarooq added the Needs Review label May 31, 2021

HamzaIbnFarooq requested review from arslanashraf7 and ziafazal May 31, 2021 06:48

ziafazal suggested changes May 31, 2021

View reviewed changes

addressing feedback

00b699e

HamzaIbnFarooq requested a review from ziafazal May 31, 2021 13:03

ziafazal approved these changes Jun 1, 2021

View reviewed changes

HamzaIbnFarooq merged this pull request into mitx/koa Jun 3, 2021

HamzaIbnFarooq self-assigned this Jun 3, 2021

HamzaIbnFarooq removed the Needs Review label Jun 3, 2021

HamzaIbnFarooq mentioned this pull request Jun 7, 2021

add validation to capa problem authoring markdown #265

Closed

pdpinch mentioned this pull request Jun 10, 2021

Adding in support for missing answer text in CAPA problem reports #239

Closed

HamzaIbnFarooq mentioned this pull request Jul 13, 2021

adding validation to capa problem markdown #240

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix capa report generation issue #264

Fix capa report generation issue #264

HamzaIbnFarooq commented May 28, 2021 •

edited

Loading

ziafazal left a comment

ziafazal May 31, 2021

HamzaIbnFarooq May 31, 2021

ziafazal May 31, 2021

HamzaIbnFarooq May 31, 2021

HamzaIbnFarooq commented May 31, 2021

HamzaIbnFarooq commented Jun 1, 2021

pdpinch commented Jun 1, 2021 via email

ziafazal left a comment

HamzaIbnFarooq commented Jun 1, 2021

pdpinch commented Jun 2, 2021 •

edited

Loading

blarghmatey commented Jun 2, 2021

HamzaIbnFarooq commented Jun 4, 2021

Fix capa report generation issue #264

Fix capa report generation issue #264

Conversation

HamzaIbnFarooq commented May 28, 2021 • edited Loading

Description

Related Issues

How to test?

Test 1:

Test 2:

ziafazal left a comment

Choose a reason for hiding this comment

ziafazal May 31, 2021

Choose a reason for hiding this comment

HamzaIbnFarooq May 31, 2021

Choose a reason for hiding this comment

ziafazal May 31, 2021

Choose a reason for hiding this comment

HamzaIbnFarooq May 31, 2021

Choose a reason for hiding this comment

HamzaIbnFarooq commented May 31, 2021

HamzaIbnFarooq commented Jun 1, 2021

pdpinch commented Jun 1, 2021 via email

ziafazal left a comment

Choose a reason for hiding this comment

HamzaIbnFarooq commented Jun 1, 2021

pdpinch commented Jun 2, 2021 • edited Loading

blarghmatey commented Jun 2, 2021

HamzaIbnFarooq commented Jun 4, 2021

HamzaIbnFarooq commented May 28, 2021 •

edited

Loading

pdpinch commented Jun 2, 2021 •

edited

Loading