search

mitodl / edx-platform

The Open edX platform, the software that powers edX!
http://open.edx.org/
GNU Affero General Public License v3.0
6 stars 1 forks source link

adding validation to capa problem markdown #240

Closed HamzaIbnFarooq closed 3 years ago

HamzaIbnFarooq commented 3 years ago

Related Issue

https://github.com/mitodl/edx-platform/pull/239

What this PR does

It adds validation to CAPA problems and shows a notification to the course author regarding the issue.

How to test this PR

To understand the purpose of this PR, follow the below steps to reproduce the issue:

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

Now use the current PR and go through the same process, and you will see that the editor will not allow saving such content and mentions an error message instead.

Screenshots

image

pdpinch commented 3 years ago

I think this is a good approach to avoid the problem in the future, but let me check with some other folks.

What do we need to do to address existing bad data? Do you think that editing the course code is sufficient to unblock the data download?

HamzaIbnFarooq commented 3 years ago

What do we need to do to address existing bad data? Do you think that editing the course code is sufficient to unblock the data download?

As per my understanding, editing the course content (through the studio) is not a solution for pre-existing data. The simplest and promising solution I can see right now is the one done by @blarghmatey (at https://github.com/mitodl/edx-platform/pull/239/commits/b580ed10b393d9d76b5da94a68408c8693350a51). It will fix the report generation (don't know why it didn't work for you in the first place but I still think that's a good solution).

HamzaIbnFarooq commented 3 years ago

Closing this PR as its similar PR has been merged upstream https://github.com/edx/edx-platform/pull/27858 and we should be cherry-picking that code in our lilac release if we want to.

Update: as we are not moving to lilac right away and using mitx/koa for a while so this PR is required to be merged. Although the PR got contaminated with some unnecessary commits so I will be removing those commits now.

HamzaIbnFarooq commented 3 years ago

FYI: The current PR https://github.com/mitodl/edx-platform/pull/240 and its sibling https://github.com/mitodl/edx-platform/pull/264 has been combined and merged upstream https://github.com/edx/edx-platform/pull/27858