Skip to content

Add Makefile lint target and device settings model with validation to smsgateway #23

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

Merged
merged 3 commits into from
May 26, 2025

Conversation

capcom6
Copy link
Member

@capcom6 capcom6 commented May 22, 2025

Summary by CodeRabbit

  • New Features

    • Introduced new device settings configuration options for encryption, messages, ping, logs, and webhooks.
    • Added support for a new "SettingsUpdated" push notification event.
  • Chores

    • Added a Makefile command to run code linting for improved code quality.

Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
smsgateway/domain_settings.go 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai bot changed the title @coderabbitai Add Makefile lint target and device settings model with validation to smsgateway May 22, 2025
Copy link

coderabbitai bot commented May 22, 2025

Walkthrough

A new Makefile linting target was introduced, and the smsgateway package received significant additions: a comprehensive device settings data model with validation tags, and an update to the upstream push event system to support a new "SettingsUpdated" event type. Additionally, unit tests were added for the new device settings validation logic.

Changes

File(s) Change Summary
Makefile Added a lint target to run golangci-lint run -v for code linting.
smsgateway/domain_settings.go Added new enums and structs representing device settings with validation and JSON serialization tags, including DeviceSettings and related sub-structs with validation methods.
smsgateway/domain_upstream.go Added PushSettingsUpdated event type constant and updated PushNotification.Event validation to include "SettingsUpdated".
smsgateway/domain_settings_test.go Added unit tests for DeviceSettings.Validate and SettingsMessages.Validate methods, including multiple test cases and a helper function for pointer creation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Device
    participant Gateway

    User->>Device: Update device settings (via API/UI)
    Device->>Gateway: Send PushNotification (Event: SettingsUpdated)
    Gateway->>Device: (optional) Acknowledge or process updated settings
Loading
sequenceDiagram
    participant Developer
    participant Makefile

    Developer->>Makefile: make lint
    Makefile->>Makefile: Run golangci-lint run -v
    Makefile-->>Developer: Output lint results
Loading

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (1.64.8)

Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2
Failed executing command with error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17246cf and 2e41cae.

📒 Files selected for processing (4)
  • Makefile (1 hunks)
  • smsgateway/domain_settings.go (1 hunks)
  • smsgateway/domain_settings_test.go (1 hunks)
  • smsgateway/domain_upstream.go (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • Makefile
🚧 Files skipped from review as they are similar to previous changes (3)
  • smsgateway/domain_upstream.go
  • smsgateway/domain_settings_test.go
  • smsgateway/domain_settings.go
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (8)
smsgateway/domain_settings.go (7)

1-10: Exported Struct Missing GoDoc Comment
The top‐level DeviceSettings struct aggregates all device configuration categories but lacks a GoDoc comment. Please add a descriptive comment above the type to improve API discoverability.


12-14: Add Validation & Documentation for Encryption Passphrase
SettingsEncryption.Passphrase currently only has omitempty. Consider enforcing a minimum length (e.g. validate:"omitempty,min=1") to prevent empty strings, and add a GoDoc comment describing expected passphrase semantics.


16-19: Verify http_url Validation Tag & Add Field Docs
The validate:"omitempty,http_url" tag may not be built in to some validator libraries (which typically use url). Confirm that http_url is registered or switch to validate:"omitempty,url". Also add GoDoc comments for SettingsGateway and its fields.


21-28: Define Typed Enums for Message Settings
Extract the raw string lists in LimitPeriod and SimSelectionMode into Go const groups (typed enums). This prevents typos, centralizes valid values, and ensures the oneof tag stays in sync.


30-32: Add GoDoc for Ping Settings Struct
SettingsPing is clear in code but should include a GoDoc comment explaining what “ping” means in context and any recommended value ranges.


34-36: Add GoDoc for Logs Settings Struct
Please add a documentation comment for SettingsLogs, describing how LifetimeDays is applied (e.g., rolling window, archive policy, etc.).


38-42: Enhance Webhooks Settings Validation & Docs

  • Consider adding validate:"omitempty,min=1" on SigningKey to prevent empty but non-nil strings.
  • Add GoDoc comments for SettingsWebhooks and its fields to clarify behavior when InternetRequired is false or absent.
smsgateway/domain_upstream.go (1)

23-23: Update Validation Tag for New Event
The oneof list now includes SettingsUpdated. To avoid manual drift, consider deriving valid event names from the PushEventType constants (e.g., via a helper or code‐generated tag) rather than hard-coding strings.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b8b2f91 and 6e8b4dd.

📒 Files selected for processing (3)
  • Makefile (1 hunks)
  • smsgateway/domain_settings.go (1 hunks)
  • smsgateway/domain_upstream.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
Makefile (1)

29-31: Add Lint Target
The new lint target running golangci-lint is a valuable addition to enforce code quality across the Go modules.

smsgateway/domain_upstream.go (1)

14-15: Add New Push Event Type
Good addition of PushSettingsUpdated to represent settings changes in the upstream event system.

Copy link

coderabbitai bot commented May 22, 2025

Caution

No docstrings were generated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
smsgateway/domain_settings.go (1)

1-119: Consider adding a constructor function for DeviceSettings

The settings structure is well-designed, but creating instances might be error-prone.

Consider adding a constructor function with sensible defaults:

// NewDeviceSettings creates a new DeviceSettings instance with default values
func NewDeviceSettings() *DeviceSettings {
    defaultLimitPeriod := Disabled
    defaultSimMode := OSDefault
    
    return &DeviceSettings{
        Messages: &SettingsMessages{
            SendIntervalMin: intPtr(1),
            SendIntervalMax: intPtr(5),
            LimitPeriod: &defaultLimitPeriod,
            LimitValue: intPtr(100),
            SimSelectionMode: &defaultSimMode,
            LogLifetimeDays: intPtr(30),
        },
        Ping: &SettingsPing{
            IntervalSeconds: intPtr(300), // 5 minutes
        },
        Logs: &SettingsLogs{
            LifetimeDays: intPtr(30),
        },
        Webhooks: &SettingsWebhooks{
            InternetRequired: boolPtr(true),
            RetryCount: intPtr(3),
        },
    }
}

// Helper functions
func intPtr(i int) *int { return &i }
func boolPtr(b bool) *bool { return &b }
🧰 Tools
🪛 GitHub Check: Lint

[failure] 86-86:
The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)

🪛 GitHub Actions: Go

[error] 86-86: golangci-lint: The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e8b4dd and 9e0855e.

📒 Files selected for processing (1)
  • smsgateway/domain_settings.go (1 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint
smsgateway/domain_settings.go

[failure] 86-86:
The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)

🪛 GitHub Actions: Go
smsgateway/domain_settings.go

[error] 86-86: golangci-lint: The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (8)
smsgateway/domain_settings.go (8)

3-15: LimitPeriod enum looks good

The enum definition for message sending limits is well-structured with clear documentation for each constant value.


17-27: SimSelectionMode enum looks good

The enum definition for SIM selection modes is well-structured with clear documentation for each constant value.


29-48: DeviceSettings structure is well-organized

The main settings structure is well-designed with:

  • Clear documentation for each field
  • Appropriate use of pointers for optional fields
  • Proper JSON tags with omitempty for nil values

This approach allows for flexible configuration with clear serialization behavior.


50-54: SettingsEncryption structure looks good

The encryption settings structure is appropriately designed with an optional passphrase field.


56-64: SettingsGateway structure has appropriate validation

The gateway settings include proper HTTP URL validation for the CloudURL field when provided.


93-98: SettingsPing structure looks good

The ping settings structure has appropriate validation for the interval field.


100-105: SettingsLogs structure looks good

The logs settings structure has appropriate validation for the lifetime field.


107-118: SettingsWebhooks structure looks good

The webhooks settings structure has appropriate fields and validation for retry count.


// SimSelectionMode defines how SIM cards are selected for sending messages.
// Valid values are "OSDefault", "RoundRobin", or "Random".
SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" validate:"omitempty,oneof=OSDefault RoundRobin Random"`
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Line exceeds maximum length limit

This line is 128 characters, exceeding the 120-character limit enforced by linters.

Split the validation tag onto a new line or shorten it:

- SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" validate:"omitempty,oneof=OSDefault RoundRobin Random"`
+ SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" 
+   validate:"omitempty,oneof=OSDefault RoundRobin Random"`

Or alternatively:

- SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" validate:"omitempty,oneof=OSDefault RoundRobin Random"`
+ SimSelectionMode *SimSelectionMode `json:"sim_selection_mode,omitempty" 
+   validate:"omitempty,enum"`

And define a custom validator for the enum type.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Check: Lint

[failure] 86-86:
The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)

🪛 GitHub Actions: Go

[error] 86-86: golangci-lint: The line is 128 characters long, which exceeds the maximum of 120 characters. (lll)

🤖 Prompt for AI Agents
In smsgateway/domain_settings.go at line 86, the line exceeds the 120-character
limit due to a long validation tag. To fix this, split the struct tag onto
multiple lines by placing the validation tag on a new line or shorten the
validation string. Alternatively, define a custom validator for the
SimSelectionMode enum type and reference it in the tag to reduce length.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
smsgateway/domain_settings.go (1)

96-96: Line exceeds maximum length limit

This line is 128 characters, exceeding the 120-character limit enforced by linters.

🧹 Nitpick comments (1)
smsgateway/domain_settings.go (1)

53-58: Add test coverage for validation methods.

The validation methods are critical for ensuring data integrity but currently lack test coverage. Consider adding comprehensive unit tests that cover:

  1. DeviceSettings.Validate() - test with valid/invalid Messages
  2. SettingsMessages.Validate() - test cross-field validation scenarios:
    • Valid: SendIntervalMax >= SendIntervalMin
    • Invalid: SendIntervalMax < SendIntervalMin
    • Edge cases: nil values, equal values

Would you like me to generate comprehensive unit tests for these validation methods?

Also applies to: 103-109

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 53-57: smsgateway/domain_settings.go#L53-L57
Added lines #L53 - L57 were not covered by tests

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e0855e and 589c7e9.

📒 Files selected for processing (1)
  • smsgateway/domain_settings.go (1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
smsgateway/domain_settings.go

[warning] 53-57: smsgateway/domain_settings.go#L53-L57
Added lines #L53 - L57 were not covered by tests


[warning] 103-106: smsgateway/domain_settings.go#L103-L106
Added lines #L103 - L106 were not covered by tests


[warning] 108-108: smsgateway/domain_settings.go#L108
Added line #L108 was not covered by tests

🔇 Additional comments (5)
smsgateway/domain_settings.go (5)

1-4: LGTM! Good use of nolint directive.

The global nolint for line length is appropriate here since validation tags can legitimately exceed line length limits.


6-30: Well-defined enum types with clear documentation.

Both LimitPeriod and SimSelectionMode enums are properly structured with descriptive constants and good documentation.


103-109: Excellent cross-field validation implementation.

The custom validation method properly ensures that SendIntervalMax is not less than SendIntervalMin, addressing the constraint mentioned in the documentation. Good use of error wrapping with ErrValidationFailed.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 103-106: smsgateway/domain_settings.go#L103-L106
Added lines #L103 - L106 were not covered by tests


[warning] 108-108: smsgateway/domain_settings.go#L108
Added line #L108 was not covered by tests


60-64: Simple and clear encryption settings structure.

The SettingsEncryption struct is well-documented with a clear approach to enabling/disabling encryption based on passphrase presence.


111-136: Well-structured settings for ping, logs, and webhooks.

All three structs (SettingsPing, SettingsLogs, SettingsWebhooks) have appropriate validation constraints and clear documentation. The use of pointer types with omitempty provides good flexibility for optional configuration.

Comment on lines +53 to +55
func (s DeviceSettings) Validate() error {
if s.Messages != nil {
return s.Messages.Validate()
}
return nil
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Incomplete validation in DeviceSettings.Validate() method.

The validation method only checks the Messages field, but other fields like Gateway have validation constraints that should also be validated. Consider validating all non-nil fields with validation requirements.

Apply this diff to improve the validation:

 func (s DeviceSettings) Validate() error {
+	if s.Gateway != nil {
+		// Gateway validation is handled by struct tags
+	}
 	if s.Messages != nil {
-		return s.Messages.Validate()
+		if err := s.Messages.Validate(); err != nil {
+			return err
+		}
 	}
+	// Add validation for other fields as needed
 	return nil
 }

Note: If you're using a validation library like go-playground/validator, you might want to integrate that here to validate struct tags across all fields.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (s DeviceSettings) Validate() error {
if s.Messages != nil {
return s.Messages.Validate()
}
return nil
}
func (s DeviceSettings) Validate() error {
if s.Gateway != nil {
// Gateway validation is handled by struct tags
}
if s.Messages != nil {
if err := s.Messages.Validate(); err != nil {
return err
}
}
// Add validation for other fields as needed
return nil
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 53-57: smsgateway/domain_settings.go#L53-L57
Added lines #L53 - L57 were not covered by tests

🤖 Prompt for AI Agents
In smsgateway/domain_settings.go around lines 53 to 58, the Validate() method
only validates the Messages field but ignores other fields like Gateway that
also require validation. Update the method to validate all relevant fields that
have validation constraints, checking each non-nil field and returning any
validation errors found. If using a validation library like
go-playground/validator, integrate it to validate struct tags across all fields
for a more comprehensive and maintainable approach.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
smsgateway/domain_settings_test.go (1)

36-36: Remove placeholder comments or replace with specific TODOs.

The "Add more test cases as needed" comments should either be removed or replaced with specific TODO items that describe what test cases are missing.

-		// Add more test cases as needed
+		// TODO: Add test cases for Encryption, Ping, Logs, and Webhooks validation
-		// Add more test cases as needed  
+		// TODO: Add test cases for LimitPeriod, LimitValue, SimSelectionMode, and LogLifetimeDays validation

Also applies to: 82-82

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84bb151 and f4805e0.

📒 Files selected for processing (1)
  • smsgateway/domain_settings_test.go (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
smsgateway/domain_settings_test.go (1)
smsgateway/domain_settings.go (4)
  • DeviceSettings (33-48)
  • SettingsMessages (64-88)
  • LimitPeriod (7-7)
  • SimSelectionMode (21-21)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
smsgateway/domain_settings_test.go (2)

1-7: Package structure and imports look good.

The test package follows Go conventions by using the _test suffix for external testing, and the imports are appropriate.


94-97: Helper function is well-implemented.

The ptr helper function is a clean and reusable utility for creating integer pointers in tests.

Comment on lines +48 to +91
func TestSettingsMessages_Validate(t *testing.T) {
tests := []struct {
name string
messages smsgateway.SettingsMessages
wantErr bool
}{
{
name: "valid settings",
messages: smsgateway.SettingsMessages{
SendIntervalMin: nil,
SendIntervalMax: nil,
LimitPeriod: nil,
LimitValue: nil,
SimSelectionMode: nil,
LogLifetimeDays: nil,
},
wantErr: false,
},
{
name: "invalid send interval",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(1),
SendIntervalMax: ptr(0), // Invalid: should be >= 1
},
wantErr: true,
},
{
name: "invalid send interval max < min",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(2),
SendIntervalMax: ptr(1), // Invalid: max < min
},
wantErr: true,
},
// Add more test cases as needed
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.messages.Validate(); (err != nil) != tt.wantErr {
t.Errorf("SettingsMessages.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Insufficient test coverage for SettingsMessages validation constraints.

The current tests only cover SendIntervalMin/Max validation but miss several important validation scenarios based on the struct tags in domain_settings.go:

Missing test cases for:

  • LimitPeriod validation (should test invalid enum values)
  • LimitValue validation (should test values < 1)
  • SimSelectionMode validation (should test invalid enum values)
  • LogLifetimeDays validation (should test values < 1)
  • Edge cases for SendIntervalMin (testing value < 1)
		// Add more test cases as needed
+		{
+			name: "invalid send interval min",
+			messages: smsgateway.SettingsMessages{
+				SendIntervalMin: ptr(0), // Invalid: should be >= 1
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid limit value",
+			messages: smsgateway.SettingsMessages{
+				LimitValue: ptr(0), // Invalid: should be >= 1
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid log lifetime days",
+			messages: smsgateway.SettingsMessages{
+				LogLifetimeDays: ptr(0), // Invalid: should be >= 1
+			},
+			wantErr: true,
+		},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestSettingsMessages_Validate(t *testing.T) {
tests := []struct {
name string
messages smsgateway.SettingsMessages
wantErr bool
}{
{
name: "valid settings",
messages: smsgateway.SettingsMessages{
SendIntervalMin: nil,
SendIntervalMax: nil,
LimitPeriod: nil,
LimitValue: nil,
SimSelectionMode: nil,
LogLifetimeDays: nil,
},
wantErr: false,
},
{
name: "invalid send interval",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(1),
SendIntervalMax: ptr(0), // Invalid: should be >= 1
},
wantErr: true,
},
{
name: "invalid send interval max < min",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(2),
SendIntervalMax: ptr(1), // Invalid: max < min
},
wantErr: true,
},
// Add more test cases as needed
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.messages.Validate(); (err != nil) != tt.wantErr {
t.Errorf("SettingsMessages.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
func TestSettingsMessages_Validate(t *testing.T) {
tests := []struct {
name string
messages smsgateway.SettingsMessages
wantErr bool
}{
{
name: "valid settings",
messages: smsgateway.SettingsMessages{
SendIntervalMin: nil,
SendIntervalMax: nil,
LimitPeriod: nil,
LimitValue: nil,
SimSelectionMode: nil,
LogLifetimeDays: nil,
},
wantErr: false,
},
{
name: "invalid send interval",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(1),
SendIntervalMax: ptr(0), // Invalid: should be >= 1
},
wantErr: true,
},
{
name: "invalid send interval max < min",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(2),
SendIntervalMax: ptr(1), // Invalid: max < min
},
wantErr: true,
},
// Add more test cases as needed
{
name: "invalid send interval min",
messages: smsgateway.SettingsMessages{
SendIntervalMin: ptr(0), // Invalid: should be >= 1
},
wantErr: true,
},
{
name: "invalid limit value",
messages: smsgateway.SettingsMessages{
LimitValue: ptr(0), // Invalid: should be >= 1
},
wantErr: true,
},
{
name: "invalid log lifetime days",
messages: smsgateway.SettingsMessages{
LogLifetimeDays: ptr(0), // Invalid: should be >= 1
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.messages.Validate(); (err != nil) != tt.wantErr {
t.Errorf("SettingsMessages.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
🤖 Prompt for AI Agents
In smsgateway/domain_settings_test.go between lines 48 and 92, the test coverage
for SettingsMessages.Validate is incomplete. Add test cases to cover validation
of LimitPeriod with invalid enum values, LimitValue with values less than 1,
SimSelectionMode with invalid enum values, LogLifetimeDays with values less than
1, and edge cases for SendIntervalMin with values less than 1. Each test case
should set the relevant field to an invalid value and assert that Validate
returns an error.

Comment on lines 9 to 45
func TestDeviceSettings_Validate(t *testing.T) {
tests := []struct {
name string
settings smsgateway.DeviceSettings
wantErr bool
}{
{
name: "nil messages",
settings: smsgateway.DeviceSettings{
Messages: nil,
},
wantErr: false,
},
{
name: "valid messages",
settings: smsgateway.DeviceSettings{
Messages: &smsgateway.SettingsMessages{
SendIntervalMin: nil,
SendIntervalMax: nil,
LimitPeriod: nil,
LimitValue: nil,
SimSelectionMode: nil,
LogLifetimeDays: nil,
},
},
wantErr: false,
},
// Add more test cases as needed
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.settings.Validate(); (err != nil) != tt.wantErr {
t.Errorf("DeviceSettings.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Expand test coverage for DeviceSettings validation.

The current tests only cover the Messages field of DeviceSettings. Based on the struct definition in domain_settings.go, DeviceSettings has additional fields (Encryption, Ping, Logs, Webhooks) that should also be tested to ensure comprehensive validation coverage.

Consider adding test cases for:

  • Non-nil values for other fields (Encryption, Ping, Logs, Webhooks)
  • Invalid scenarios that should trigger validation errors
		// Add more test cases as needed
+		{
+			name: "valid settings with all fields",
+			settings: smsgateway.DeviceSettings{
+				Encryption: &smsgateway.SettingsEncryption{},
+				Messages:   &smsgateway.SettingsMessages{},
+				Ping:       &smsgateway.SettingsPing{},
+				Logs:       &smsgateway.SettingsLogs{},
+				Webhooks:   &smsgateway.SettingsWebhooks{},
+			},
+			wantErr: false,
+		},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestDeviceSettings_Validate(t *testing.T) {
tests := []struct {
name string
settings smsgateway.DeviceSettings
wantErr bool
}{
{
name: "nil messages",
settings: smsgateway.DeviceSettings{
Messages: nil,
},
wantErr: false,
},
{
name: "valid messages",
settings: smsgateway.DeviceSettings{
Messages: &smsgateway.SettingsMessages{
SendIntervalMin: nil,
SendIntervalMax: nil,
LimitPeriod: nil,
LimitValue: nil,
SimSelectionMode: nil,
LogLifetimeDays: nil,
},
},
wantErr: false,
},
// Add more test cases as needed
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.settings.Validate(); (err != nil) != tt.wantErr {
t.Errorf("DeviceSettings.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
func TestDeviceSettings_Validate(t *testing.T) {
tests := []struct {
name string
settings smsgateway.DeviceSettings
wantErr bool
}{
{
name: "nil messages",
settings: smsgateway.DeviceSettings{
Messages: nil,
},
wantErr: false,
},
{
name: "valid messages",
settings: smsgateway.DeviceSettings{
Messages: &smsgateway.SettingsMessages{
SendIntervalMin: nil,
SendIntervalMax: nil,
LimitPeriod: nil,
LimitValue: nil,
SimSelectionMode: nil,
LogLifetimeDays: nil,
},
},
wantErr: false,
},
// Add more test cases as needed
{
name: "valid settings with all fields",
settings: smsgateway.DeviceSettings{
Encryption: &smsgateway.SettingsEncryption{},
Messages: &smsgateway.SettingsMessages{},
Ping: &smsgateway.SettingsPing{},
Logs: &smsgateway.SettingsLogs{},
Webhooks: &smsgateway.SettingsWebhooks{},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.settings.Validate(); (err != nil) != tt.wantErr {
t.Errorf("DeviceSettings.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
🤖 Prompt for AI Agents
In smsgateway/domain_settings_test.go between lines 9 and 46, the test coverage
for DeviceSettings.Validate() only includes the Messages field. To improve
coverage, add test cases that set non-nil values for the other fields
Encryption, Ping, Logs, and Webhooks in DeviceSettings. Include both valid and
invalid scenarios for these fields to verify that validation errors are
correctly triggered or not. This will ensure comprehensive validation testing
for all fields in the DeviceSettings struct.

@capcom6 capcom6 force-pushed the settings/device-settings branch from 17246cf to 2e41cae Compare May 24, 2025 09:53
@capcom6 capcom6 marked this pull request as ready for review May 25, 2025 23:17
@capcom6 capcom6 merged commit 51d4a14 into master May 26, 2025
7 checks passed
@capcom6 capcom6 deleted the settings/device-settings branch May 26, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant