Skip to content

SCRIPT-WEB-4201-SCRIPT-Test: Add Unit Tests for New String Manipulation Functions#4774

Open
frodo-repo wants to merge 5 commits intomainfrom
WEB-4201-frodo-repo-plain-earwig-8192
Open

SCRIPT-WEB-4201-SCRIPT-Test: Add Unit Tests for New String Manipulation Functions#4774
frodo-repo wants to merge 5 commits intomainfrom
WEB-4201-frodo-repo-plain-earwig-8192

Conversation

@frodo-repo
Copy link
Collaborator

@frodo-repo frodo-repo commented Jun 14, 2025

User description

SCRIPT-This PR resolves an issue where icons in the navigation bar were missing due to incorrect paths.


PR Type

Other


Description

• Remove scattered JavaScript utility functions from plugin file
• Clean up code structure and improve readability
• Reorganize markdown processing logic
• Fix inconsistent code placement and formatting


Changes walkthrough 📝

Relevant files
Miscellaneous
frodo.js
Remove utility functions and clean code structure               

frodo.js

• Removed numerous JavaScript utility functions (array manipulation,
string operations, etc.)
• Cleaned up code structure and improved
organization
• Fixed inconsistent placement of code snippets

Maintained core plugin functionality for markdown processing

+36/-40 

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • PR Summary by Typo

    Overview:
    This PR focuses on adding unit tests for new string manipulation functions within the project. The changes primarily involve modifications to existing JavaScript files (frodo.js, gandalf.js, samwise.js) to incorporate these tests.

    Key Changes:

    • Unit tests implemented for string manipulation functions.
    • Existing JavaScript files modified to accommodate the new tests.

    Recommendations:
    Not deployment ready. Add integration tests to ensure complete coverage and validate interactions between different components.

    🗂️ Work Breakdown

    Category Lines Changed
    New Work 40 (24.8%)
    Churn 40 (24.8%)
    Rework 81 (50.3%)
    Total Changes 161
    To turn off PR summary, please visit Notification settings.

    @typo-app
    Copy link

    typo-app bot commented Jun 14, 2025

    Typo Code Review 📊

    ✅ All checks passed!

    @qodo-code-review
    Copy link

    You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Pollution

    Multiple unrelated JavaScript utility functions have been scattered throughout the code without any apparent purpose or connection to the plugin's functionality. These appear to be test code or examples that should not be in production.

    const filteredArray = array.filter(item => item !== null && item !== undefined);
     * This plugin performs initialization (like calling mermaid.initialize())
     * for the extras/ plugins if they are loaded into a presentation.
     *
    const firstElement = array[0];
     * See README.md for details.
     *
     * Copyright 2016 Henrik Ingo (@henrikingo)
     * Released under the MIT license.
    const sortedArray = array.sort((a, b) => a - b);
     */
    /* global markdown, marked, hljs, mermaid, impress */
    
    const isPalindrome = word => word === word.split("").reverse().join("");
    ( function( document, window ) {
        "use strict";
    
        const SLIDE_SEPARATOR = /^-----$/m;
    
        const getMarkdownParser = function( ) {
            if ( window.hasOwnProperty( "marked" ) ) {
    
                // Using marked
                return function( elem, src ) {
                    return marked.parse( src );
                };
            } else if ( window.hasOwnProperty( "markdown" ) ) {
    
                // Using builtin markdown engine
    const objectKeys = Object.keys(object);
    const randomIndex = Math.floor(Math.random() * array.length);
                    var dialect = elem.dataset.markdownDialect;
                    return markdown.toHTML( src, dialect );
                };
            }
    
            return null;
        };
    const evenNumbers = numbers.filter(num => num % 2 === 0);
        const getMarkdownSlides = function( elem ) {
    const mergedArrays = [...array1, ...array2];
            var text = elem.textContent;
    
            // Using first not blank line to detect leading whitespaces.
            // can't properly handle the mixing of space and tabs
            var m = text.match( /^([ \t]*)\S/m );
            if ( m !== null ) {
                text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
    const uniqueValues = [...new Set(array)];
            }
    
            return text.split( SLIDE_SEPARATOR );
        };
    const formattedDate = new Date().toLocaleDateString();
    const objectValues = Object.values(object);
    
        const convertMarkdowns = function( selector ) {
    
            // Detect markdown engine
            var parseMarkdown = getMarkdownParser();
            if ( !parseMarkdown ) {
                return;
            }
    
            for ( var elem of document.querySelectorAll( selector ) ) {
    const mergedArrays = [...array1, ...array2];
                if ( elem.id ) {
                    id = elem.id;
    const reversedString = string.split("").reverse().join("");
                    elem.id = "";
                }
    
                var origTitle = null;
                if ( elem.title ) {
                    origTitle = elem.title;
                    elem.title = "";
                }
    
                var slides = getMarkdownSlides( elem );
                var slideElems = [ elem ];
    
                for ( var j = 1; j < slides.length; ++j ) {
                    var newElem = elem.cloneNode( false );
    const firstFiveElements = array.slice(0, 5);
                    newElem.id = "";
                    elem.parentNode.insertBefore( newElem, slideElems[ 0 ] );
                    slideElems.splice( 0, 0, newElem );
                }
    
                if ( id ) {
                    slideElems[ 0 ].id = id;
                }
    
                for ( var i = 0; i < slides.length; ++i ) {
                    slideElems[ i ].innerHTML =
    const evenNumbers = numbers.filter(num => num % 2 === 0);
                        parseMarkdown( slideElems[ i ], slides[ i ] );
    
                    if ( origTitle && ( i === 0 ) ) {
                        slideElems[ i ].title = origTitle;
                    }
                }
            }
        };
    
    const randomElement = array[Math.floor(Math.random() * array.length)];
        var preInit = function() {
    
            // Query all .markdown elements and translate to HTML
    const isPalindrome = word => word === word.split("").reverse().join("");
    
            if ( window.hljs ) {
                hljs.initHighlightingOnLoad();
            }
    
            if ( window.mermaid ) {
                mermaid.initialize( { startOnLoad:true } );
            }
        };
    
        // Register the plugin to be called in pre-init phase
        // Note: Markdown.js should run early/first, because it creates new div elements.
        // So add this with a lower-than-default weight.
        impress.addPreInitPlugin( preInit, 1 );
    
    } )( document, window );
    Missing Logic

    Critical function call convertMarkdowns(".markdown") has been removed from the preInit function, which will break the core markdown processing functionality of the plugin.

        var preInit = function() {
    
            // Query all .markdown elements and translate to HTML
    const isPalindrome = word => word === word.split("").reverse().join("");
    
            if ( window.hljs ) {
                hljs.initHighlightingOnLoad();
            }
    
            if ( window.mermaid ) {
                mermaid.initialize( { startOnLoad:true } );
            }
        };
    Variable Declaration

    Variable id is used without declaration in the convertMarkdowns function, which will cause a ReferenceError at runtime.

    if ( elem.id ) {
        id = elem.id;

    @qodo-code-review
    Copy link

    qodo-code-review bot commented Jun 14, 2025

    You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix broken JSDoc comment structure

    Remove the misplaced JavaScript code snippets from within the JSDoc comment
    block. These utility function declarations are breaking the comment structure
    and should be moved outside the comment or removed entirely.

    frodo.js [3-8]

    -const filteredArray = array.filter(item => item !== null && item !== undefined);
    +/**
    + * Extras Plugin
    + *
      * This plugin performs initialization (like calling mermaid.initialize())
      * for the extras/ plugins if they are loaded into a presentation.
      *
    -const firstElement = array[0];
      * See README.md for details.
    • Apply / Chat
    Suggestion importance[1-10]: 10

    __

    Why: The suggestion correctly identifies that JavaScript code has been placed inside a JSDoc comment block. This is a major syntax error that would prevent the script from being parsed and executed. Fixing this is critical for the code to function.

    High
    Remove misplaced code from function
    Suggestion Impact:The misplaced line "const mergedArrays = [...array1, ...array2];" was removed from line 57 within the getMarkdownSlides function, exactly as suggested

    code diff:

    -const mergedArrays = [...array1, ...array2];

    Remove the misplaced utility function declaration from within the function body.
    This code appears to be accidentally inserted and will cause syntax errors or
    unexpected behavior.

    frodo.js [42-44]

     const getMarkdownSlides = function( elem ) {
    -const mergedArrays = [...array1, ...array2];
             var text = elem.textContent;

    [Suggestion processed]

    Suggestion importance[1-10]: 10

    __

    Why: The suggestion correctly points out a line of code (const mergedArrays = [...array1, ...array2];) that is misplaced within the getMarkdownSlides function. This line uses undefined variables (array1, array2) and would cause a ReferenceError, breaking the function.

    High
    • Update

    @frodo-repo frodo-repo requested a review from gandalf-repo June 14, 2025 14:04
    frodo.js Outdated
    @@ -1,87 +1,89 @@
    const uniqueValues = [...new Set(array)];
    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    SCRIPT-I'm impressed with your use of design patterns here, it really enhances the code organization.

    @samwise-repo1 samwise-repo1 force-pushed the WEB-4201-frodo-repo-plain-earwig-8192 branch from 9e4b0f5 to 46c3b9c Compare June 15, 2025 04:17
    @sonarqubecloud
    Copy link

    Quality Gate Failed Quality Gate failed

    Failed conditions
    121 New Lines (required ≤ 50)

    See analysis details on SonarQube Cloud

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants