Merge pull request #231 from AdiAkhileshSingh15/repro-checklists

Adds comments and enhances checklist docs for data structure. 📜

app/components/Project/Project.js

@@ -305,6 +305,14 @@ class Project extends Component<Props> {
     }
   };
 
+  /**
+   * General handler to either insert a new note for a checklist item or update an existing note for
+   * a checklist item.  It figures out the appropriate action to take depending on if the note parameter
+   * is provided or not. It also updates the entire checklist linked to the project.
+   * @param {object} checklistItem The checklist item for which the note should be added/updated
+   * @param {string} text The note text
+   * @param {object} note Optional parameter if there is an existing note being updated.  If not provided, a new note is assumed.
+   */
   checklistUpsertNoteHandler = (checklistItem, text, note) => {
     if (this.unchangedNote(note,text)) {
       return;
@@ -424,12 +432,6 @@ class Project extends Component<Props> {
     }
   };
 
-  assetSelectedHandler = asset => {
-    if (this.props.onAssetSelected) {
-      this.props.onAssetSelected(asset);
-    }
-  };
-
   projectDeleteNoteHandler = (project, note) => {
     const currentProject = { ...this.props.project };
     if (currentProject.id !== project.id) {
@@ -479,6 +481,11 @@ class Project extends Component<Props> {
     }
   };
 
+  /**
+   * General handler to delete a note for a checklist item. It also updates the entire checklist linked to the project.
+   * @param {object} checklistItem The checklist item for which the note should be deleted
+   * @param {object} note The note to delete
+   */
   checklistDeleteNoteHandler = (checklistItem, note) => {
     const currentProject = { ...this.props.project };
     const actionDescription = this.deleteNoteHandler(
@@ -749,7 +756,7 @@ class Project extends Component<Props> {
             onAddedNote={this.checklistUpsertNoteHandler}
             onUpdatedNote={this.checklistUpsertNoteHandler}
             onDeletedNote={this.checklistDeleteNoteHandler}
-            onSelectedAsset={this.assetSelectedHandler}
+            onSelectedAsset={this.props.onAssetSelected}
           />
         ) : null;
 

app/components/ReproChecklist/ChecklistItem/ChecklistItem.js

@@ -2,8 +2,8 @@ import React, { useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
 import styles from './ChecklistItem.css';
 import NoteEditor from '../../NoteEditor/NoteEditor';
-import { AddBox, ContentCopy , Done, Delete} from '@mui/icons-material';
-import { IconButton, Button, Dialog, DialogActions, DialogContent, DialogContentText, DialogTitle } from '@mui/material';
+import { ContentCopy , Done, Delete} from '@mui/icons-material';
+import { IconButton, Dialog, DialogActions, DialogContent, DialogContentText, DialogTitle } from '@mui/material';
 import { FaFolderOpen, FaFolderMinus, FaChevronUp, FaChevronDown } from 'react-icons/fa';
 import AssetTree from '../../AssetTree/AssetTree';
 import AssetUtil from '../../../utils/asset';
@@ -17,15 +17,25 @@ function ChecklistItem(props) {
   const treeRef = React.useRef(null);
 
   const [expanded, setExpanded] = useState(false);
+
+  const [copiedUrlId, setCopiedUrlId] = useState(null);
+
   const [showImages, setShowImages] = useState(false);
   const [showURLs, setShowURLs] = useState(false);
   const [showSubChecks, setShowSubChecks] = useState(false);
-  const [copiedUrlId, setCopiedUrlId] = useState(null);
-  const [selectedAsset, setSelectedAsset] = useState();
+
+  const [selectedAsset, setSelectedAsset] = useState(null);
   const [addAsset, setAddAsset] = useState(false);
   const [assetTitle, setAssetTitle] = useState('');
   const [assetDescription, setAssetDescription] = useState('');
 
+  const resetAssetDialog = () => {
+    setAddAsset(false);
+    setAssetTitle('');
+    setAssetDescription('');
+    setSelectedAsset(null);
+  };
+
   const handleSelectAsset = (selAsset) => {
     let asset = selAsset;
     if (asset && (asset.contentTypes === null || asset.contentTypes === undefined)) {
@@ -38,6 +48,8 @@ function ChecklistItem(props) {
       if (asset.type === Constants.AssetType.FILE) {
         const fileName = AssetUtil.getAssetNameFromUri(asset.uri);
         setAssetTitle(fileName);
+      // Handles case for URL assets
+      // TODO: Title is currently the same as URI, some way to get a better title?
       } else if (asset.type === Constants.AssetType.URL) {
         setAssetTitle(asset.uri);
       } else {
@@ -62,10 +74,12 @@ function ChecklistItem(props) {
     }
 
     if (selectedAsset.type === Constants.AssetType.FILE) {
+      // Adds image files to checklist images
       if(selectedAsset.contentTypes.includes(Constants.AssetContentType.IMAGE)) {
         const updatedItem = { ...item, images: [...item.images, {id: uuidv4(), uri: selectedAsset.uri, title: assetTitle, description: assetDescription}] };
         onItemUpdate(updatedItem);
         setShowImages(true);
+      // Adds file as URL
       } else {
         const updatedItem = { ...item, urls: [...item.urls, {id: uuidv4(), hyperlink: AssetUtil.absoluteToRelativePath(project.path, selectedAsset), title: assetTitle, description: assetDescription}] };
         onItemUpdate(updatedItem);
@@ -77,13 +91,11 @@ function ChecklistItem(props) {
       setShowURLs(true);
     }
 
-    setAddAsset(false);
-    setAssetTitle('');
-    setAssetDescription('');
-    setSelectedAsset(null);
+    resetAssetDialog();
   }
 
   const handleCopy = (urlId, hyperlink) => {
+    // Copy the URL to the clipboard
     navigator.clipboard.writeText(hyperlink).then(() => {
       setCopiedUrlId(urlId);
       setTimeout(() => {
@@ -103,10 +115,6 @@ function ChecklistItem(props) {
   };
 
   const handleNoteUpdate = (note, text) => {
-    if (!text) {
-      console.log("Detected an empty new note - this will not be created");
-      return;
-    }
     if (note) {
       if (onUpdatedNote) {
         onUpdatedNote(item, text, note);
@@ -122,9 +130,6 @@ function ChecklistItem(props) {
     if (onDeletedNote) {
       onDeletedNote(item, note);
     }
-
-    const updatedItem = { ...item, notes: item.notes.filter((n) => n.id !== note.id) };
-    onItemUpdate(updatedItem);
   };
 
   return (
@@ -248,12 +253,7 @@ function ChecklistItem(props) {
                 <button onClick={() => handleAddAsset()} className={styles.submitButton}>
                   Add
                 </button>
-                <button onClick={() => {
-                  setAddAsset(false);
-                  setAssetTitle('');
-                  setAssetDescription('');
-                  setSelectedAsset(null);
-                }} className={styles.cancelButton} autoFocus>
+                <button onClick={() => resetAssetDialog()} className={styles.cancelButton} autoFocus>
                   Cancel
                 </button>
               </DialogActions>

app/components/ReproChecklist/ReproChecklist.js

@@ -18,23 +18,28 @@ function ReproChecklist(props) {
   const { project, checklist, error, onUpdated, onAddedNote, onUpdatedNote, onDeletedNote, onSelectedAsset} = props;
   const [openExportDialog, setOpenExportDialog] = useState(false);
 
+  // this useEffect hook is here to load the scan results for all the checklist statements
   useEffect(() => {
     if (project && checklist && !error) {
       if(project.assets) {
+        // scan result for the first checklist statement
         const scanResult1 = ChecklistUtil.findAssetLanguagesAndDependencies(project.assets);
         checklist[0].scanResult = scanResult1;
       }
     }
   }, [project]);
 
+  // Handles the update of checklist for changes in the checklist items
   const handleItemUpdate = (updatedItem) => {
     const updatedChecklist = checklist.map(item =>
       item.id === updatedItem.id ? updatedItem : item
     );
     onUpdated(project, updatedChecklist);
   };
 
+  // Handles the generation of the reproducibility checklist report in PDF format
   const handleReportGeneration = (exportNotes) => {
+    // pdfMake requires base64 encoded images
     const checkedIcon = GeneralUtil.convertImageToBase64(path.join(__dirname, 'images/yes.png'));
     const statWrapLogo = GeneralUtil.convertImageToBase64(path.join(__dirname, 'images/banner.png'));
 
@@ -132,7 +137,7 @@ function ReproChecklist(props) {
           const imageWidth = 135;
           if(item.images && item.images.length > 0){
             images = item.images.map((image) => {
-              const base64Image = convertImageToBase64(image.uri);
+              const base64Image = GeneralUtil.convertImageToBase64(image.uri);
               if (base64Image) {
                 return {
                   image: base64Image,
@@ -145,6 +150,8 @@ function ReproChecklist(props) {
             });
           }
 
+          // Rendering images by rows, as rendering in columns overflows the page and we can't wrap columns under each other,
+          // math for 3 images per row is as follows:
           // imageWidth*n + calumnGap*(n-1) <= maxWidth - leftMargin - rightMargin
           // 135*n + 10*(n-1) <= 450 - 20 - 0;
           // n <= 440/145 --> n = 3
@@ -297,6 +304,7 @@ function ReproChecklist(props) {
           </DialogContentText>
         </DialogContent>
         <DialogActions>
+          {/* selective export of notes */}
           <Button onClick={() => handleReportGeneration(true)} color="primary">
             Yes
           </Button>

app/containers/CreateProjectDialog/CreateProjectDialog.js

@@ -161,6 +161,7 @@ class CreateProjectDialog extends Component {
           'info',
           this.context,
         );
+        // Seed the project checklist with the null or falsey values
         const projectChecklist = ChecklistUtil.initializeChecklist();
         ipcRenderer.send(
           Messages.WRITE_PROJECT_CHECKLIST_REQUEST,

app/containers/ProjectPage/ProjectPage.js

@@ -37,7 +37,7 @@ class ProjectPage extends Component {
       selectedProject: null,
       // The logs (aka Project Log) for the selected project
       selectedProjectLogs: null,
-
+      // The checklist for the selected project
       selectedProjectChecklist: null,
 
       // UI element to inform us what the popup project list menu is attached to
@@ -176,6 +176,7 @@ class ProjectPage extends Component {
     ipcRenderer.send(Messages.LOAD_PROJECT_CHECKLIST_REQUEST, this.state.selectedProject);
   }
 
+  // This handler writes the updated checklist to the checklist file.
   handleChecklistUpdate(project, checklist) {
     ipcRenderer.send(Messages.WRITE_PROJECT_CHECKLIST_REQUEST, project.path, checklist);
   }
@@ -209,6 +210,11 @@ class ProjectPage extends Component {
     }
   }
 
+  /**
+   * Called when a project checklist is loaded.
+   * @param {object} sender The sender of the message
+   * @param {object} response Response containing a project ID and the checklist (if loaded)
+   */
   handleLoadProjectChecklistResponse(sender, response) {
     if (!response || !response.projectId) {
       return;
@@ -217,7 +223,7 @@ class ProjectPage extends Component {
     // Initialize the checklist file if it doesn't exist, for all the already existing projects
     const projectChecklist = ChecklistUtil.initializeChecklist();
     if (response.checklist && response.checklist.length === 0) {
-      // response only returns project id, we need to get the project path
+      // response only returns project id, we need to find the project path
       const project = this.state.projects.find((p) => p.id === response.projectId);
       ipcRenderer.send(Messages.WRITE_PROJECT_CHECKLIST_REQUEST, project.path, projectChecklist);
     }
@@ -234,7 +240,7 @@ class ProjectPage extends Component {
 
   handleProjectExternallyChangedResponse(sender, response) {
     // If any project externally changes - even if it's not the one we currently have selected - we want
-    // to reload the project log so we can detect changes and notify (if needed);
+    // to reload the project log and checklist, so we can detect changes and notify (if needed);
     if (response && response.projectId && this.state.projects) {
       const foundProject = this.state.projects.find((project) => project.id === response.projectId);
       if (foundProject) {

app/main.dev.js

@@ -666,6 +666,9 @@ ipcMain.on(Messages.LOAD_PROJECT_LOG_REQUEST, async (event, project) => {
   });
 });
 
+/**
+ * Write the project checklist to the checklist file.
+ */
 ipcMain.on(
   Messages.WRITE_PROJECT_CHECKLIST_REQUEST,
   async (event, projectPath, checklist) => {
@@ -674,6 +677,10 @@ ipcMain.on(
   },
 );
 
+/**
+ * Load the project checklist from the checklist file.
+ * Returns response with checklist data and error message if any.
+ */
 ipcMain.on(Messages.LOAD_PROJECT_CHECKLIST_REQUEST, async (event, project) => {
   const response = {
     projectId: project ? project.id : null,
@@ -688,7 +695,9 @@ ipcMain.on(Messages.LOAD_PROJECT_CHECKLIST_REQUEST, async (event, project) => {
     return;
   }
 
-  checklistService.loadChecklist(project.path, (error, checklist) =>{
+  checklistService.loadChecklist(project.path, (error, checklist) => {
+    // This checks for error when there is issue reading the checklist file,
+    // not when the checklist file is not found. For the latter, we return an empty array.
     if (error && !checklist) {
       response.error = true;
       response.errorMessage = `There was an error reading the project checklist ${error}`;

app/services/checklist.js

@@ -5,11 +5,18 @@ const os = require('os');
 const path = require('path');
 
 export default class ChecklistService {
+  /**
+   * Writes the checklist data to the checklist file
+   * @param {string} projectPath The path to the project
+   * @param {object} checklist The checklist data to write
+   * @throws {Error} If the project path or checklist data is invalid or if there is an error writing the file
+   */
   writeChecklist(projectPath, checklist) {
     if (!projectPath || !checklist) {
       throw new Error('Invalid project path or checklist data');
     }
 
+    // Unix-based paths may start with ~, which needs to be replaced with the home directory
     const checklistFilePath = path.join(
       projectPath.replace('~', os.homedir()),
       Constants.StatWrapFiles.BASE_FOLDER,
@@ -19,27 +26,35 @@ export default class ChecklistService {
     fs.writeFileSync(checklistFilePath, JSON.stringify(checklist));
   }
 
+  /**
+   * Loads the checklist data from the checklist file
+   * @param {string} projectPath The path to the project
+   * @param {function} callback The callback function to call with the loaded checklist data and error message
+   */
   loadChecklist(projectPath, callback) {
     if (!projectPath) {
       callback('The project path must be specified', null);
       return;
     }
 
+    // Unix-based paths may start with ~, which needs to be replaced with the home directory
     const checklistFilePath = path.join(
       projectPath.replace('~', os.homedir()),
       Constants.StatWrapFiles.BASE_FOLDER,
       Constants.StatWrapFiles.CHECKLIST,
     );
 
     if (!fs.existsSync(checklistFilePath)) {
+      // If the checklist file does not exist, return an empty array
+      // so that it doesn't display error in the UI for existing projects
       callback('Checklist file not found', []);
       return;
     }
 
     try {
       const data = fs.readFileSync(checklistFilePath);
-      const checklists = JSON.parse(data);
-      callback(null, checklists);
+      const checklist = JSON.parse(data);
+      callback(null, checklist);
     } catch (err) {
       callback('Error reading or parsing checklist file', null);
     }

app/utils/checklist.js

@@ -3,6 +3,10 @@ import AssetsConfig from '../constants/assets-config';
 const path = require('path');
 
 export default class ChecklistUtil {
+  /**
+   * This function initializes the checklist with the statements and seeds other properties
+   * @returns {object} The initialized checklist
+   */
   static initializeChecklist() {
     const checklist = [];
     Constants.CHECKLIST.forEach((statement, index) => {
@@ -21,6 +25,13 @@ export default class ChecklistUtil {
     return checklist;
   }
 
+  /**
+   * This function returns the languages and dependencies of an asset and its children
+   * @param {object} asset The asset to find the languages and dependencies of
+   * @param {object} languages Empty object that acts like a map to store the languages found as keys
+   * @param {object} dependencies Empty object that acts like a map to store the dependencies found as keys
+   * @returns {object} An object containing the languages and dependencies found as arrays
+   */
   static findAssetLanguagesAndDependencies(asset, languages = {}, dependencies = {}) {
     if (!asset) {
       return {
@@ -35,6 +46,7 @@ export default class ChecklistUtil {
 
       if(ext){
         AssetsConfig.contentTypes.forEach((contentType) => {
+          // Ensures both the extension and content type are for code files
           if(contentType.categories.includes(Constants.AssetContentType.CODE) && contentType.extensions.includes(ext)) {
             languages[contentType.name] = true;
           }