refactor: simplify provider management and add Type method#43
Conversation
- Change providers from map to slice for simpler management - Add Type() method to Provider interface for CSS class naming - Update template to use provider Type() for styling instead of Name - Simplify login template data structure - Add OIDC provider styling support
Update OAuth callback handler to use provider.Name() instead of loop variable providerName for consistency with provider management simplification.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the authentication provider management system by simplifying the data structure from map-based to slice-based storage and introducing a new Type() method for better CSS styling support. The changes improve code maintainability while adding styling support for OIDC providers.
Key changes:
- Replaced
map[string]Providerwith[]Providerfor simpler provider management - Added
Type()method to Provider interface returning CSS-friendly identifiers - Updated HTML template to use
Type()for CSS classes and direct provider properties
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/auth/interface.go | Added Type() method to Provider interface |
| pkg/auth/github.go | Implemented Type() method returning "github" |
| pkg/auth/google.go | Implemented Type() method returning "google" |
| pkg/auth/oidc.go | Implemented Type() method returning "oidc" |
| pkg/auth/mock.go | Added Type() method to mock provider |
| pkg/auth/auth.go | Refactored from map to slice-based provider storage and simplified template data |
| pkg/auth/templates/login.html | Updated CSS classes to lowercase and template to use Type() and AuthURL() |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| for _, provider := range a.providers { | ||
| if provider.Name() == name { | ||
| return provider | ||
| } |
There was a problem hiding this comment.
The getProvider method performs a linear search through the providers slice. For applications with many providers, this could impact performance. Consider adding an index or maintaining a map alongside the slice if provider lookups are frequent.
| } | |
| if provider, ok := a.providerMap[name]; ok { | |
| return provider |
Add gofmt format checking to ensure consistent code formatting
Summary
This PR refactors the provider management system to use a simpler slice-based approach instead of a map, and adds a new
Type()method to the Provider interface for better CSS styling support.Type of Change
Related Issues
Changes Made
map[string]Providerto[]Providerfor simpler managementType() stringmethod to Provider interface to return CSS-friendly identifiersProviderDatastructType()for CSS classes instead ofName()Benefits