From e61b97148976112a9c3a9f11b3fd1bde182912f3 Mon Sep 17 00:00:00 2001
From: Sarah Rietkerk <sarahnrietkerk@gmail.com>
Date: Wed, 31 Jan 2024 15:28:16 -0800
Subject: [PATCH] fine-tuned the comment validator, split out the logic for
 comments on specific blocks

---
 docs/teachertool/catalog-shared.json          |  8 +++++++-
 docs/teachertool/validator-plans-shared.json  | 13 ++++++++++++-
 .../code-validation/runValidatorPlanAsync.ts  | 13 ++++++++-----
 .../code-validation/validateCommentsExist.ts  | 19 ++++++++++++++++---
 pxtblocks/code-validation/validatorPlans.ts   |  2 +-
 5 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/docs/teachertool/catalog-shared.json b/docs/teachertool/catalog-shared.json
index 832c1f32d510..93eb173e8ef1 100644
--- a/docs/teachertool/catalog-shared.json
+++ b/docs/teachertool/catalog-shared.json
@@ -13,10 +13,16 @@
             "docPath": "/teachertool"
         },
         {
-            "id": "49262A2B-C02A-43A2-BAD5-FCAC6AE9D465",
+            "id": "76D86387-E3CF-4BEB-B62C-B811F5997631",
             "use": "block_comment_used",
             "template": "At least one comment was left on a block in the code",
             "docPath": "/teachertool"
+        },
+        {
+            "id": "8F97C9A6-CF16-48B4-A84F-3105C24B20DE",
+            "use": "functions_have_comments",
+            "template": "All function definitions have block comments",
+            "docPath": "/teachertool"
         }
     ]
 }
\ No newline at end of file
diff --git a/docs/teachertool/validator-plans-shared.json b/docs/teachertool/validator-plans-shared.json
index 858d0a513dee..f098e8620ed9 100644
--- a/docs/teachertool/validator-plans-shared.json
+++ b/docs/teachertool/validator-plans-shared.json
@@ -32,7 +32,7 @@
             ]
         },
         {
-            ".desc": "A comment exists in a project",
+            ".desc": "A block comment exists in a project",
             "name": "block_comment_used",
             "threshold": 1,
             "checks": [
@@ -40,6 +40,17 @@
                     "validator": "blockCommentExists"
                 }
             ]
+        },
+        {
+            ".desc": "All function definitions have block comments",
+            "name": "functions_have_comments",
+            "threshold": 1,
+            "checks": [
+                {
+                    "validator": "blockCommentExists",
+                    "blockType": "function_definition"
+                }
+            ]
         }
     ]
 }
diff --git a/pxtblocks/code-validation/runValidatorPlanAsync.ts b/pxtblocks/code-validation/runValidatorPlanAsync.ts
index 7cf5036acf40..7d6adb48420b 100644
--- a/pxtblocks/code-validation/runValidatorPlanAsync.ts
+++ b/pxtblocks/code-validation/runValidatorPlanAsync.ts
@@ -10,7 +10,7 @@ namespace pxt.blocks {
                 case "blocksExist":
                     return runBlocksExistValidation(usedBlocks, check as BlocksExistValidatorCheck);
                 case "blockCommentExists":
-                    return validateCommentsExist(usedBlocks);
+                    return runValidateBlockCommentsExist(usedBlocks, check as BlockCommentExistsValidatorCheck);
                 default:
                     pxt.debug(`Unrecognized validator: ${check.validator}`);
                     return false;
@@ -38,9 +38,12 @@ namespace pxt.blocks {
         return success;
     }
 
-    function runValidateCommentsExist(usedBlocks: Blockly.Block[], inputs: BlockCommentExistsValidatorCheck): boolean {
-        const blockResults = validateCommentsExist(usedBlocks);
-
-        return true;
+    function runValidateBlockCommentsExist(usedBlocks: Blockly.Block[], inputs: BlockCommentExistsValidatorCheck): boolean {
+        const blockTypeUsed = inputs.blockType;
+        const blockResults = blockTypeUsed ?
+            validateCommentsOnSpecificBlocksExist({ usedBlocks, blockType: blockTypeUsed }) :
+            validateBlockCommentsExist({ usedBlocks });
+        const success = blockTypeUsed ? blockResults.length === 0 : blockResults.length !== 0;
+        return success;
     }
 }
diff --git a/pxtblocks/code-validation/validateCommentsExist.ts b/pxtblocks/code-validation/validateCommentsExist.ts
index 991dc6308ce4..4219513dc2d9 100644
--- a/pxtblocks/code-validation/validateCommentsExist.ts
+++ b/pxtblocks/code-validation/validateCommentsExist.ts
@@ -1,7 +1,20 @@
 
 namespace pxt.blocks {
-    export function validateCommentsExist(usedBlocks: Blockly.Block[]): boolean {
-        const commentBlocks = usedBlocks.filter((block) => !!block.comment);
-        return commentBlocks.length > 0;
+    // validates that one or more blocks comments are in the project
+    // returns the blocks that have comments for teacher tool scenario
+    export function validateBlockCommentsExist({ usedBlocks }: { usedBlocks: Blockly.Block[] }): Blockly.Block[] {
+        const commentBlocks = usedBlocks.filter((block) => !!block.getCommentText());
+        return commentBlocks;
+    }
+
+    // validates that all of a specific block type have comments
+    // returns the blocks that do not have comments for a tutorial validation scenario
+    export function validateCommentsOnSpecificBlocksExist({ usedBlocks, blockType }: {
+            usedBlocks: Blockly.Block[],
+            blockType: string,
+        }): Blockly.Block[] {
+        const allSpecifcBlocks = usedBlocks.filter((block) => block.type === blockType);
+        const noncommentBlocks = allSpecifcBlocks.filter((block) => !block.getCommentText());
+        return noncommentBlocks;
     }
 }
\ No newline at end of file
diff --git a/pxtblocks/code-validation/validatorPlans.ts b/pxtblocks/code-validation/validatorPlans.ts
index 731ea2d4bb75..db9bcc8d1477 100644
--- a/pxtblocks/code-validation/validatorPlans.ts
+++ b/pxtblocks/code-validation/validatorPlans.ts
@@ -20,6 +20,6 @@ namespace pxt.blocks {
 
     export interface BlockCommentExistsValidatorCheck extends ValidatorCheckBase {
         validator: "blockCommentExists";
-        onBlocks?: pxt.Map<number>;
+        blockType?: string;
     }
 }
\ No newline at end of file