Add nesting of questions in subfolders (#2489)

* allow recursive question folders

* fix semicolon

* fail on directories without questions

* add nested question to test course

* add nested question tests

* Update questionsSync.js

* add example question to hw1

* add example question to test

* annoying

* Update course-db.js

* update docs

* create news item

* Update docs/question.md

Co-authored-by: Matthew West <mwest@illinois.edu>

* renumber news item

* add similar comment to the docs about assessments and course instances

* update news item

* renumber news item

* update news item

Co-authored-by: Matthew West <mwest@illinois.edu>

docs/assessment.md

@@ -5,7 +5,7 @@
 
 ## Overview
 
-Each assessment is a single directory in the `assessments` folder. The directory must contain a single file called `infoAssessment.json` that describes the assessment and looks like:
+Each assessment is a single directory in the `assessments` folder or any subfolder.  Assessments may be nested in subdirectories of the `assessments` folder.  The assessment directory must contain a single file called `infoAssessment.json` that describes the assessment and looks like:
 
 ```json
 {
@@ -19,6 +19,8 @@ Each assessment is a single directory in the `assessments` folder. The directory
 }
 ```
 
+The assessment ID is the full path relative to `assessments`.
+
 * [Format specification for assessment `infoAssessment.json`](https://github.com/PrairieLearn/PrairieLearn/blob/master/schemas/schemas/infoAssessment.json)
 
 ## Assessment naming

docs/courseInstance.md

@@ -5,7 +5,7 @@
 
 ## Directory layout
 
-A _course instance_ corresponds to a single offering of a [course](course.md), such as "Fall 2016", or possibly "Fall 2016, Section 1". A course instance like `Fa16` is contained in one directory and has a configuration file (`infoCourseInstance.json`) and a subdirectory (`assessments`) containing a list of [assessments](assessment.md). The `assessments` directory should always exist, but may be empty if no assessments have been added.
+A _course instance_ corresponds to a single offering of a [course](course.md), such as "Fall 2016", or possibly "Fall 2016, Section 1". A course instance like `Fa16` is contained in one directory and has a configuration file (`infoCourseInstance.json`) and a subdirectory (`assessments`) containing a list of [assessments](assessment.md). The `assessments` directory should always exist, but may be empty if no assessments have been added.  A course instance may be located in the root `courseInstances` directory, or any subfolder that is not a courseInstance itself.
 
 ```text
 exampleCourse

docs/question.md

@@ -7,7 +7,7 @@
 
 ## Directory structure
 
-Questions are all stored inside the main `questions` directory for a course. Each question is a single directory that contains all the files for that question. The name of the question directory is the question ID label (the `id`) for that question. For example, here are two different questions:
+Questions are all stored inside the `questions` directory (or any subfolder) for a course. Each question is a single directory that contains all the files for that question. The name of the full question directory relative to `questions` is the QID (the "question ID") for that question. For example, here are three different questions:
 
 ```text
 questions
@@ -18,19 +18,26 @@ questions
 |   +-- server.py             # secret server-side code (optional)
 |   `-- question.html         # HTML template for the question
 |
-`-- addVectors                # second question, id is "addVectors"
-    |
-    +-- info.json             # metadata for the addVectors question
-    +-- server.py
-    +-- question.html
-    +-- notes.docx            # more files, like notes on how the question works
-    +-- solution.docx         # these are secret (can't be seen by students)
-    |
-    +-- clientFilesQuestion/  # Files accessible to the client (web browser)
-    |   `-- fig1.png          # A client file (an image)
+|-- addVectors                # second question, id is "addVectors"
+|   |
+|   +-- info.json             # metadata for the addVectors question
+|   +-- server.py
+|   +-- question.html
+|   +-- notes.docx            # more files, like notes on how the question works
+|   +-- solution.docx         # these are secret (can't be seen by students)
+|   |
+|   +-- clientFilesQuestion/  # Files accessible to the client (web browser)
+|   |   `-- fig1.png          # A client file (an image)
+|   |
+|   +-- tests/                # external grading files (see other doc)
+|       `-- ...
+|
+`-- subfolder                 # a subfolder we can put questions in -- this itself can't be a question
     |
-    +-- tests/                # external grading files (see other doc)
-        `-- ...
+    `-- nestedQuestion        # third question, id is "subfolder/nestedQuestion"
+        |
+        +-- info.json         # metadata for the "subfolder/nestedQuestion" question
+        `-- question.html
 ```
 
 PrairieLearn assumes independent questions; nothing ties them together. However, each question could have multiple parts (inputs that are validated together).

lib/course-db.js

@@ -162,45 +162,68 @@ function loadInfoDB(db, idName, parentDir, infoFilename, defaultInfo, schema, op
     // `cache` is an object with which we can cache information derived from course info
     // in between successive calls to `checkInfoValid`
     const cache = {};
-    fs.readdir(parentDir, function(err, files) {
-        if (ERR(err, callback)) return;
 
-        async.each(files, function(dir, callback) {
-            var infoFile = path.join(parentDir, dir, infoFilename);
-            jsonLoad.readInfoJSON(infoFile, schema, function(err, info) {
-                if (err && err.code && err.path && (err.code === 'ENOTDIR') && err.path === infoFile) {
-                    // In a previous version of this code, we'd pre-filter
-                    // all files in the parent directory to remove anything
-                    // that may have accidentally slipped in, like .DS_Store.
-                    // However, that resulted in a huge number of system calls
-                    // that got really slow for large directories. Now, we'll
-                    // just blindly try to read a file from the directory and assume
-                    // that if we see ENOTDIR, that means the directory was not
-                    // in fact a directory.
-                    callback(null);
-                    return;
-                }
-                if (ERR(err, callback)) return;
-                jsonLoad.validateOptions(info, infoFile, optionSchemaPrefix, schemas, function(err, info) {
-                    if (ERR(err, callback)) return;
-                    info[idName] = dir;
-                    info.directory = dir;
-                    err = checkInfoValid(idName, info, infoFile, courseInfo, logger, cache);
-                    if (ERR(err, callback)) return;
-                    if (info.disabled) {
-                        callback(null);
+    let walk = function(relativeDir, callback) {
+        fs.readdir(path.join(parentDir, relativeDir), function(err, files) {
+            if (ERR(err, callback)) return;
+
+            async.map(files, function(dir, callback) {
+                var infoFile = path.join(parentDir, relativeDir, dir, infoFilename);
+                jsonLoad.readInfoJSON(infoFile, schema, function(err, info) {
+                    if (err && err.code && err.path && (err.code === 'ENOTDIR') && err.path === infoFile) {
+                        // In a previous version of this code, we'd pre-filter
+                        // all files in the parent directory to remove anything
+                        // that may have accidentally slipped in, like .DS_Store.
+                        // However, that resulted in a huge number of system calls
+                        // that got really slow for large directories. Now, we'll
+                        // just blindly try to read a file from the directory and assume
+                        // that if we see ENOTDIR, that means the directory was not
+                        // in fact a directory.
+                        callback(null, 0);
                         return;
                     }
-                    info = _.defaults(info, defaultInfo);
-                    db[dir] = info;
-                    return callback(null);
+                    if (err && err.code && (err.code == 'ENOENT') && err.path === infoFile) {
+                        /* Try to recurse into the directory if we don't find an info json file. */
+                        walk(path.join(relativeDir, dir), function(walk_err, num_loaded) {
+                            if (ERR(walk_err, callback)) return;
+                            if (num_loaded == 0) {
+                                /* We didn't actually find any children, so just pass along the error. */
+                                callback(err, 0);
+                            } else {
+                                callback(null, num_loaded);
+                            }
+                        });
+                        return;
+                    }
+                    if (ERR(err, callback)) return;
+                    jsonLoad.validateOptions(info, infoFile, optionSchemaPrefix, schemas, function(err, info) {
+                        if (ERR(err, callback)) return;
+                        info[idName] = path.join(relativeDir, dir);
+                        info.directory = path.join(relativeDir, dir);
+
+                        err = checkInfoValid(idName, info, infoFile, courseInfo, logger, cache);
+                        if (ERR(err, callback)) return;
+                        if (info.disabled) {
+                            return callback(null, 0);
+                        }
+
+                        info = _.defaults(info, defaultInfo);
+                        db[path.join(relativeDir, dir)] = info;
+                        return callback(null, 1);
+                    });
                 });
+            }, function(err, results) {
+                if (ERR(err, callback)) return;
+                let loaded_vals = results.reduce((x, y) => x + y, 0);
+                callback(null, loaded_vals);
             });
-        }, function(err) {
-            if (ERR(err, callback)) return;
-            logger.debug('successfully loaded info from ' + parentDir + ', number of items = ' + _.size(db));
-            callback(null);
         });
+    };
+
+    return walk('', function(err, num_loaded) {
+        if (ERR(err, callback)) return;
+        logger.debug('successfully loaded info from ' + parentDir + ', number of items = ' + num_loaded);
+        callback(err);
     });
 }
 

news_items/014_nested_questions/index.html

@@ -0,0 +1,44 @@
+<style>
+ img {
+     display: block;
+     margin-left: auto;
+     margin-right: auto;
+     margin-top: 15px;
+     margin-bottom: 15px;
+ }
+ .code-block {
+     font-family: SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace;
+     background: #f0f0f0;
+     width: 70%;
+     margin-left: auto;
+     margin-right: auto;
+ }
+ .code-block > pre {
+     padding: 0.5rem;
+     display: block;
+     margin-bottom: 0px;
+     line-height: 125%;
+     overflow: wrap;
+ }
+</style>
+
+<p>
+    Questions now have the ability to be nested inside of subfolders! This long-requested feature was implemented by <a href="https://github.com/nicknytko">Nicolas Nytko</a> (CS BS '19, CS MS '22). Instead of a sprawling <code class="user-output">questions</code> folder inside of your course, you can now organize these into folders however you wish:
+</p>
+
+<img src="nested.png" width="867px">
+
+<p>
+    The QID of the question is now the path from the <code class="user-output">questions</code> folder.  For example, the first question above has a QID of <code class="user-output">demo/ansiOutput</code> and this is how it should be referenced in assessments.  This is also reflected in the "Questions" tab:
+</p>
+
+<img class="border rounded" src="questions.png" width="1016px">
+
+<p>
+    Nested folders have also been enabled under the <code class="user-output">courseInstances</code> and <code class="user-output">assessments</code> folders.
+</p>
+
+<hr class="mt-5">
+<p class="text-right small">
+    Want to help make PrairieLearn better? It's open source and <a href="https://github.com/PrairieLearn/PrairieLearn">contributions are welcome!</a>
+</p>

news_items/014_nested_questions/info.json

@@ -0,0 +1,6 @@
+{
+    "uuid": "244367fd-9a96-4a5b-9db4-9c1fdd0b2ca6",
+    "title": "Question folder nesting",
+    "author": "<a href=\"https://github.com/mwest1066\">Matt West</a>",
+    "visible_to_students": false
+}

testCourse/courseInstances/Sp15/assessments/hw2-miscProblems/infoAssessment.json

@@ -37,7 +37,8 @@
                 {"id": "fossilFuelsRadio",           "points": 2, "maxPoints": 10},
                 {"id": "positionTimeGraph",          "points": 2, "maxPoints": 10},
                 {"id": "downloadFile",               "points": 2, "maxPoints": 10},
-                {"id": "differentiatePolynomial",    "points": 2, "maxPoints": 10}
+                {"id": "differentiatePolynomial",    "points": 2, "maxPoints": 10},
+                {"id": "subfolder/nestedQuestion",   "points": 2, "maxPoints": 10}
             ]
         }
     ]

testCourse/questions/subfolder/nestedQuestion/info.json

@@ -0,0 +1,7 @@
+{
+    "uuid": "5fa17ff4-20a9-4e93-af03-f4e52b4cc485",
+    "title": "Nested question",
+    "topic": "Algebra",
+    "tags": ["nnytko2", "sp20", "v3"],
+    "type": "v3"
+}

testCourse/questions/subfolder/nestedQuestion/question.html

@@ -0,0 +1,5 @@
+<pl-question-panel>
+    <p>
+        This question demonstrates functionality to nest questions in subfolders.  The qid of the question then becomes its full path relative to <code class="user-output">questions</code>.  This specific question has a qid of <code class="user-output">subfolder/nestedQuestion</code>.
+    </p>
+</pl-question-panel>

testCourse/questions/subfolder/nestedQuestion/server.py

@@ -0,0 +1,2 @@
+def grade(data):
+    data['score'] = 1.0

tests/sync/courseInstancesSync.js

@@ -13,7 +13,7 @@ describe('Course instance syncing', () => {
   beforeEach('set up testing database', helperDb.before);
   afterEach('tear down testing database', helperDb.after);
 
-  it('soft-deletes and restores course instnaces', async () => {
+  it('soft-deletes and restores course instances', async () => {
     const { courseData, courseDir } = await util.createAndSyncCourseData();
     const originalCourseInstance = courseData.courseInstances[util.COURSE_INSTANCE_ID];
     let syncedCourseInstances = await util.dumpTable('course_instances');

tests/sync/questionsSync.js

@@ -106,4 +106,29 @@ describe('Question syncing', () => {
     await fs.ensureDir(path.join(courseDir, 'questions', 'badQuestion'));
     await assert.isRejected(util.syncCourseData(courseDir), /ENOENT/);
   });
+
+  it('allows arbitrary nesting of questions in subfolders', async() => {
+    const courseData = util.getCourseData();
+    const courseDir = await util.writeCourseToTempDirectory(courseData);
+    const nestedQuestionStructure = ['subfolder1', 'subfolder2', 'subfolder3', 'subfolder4', 'subfolder5', 'subfolder6', 'nestedQuestion'];
+    const nestedQid = path.join(...nestedQuestionStructure);
+    const questionPath = path.join(courseDir, 'questions', nestedQid);
+
+    await fs.ensureDir(questionPath);
+    await fs.copyFile(path.join(courseDir, 'questions', util.QUESTION_ID, 'info.json'), path.join(questionPath, 'info.json'));
+    await fs.rmdir(path.join(courseDir, 'questions', util.QUESTION_ID), {'recursive': true});
+
+    await util.syncCourseData(courseDir);
+  });
+
+  it('fails if a nested question directory does not eventually contain an info.json file', async() => {
+    const courseData = util.getCourseData();
+    const courseDir = await util.writeCourseToTempDirectory(courseData);
+    const nestedQuestionStructure = ['subfolder1', 'subfolder2', 'subfolder3', 'subfolder4', 'subfolder5', 'subfolder6', 'badQuestion'];
+    const nestedQid = path.join(...nestedQuestionStructure);
+    const questionPath = path.join(courseDir, 'questions', nestedQid);
+
+    await fs.ensureDir(questionPath);
+    await assert.isRejected(util.syncCourseData(courseDir), /ENOENT/);
+  });
 });