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

rename cloud account GUID attribute and update validation error messages #426

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

kooomix
Copy link
Contributor

@kooomix kooomix commented Jan 6, 2025

PR Type

Enhancement, Bug fix


Description

  • Renamed AttributeCloudAccountGUID to cloudAccountGUID for consistency.

  • Updated validation error messages for cloud resources.

  • Replaced hardcoded strings with constants in ToMap function.


Changes walkthrough 📝

Relevant files
Enhancement
designators.go
Rename cloud account GUID attribute                                           

identifiers/designators.go

  • Renamed AttributeCloudAccountGUID to cloudAccountGUID.
  • Improved naming consistency for cloud account attributes.
  • +1/-1     
    Bug fix
    integrationref.go
    Update validation and attribute mapping logic                       

    notifications/integrationref.go

  • Updated validation logic for cloud resources.
  • Replaced hardcoded strings with constants in ToMap.
  • Improved error messages for missing attributes.
  • +4/-4     

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

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Validation Change

    The validation requirements for EntityTypeCloudResource have been relaxed by removing the Name field check. Verify this is intentional and aligns with business requirements.

    if e.ResourceHash == "" || e.ResourceID == "" || e.CloudAccountGUID == "" {
    	return fmt.Errorf("resource hash, resource id and cloud account guid are required for %s", e.Type)

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Restore validation check for required name field to ensure complete resource identification

    The validation logic was weakened by removing the 'Name' field check from
    EntityTypeCloudResource validation, which could lead to incomplete resource
    identification.

    notifications/integrationref.go [208-209]

    -if e.ResourceHash == "" || e.ResourceID == "" || e.CloudAccountGUID == "" {
    -    return fmt.Errorf("resource hash, resource id and cloud account guid are required for %s", e.Type)
    +if e.Name == "" || e.ResourceHash == "" || e.ResourceID == "" || e.CloudAccountGUID == "" {
    +    return fmt.Errorf("name, resource hash, resource id and cloud account guid are required for %s", e.Type)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies that the PR removes the 'Name' field validation, which could lead to incomplete or incorrect resource identification. Restoring this validation is important for maintaining data integrity.

    8

    @kooomix kooomix merged commit 296087b into main Jan 6, 2025
    3 checks 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