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

3083 fix validation #33

Merged
merged 8 commits into from
Oct 31, 2024
Merged

3083 fix validation #33

merged 8 commits into from
Oct 31, 2024

Conversation

Stevesibilia
Copy link
Contributor

@Stevesibilia Stevesibilia commented Oct 31, 2024

PR Type

Enhancement, Bug fix


Description

  • Enhanced validation for drupal_projects_list.project_name:
    • Now considers database configuration when validating project name length
    • Allows 6-16 characters when database is created by the module
    • Allows 6-23 characters when database already exists or no database is created
  • Updated regex patterns to ensure project names start and end with alphanumeric characters
  • Added validation for database_user_name:
    • Must be 6-32 characters long with lowercase letters, numbers, hyphens, and underscores
    • Can be null
  • Updated CHANGELOG to reflect the changes in project name length validation
  • Improved error messages for better clarity on validation requirements

Changes walkthrough 📝

Relevant files
Enhancement
variables.tf
Enhance project name and database user validation               

variables.tf

  • Updated validation for project_name in drupal_projects_list variable
  • Added more specific conditions for project name length based on
    database configuration
  • Modified validation for database_user_name
  • +14/-6   
    Documentation
    CHANGELOG.md
    Update CHANGELOG for project name length change                   

    CHANGELOG.md

  • Updated description of drupal_projects_list.project_name max length
    change
  • +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @sparkfabrik-ai-bot
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Complex Validation
    The validation condition for project_name is complex and may be difficult to maintain. Consider refactoring into separate functions for better readability and maintainability.

    Inconsistent Validation
    The error message for project_name validation doesn't accurately reflect all the conditions in the validation logic, particularly for the 6-23 character length cases.

    Typo
    There's a typo in the CHANGELOG entry: "lenght" should be "length".

    @sparkfabrik-ai-bot
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Simplify the validation condition using a single regex pattern and length checks

    Simplify the condition in the validation block by using a single regex pattern for
    all cases. This will make the code more readable and maintainable.

    variables.tf [78-83]

     condition = alltrue([
       for p in var.drupal_projects_list :
    -  (can(regex("^[0-9a-z_-]{6,16}$", p.project_name)) && p.database_host != null && p.database_name == null && p.database_user_name == null && can(regex("^[0-9a-z]{1}[0-9a-z_-]+[0-9a-z]{1}$", p.project_name))) ||
    -  (can(regex("^[0-9a-z_-]{6,23}$", p.project_name)) && p.database_host != null && p.database_name != null && p.database_user_name != null && can(regex("^[0-9a-z]{1}[0-9a-z_-]+[0-9a-z]{1}$", p.project_name))) ||
    -  (can(regex("^[0-9a-z_-]{6,23}$", p.project_name)) && p.database_host == null && can(regex("^[0-9a-z]{1}[0-9a-z_-]+[0-9a-z]{1}$", p.project_name)))
    +  can(regex("^[0-9a-z][0-9a-z_-]{4,21}[0-9a-z]$", p.project_name)) &&
    +  (
    +    (p.database_host != null && p.database_name == null && p.database_user_name == null && length(p.project_name) <= 16) ||
    +    (p.database_host != null && p.database_name != null && p.database_user_name != null && length(p.project_name) <= 23) ||
    +    (p.database_host == null && length(p.project_name) <= 23)
    +  )
     ])
     
    Suggestion importance[1-10]: 8

    Why: The suggestion significantly improves code readability and maintainability by simplifying the complex condition with a single regex pattern and clear length checks.

    8
    Improve the error message to provide more accurate and detailed validation requirements

    Update the error message to provide more specific information about the validation
    requirements, including the different length constraints based on the database
    configuration.

    variables.tf [84]

    -error_message = "The project name is invalid. Must be 6 to 16 characters long, with only lowercase letters, numbers, hyphens and underscores if the database must be created or 16 to 23 characters long if database already exists."
    +error_message = "The project name is invalid. It must start and end with a lowercase letter or number, contain only lowercase letters, numbers, hyphens and underscores, and be: 6-16 characters if a new database is to be created, or 6-23 characters if using an existing database or no database."
     
    Suggestion importance[1-10]: 7

    Why: The updated error message provides more specific and accurate information about the validation requirements, which will help users understand and correct their input more easily.

    7
    Correct spelling and improve clarity in the changelog entry

    Correct the spelling of "length" in the changelog entry and clarify the condition
    for the 16-character limit.

    CHANGELOG.md [19]

    -- Changed `drupal_projects_list.project_name` max lenght to 16 database creation is handled by the module itself.
    +- Changed `drupal_projects_list.project_name` max length to 16 characters when database creation is handled by the module itself.
     
    Suggestion importance[1-10]: 5

    Why: The suggestion corrects a spelling error and slightly improves the clarity of the changelog entry, which is a minor but useful improvement for documentation accuracy.

    5
    Best practice
    Enhance the database username validation to ensure it starts and ends with a letter or number

    Consider using a more specific regex pattern for database_user_name to ensure it
    starts and ends with a letter or number, similar to the project name validation.

    variables.tf [90-91]

    -(can(regex("^[0-9a-z_-]{6,32}$", p.database_user_name))) ||
    +(can(regex("^[0-9a-z][0-9a-z_-]{4,30}[0-9a-z]$", p.database_user_name))) ||
     (p.database_user_name == null)
     
    Suggestion importance[1-10]: 6

    Why: This suggestion improves the validation of the database username, making it consistent with the project name validation and potentially preventing invalid usernames.

    6

    variables.tf Outdated Show resolved Hide resolved
    Co-authored-by: Andrea Panisson <[email protected]>
    variables.tf Outdated Show resolved Hide resolved
    variables.tf Outdated Show resolved Hide resolved
    variables.tf Outdated Show resolved Hide resolved
    @andypanix andypanix merged commit 15f0ab0 into main Oct 31, 2024
    1 check passed
    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.

    2 participants