Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Next #127

Merged
merged 2 commits into from
Oct 21, 2024
Merged

Next #127

merged 2 commits into from
Oct 21, 2024

Conversation

seeratawan01
Copy link
Member

@seeratawan01 seeratawan01 commented Oct 21, 2024

PR Type

Tests, Enhancement, Configuration changes


Description

  • Enhanced the parseQueryString function to handle leading '?' and prevent empty keys in parameters.
  • Improved the isObject function to correctly handle null values.
  • Added comprehensive unit tests for helper functions, including generateId, isValidEmail, debounce, getUtmParams, parseQueryString, isString, isObject, and parseLogLevel.
  • Implemented unit tests for the RetryQueue class, covering queue operations, retry logic, and offline/online transitions.
  • Introduced a new GitHub Actions workflow to automate the creation of release candidates from the develop branch.

Changes walkthrough 📝

Relevant files
Enhancement
helpers.ts
Enhance query string parsing and object validation             

packages/javascript-sdk/src/utils/helpers.ts

  • Improved query string parsing by removing leading '?'.
  • Added check to avoid empty keys in query parameters.
  • Enhanced isObject function to handle null values.
  • +6/-3     
    Tests
    helpers.test.ts
    Add unit tests for helper functions                                           

    packages/javascript-sdk/test/unit/utils/helpers.test.ts

  • Added unit tests for helper functions.
  • Tested generateId, isValidEmail, and debounce.
  • Verified getUtmParams and parseQueryString functionality.
  • Included tests for isString, isObject, and parseLogLevel.
  • +161/-0 
    queue.test.ts
    Implement unit tests for RetryQueue                                           

    packages/javascript-sdk/test/unit/utils/queue.test.ts

  • Added tests for RetryQueue functionality.
  • Tested queue addition and batch processing.
  • Verified retry mechanism and storage loading.
  • Checked offline/online transition handling.
  • +115/-0 
    Configuration changes
    release-candidate.yml
    Add GitHub Actions workflow for release candidates             

    .github/workflows/release-candidate.yml

  • Created a GitHub Actions workflow for release candidates.
  • Configured to trigger on pull request closure to develop.
  • Generates a release candidate version based on latest tag.
  • +52/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @seeratawan01 seeratawan01 merged commit 24971b5 into develop Oct 21, 2024
    1 of 2 checks passed
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🏅 Score: 85
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling
    Ensure that the parseQueryString function handles cases where decodeURIComponent throws an error due to malformed URI components.

    Type Checking
    The isObject function should ensure that functions and arrays are not incorrectly identified as plain objects.

    Code feedback:
    relevant filepackages/javascript-sdk/src/utils/helpers.ts
    suggestion      

    Consider adding error handling for decodeURIComponent to catch exceptions from malformed URI components. This can prevent potential runtime errors. [important]

    relevant lineparams[decodeURIComponent(pair[0])] = decodeURIComponent(pair[1] || '');

    relevant filepackages/javascript-sdk/src/utils/helpers.ts
    suggestion      

    Refine the isObject function to exclude functions and arrays, which are technically objects but not plain objects. You can add checks like !Array.isArray(value) && !(value instanceof Function). [important]

    relevant linereturn value !== null && typeof value === 'object' && value.constructor === Object;

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Ensure robustness by verifying the structure of query string pairs before processing

    Add a check to ensure that the pair array has exactly two elements after splitting
    to avoid accessing undefined indices.

    packages/javascript-sdk/src/utils/helpers.ts [48]

     const pair = queries[i].split('=');
    +if (pair.length !== 2) continue;
    Suggestion importance[1-10]: 9

    Why: The suggestion improves the robustness of the code by ensuring that the pair array has exactly two elements before accessing them. This prevents potential errors when dealing with malformed query strings and enhances the reliability of the function.

    9
    Prevent potential runtime errors by checking the existence of query parameter values before decoding

    Ensure that the pair[1] value is checked for existence before decoding to prevent
    potential runtime errors if the query string is malformed.

    packages/javascript-sdk/src/utils/helpers.ts [50-51]

    -params[decodeURIComponent(pair[0])] = decodeURIComponent(pair[1] || '');
    +if (pair[1]) {
    +    params[decodeURIComponent(pair[0])] = decodeURIComponent(pair[1]);
    +} else {
    +    params[decodeURIComponent(pair[0])] = '';
    +}
    Suggestion importance[1-10]: 8

    Why: This suggestion addresses a potential runtime error by ensuring that the value of pair[1] is checked for existence before decoding. This is crucial for handling malformed query strings gracefully, thereby improving the robustness of the code.

    8
    Enhancement
    Enhance the robustness of object type checking by safely verifying the constructor property

    Consider using Object.prototype.hasOwnProperty.call(value, 'constructor') to check
    for the constructor property to avoid potential issues with objects that do not
    inherit from Object.prototype.

    packages/javascript-sdk/src/utils/helpers.ts [62]

    -return value !== null && typeof value === 'object' && value.constructor === Object;
    +return value !== null && typeof value === 'object' && Object.prototype.hasOwnProperty.call(value, 'constructor') && value.constructor === Object;
    Suggestion importance[1-10]: 7

    Why: This suggestion enhances the robustness of the isObject function by safely checking the constructor property, which can prevent issues with objects that do not inherit from Object.prototype. It is a good practice for more reliable type checking.

    7
    Maintainability
    Improve code readability and maintainability by using Array.prototype.forEach for iteration

    Use Array.prototype.forEach for iterating over queries to improve readability and
    leverage functional programming practices.

    packages/javascript-sdk/src/utils/helpers.ts [47-52]

    -for (let i = 0; i < queries.length; i++) {
    -    const pair = queries[i].split('=');
    +queries.forEach(query => {
    +    const pair = query.split('=');
         if (pair[0] !== '') {
             params[decodeURIComponent(pair[0])] = decodeURIComponent(pair[1] || '');
         }
    -}
    +});
    Suggestion importance[1-10]: 6

    Why: The suggestion to use Array.prototype.forEach improves code readability and aligns with functional programming practices. While it doesn't change the functionality, it enhances maintainability and clarity of the code.

    6

    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.

    1 participant