Skip to content
This repository was archived by the owner on Oct 10, 2025. It is now read-only.

feat: add Modica Group SMS provider support - #667

Closed
superstructor wants to merge 1 commit into
nhost:mainfrom
superstruct:main
Closed

feat: add Modica Group SMS provider support#667
superstructor wants to merge 1 commit into
nhost:mainfrom
superstruct:main

Conversation

@superstructor

Copy link
Copy Markdown
Contributor

Summary

  • Add ModicaSMS provider implementing controller.SMSer interface
  • Support provider selection via AUTH_SMS_PROVIDER environment variable
  • Maintain backward compatibility with existing Twilio configuration
  • Add extensive test coverage including mock server and validation tests

Configuration

New environment variables:

  • AUTH_SMS_MODICA_USERNAME - Modica API username
  • AUTH_SMS_MODICA_PASSWORD - Modica API password

Provider selection:

  • Set AUTH_SMS_PROVIDER=modica to use Modica Group
  • Defaults to twilio for backward compatibility

Technical Details

  • API Integration: Uses Modica REST v2 API with HTTPS and Basic Authentication
  • Input Validation: Enforces international phone format (+ prefix required)
  • Error Handling: Parses structured API error responses with fallback for malformed responses
  • HTTP Client: Production configuration with 30s timeout, connection pooling, and TLS settings
  • Testing: Comprehensive test suite with mock server, validation tests, and integration test support

Test Coverage

  • Unit tests for provider initialization and SMS sending
  • Mock server tests for various API response scenarios
  • Input validation tests for phone format and message content
  • Integration tests (requires environment variables)
  • Error handling tests for API failures and malformed responses

Before submitting this PR:

Checklist

  • No breaking changes
  • Tests pass
  • New features have new tests
  • Documentation is updated

Breaking changes

No breaking changes. The implementation maintains full backward compatibility with existing Twilio configurations and defaults to Twilio when no
provider is specified.

Tests

  • All existing tests pass with go test -v ./...
  • New comprehensive test suite for Modica provider with 100% coverage
  • Mock server tests validate API request/response format
  • Integration tests support real API testing with credentials
  • Input validation tests ensure proper error handling

Documentation

  • Updated environment-variables.md with new Modica configuration options
  • Updated AUTH_SMS_PROVIDER documentation to show both twilio and modica options
  • Added documentation for AUTH_SMS_MODICA_USERNAME and AUTH_SMS_MODICA_PASSWORD
  • Code follows existing patterns and conventions from CLAUDE.md guidelines (as DEVELOPER.md seemed outdated)

Test Plan

  • Verify Twilio SMS still works (backward compatibility)
  • Test Modica SMS with valid credentials
  • Confirm proper error messages for invalid phone formats
  • Validate provider switching via environment variable
  • Run full test suite: go test -v ./go/notifications/sms/

Warning

Despite extensive tests I havn't actually done e2e testing in the nhost stack with this setup and would appreciate your help with doing so, I can provide test Modica group credentials on DM if needed.

Comment thread go/notifications/sms/modica_sms.go Outdated
@dbarrosop

Copy link
Copy Markdown
Member

Closing/Reopening in order to re-trigger tests with safe_to_test label.

@dbarrosop

Copy link
Copy Markdown
Member

This looks go to me, the only thing I am unsure about is whether to generate a client automatically from their spec using oapi-codegen or just leave it as you implemented it given how small the surface is. We can probably leave as is for now.

The linter isn't happy, just run:

golines -w  --base-formatter=gofumpt .

To fix the formatting and then golangci-lint run ./....

Re e2e tests, how do you want to do it? Using the cli or the cloud? In the cli case I can build a binary with support for this in the toml, in the cloud case I will need to set the env vars for you.

- Add ModicaSMS provider implementing controller.SMSer interface
- Support provider selection via AUTH_SMS_PROVIDER environment variable
- Maintain backward compatibility with existing Twilio configuration
- Add extensive test coverage including mock server and validation tests
@superstructor

Copy link
Copy Markdown
Contributor Author

@dbarrosop thanks for reviewing and your patience.

Have fixed the lint issues. Happy to provide secrets to you for e2e tests via Discord DM.

@superstructor

Copy link
Copy Markdown
Contributor Author

@dbarrosop sorry I think I broke my fork, so I'll re-create this PR with the latest changes as this appears unlinked/unmergable.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants