feat: v0.4.4 - Enhanced Currency Support & Renewal Date Fixes #46
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "v0.4.4"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
🎯 Overview
This release introduces comprehensive currency support and fixes critical renewal date calculation issues, along with extensive testing improvements.
✨ New Features
🌍 Currency Conversion System
📅 Enhanced Date Handling
🛠️ Migration & Safety Tools
🐛 Bug Fixes
🧪 Testing Improvements
🔧 Technical Enhancements
AddMonthsNoOverflow,AddYearsNoOverflow)🔑 Important Notes
Currency API is Completely Optional
Database Changes
exchange_ratestable (auto-created)date_calculation_versioncolumn (default: 1)🙏 Thanks to Contributors
📊 Validation Results
Real-World Testing:
Unit Tests: All 17 test suites passing
🚀 Ready for Production
This release has been thoroughly tested with comprehensive edge case validation and is ready for production deployment.
Full Changelog: https://github.com/bscott/subtrackr/compare/v0.4.3...v0.4.4
Pull Request Overview
This release introduces comprehensive currency support with optional real-time exchange rates and fixes critical renewal date calculation issues. The PR adds multi-currency support for 8 currencies (USD, EUR, GBP, JPY, RUB, SEK, PLN, INR), implements a versioned date calculation system using the Carbon library for better edge case handling, and provides migration tools for safe upgrades.
Key Changes:
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
Comments suppressed due to low confidence (1)
templates/subscription-form.html:1
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -94,0 +284,4 @@targetDay := startDayif startDay > lastDay {targetDay = lastDay}This complex manual date arithmetic for handling month-end dates duplicates logic that could be simplified. Consider extracting this logic into a helper function since it's repeated in the default case (lines 257-286) and could benefit from unit testing in isolation.
@ -0,0 +1,208 @@package serviceThe API URL uses HTTP instead of HTTPS, which exposes the API key in plain text during transmission. This should be changed to 'https://data.fixer.io/api/latest' to ensure secure transmission of the API key.
@ -158,4 +174,75 @@</button>Using setMonth() can cause issues with month-end dates. For example, if today is January 31st and you add 1 month, JavaScript will overflow to March 3rd instead of February 28th. This doesn't match the robust date handling implemented in the backend Carbon library.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -73,24 +75,274 @@ func (s *Subscription) IsHighCost() bool {func (s *Subscription) BeforeCreate(tx *gorm.DB) error {The comment should specify what 'backward compatibility' means in this context - that existing subscriptions will continue using the original date calculation logic unless explicitly migrated to V2.
@ -0,0 +1,982 @@package modelsThis assertion will fail for month-end dates like January 31st when the renewal falls in February (28/29 days). The test should account for month-end adjustments in date calculations.
@ -0,0 +1,208 @@package serviceThe supported currencies list is hardcoded as a string. Consider extracting this to a constant or configuration to improve maintainability and consistency with the settings service validation.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
internal/service/currency.go:1
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -262,6 +346,7 @@ func (h *SubscriptionHandler) UpdateSubscription(c *gin.Context) {}The handler fetches the existing subscription to check for schedule changes, but this adds an extra database query to every update operation. Consider optimizing this by checking for schedule changes in the service layer or using a more efficient approach.
@ -73,24 +75,274 @@ func (s *Subscription) IsHighCost() bool {func (s *Subscription) BeforeCreate(tx *gorm.DB) error {The long comment within the switch statement breaks readability. Consider moving this explanation to the function documentation or using a shorter inline comment.
@ -39,16 +39,16 @@ func (r *SubscriptionRepository) Create(subscription *models.Subscription) (*moderr := r.db.Transaction(func(tx *gorm.DB) error {The SQL INSERT statement uses positional parameters (?) which makes it fragile and hard to maintain. Consider using named parameters or splitting this into smaller, more manageable parts.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -0,0 +1,208 @@package serviceThe HTTP client lacks timeout configuration, which could lead to indefinite blocking. Consider using http.Client with a reasonable timeout or context.WithTimeout for API calls.
Using fmt.Printf for logging in production code is not ideal. Consider using a proper logging library like log.Printf or a structured logger for consistent log formatting and levels.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -0,0 +1,208 @@package serviceThe supported currency symbols are hardcoded in multiple places (here and in settings.go). Consider extracting this to a shared constant or configuration to avoid duplication and ensure consistency.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -14,15 +14,82 @@ import ("github.com/gin-gonic/gin"[nitpick] The comment mentions avoiding pointer to loop variable issue, but this pattern is correct for Go 1.22+. Consider updating the comment to reflect that this creates a copy for modification rather than addressing a loop variable issue.
@ -94,0 +180,4 @@// Default to monthlycurrent := start.Copy()for current.Lte(now) {current = current.AddMonthsNoOverflow(1)[nitpick] The versioned calculation approach should document which scenarios trigger V2 vs V1 logic. Consider adding a comment explaining when DateCalculationVersion would be 2 vs default/1.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
internal/models/subscription_test.go:1
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -0,0 +1,167 @@package mainConsider using
v2Date.Sub(*v1Date).Truncate(24*time.Hour).Hours()/24to get more accurate day differences instead of floating-point division which may introduce precision errors.Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -0,0 +119,4 @@v2Str = v2Date.Format("2006-01-02")}if v1Date != nil && v2Date != nil {diff := v2Date.Sub(*v1Date).Truncate(24*time.Hour).Hours() / 24Missing import for the
timepackage. The code usestime.Hourbuttimeis not imported in the file.Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -49,1 +46,4 @@subscription.Name, subscription.Cost, subscription.Schedule,subscription.Status, subscription.CategoryID, category.Name, subscription.OriginalCurrency,subscription.PaymentMethod, subscription.Account,subscription.StartDate, subscription.RenewalDate,There's a mismatch between the number of columns (7 in the INSERT statement) and the number of placeholders (17). The VALUES clause has too many placeholders for the columns specified.
@ -0,0 +1,208 @@package serviceThe HTTP client should validate SSL certificates and handle potential malicious redirects. Consider using a more restrictive HTTP client configuration or validating the response URL matches the expected domain.
Pull Request Overview
Copilot reviewed 21 out of 23 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -0,0 +1,208 @@package serviceThe comment 'Fetch all supported currencies' is misleading. The function is called with 'USD' as targetCurrency but fetches rates for all supported currencies with EUR as base. Consider updating the comment to reflect the actual behavior.
Pull Request Overview
Copilot reviewed 21 out of 23 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
The comment '// (Line removed)' on the deleted line indicates incomplete cleanup. This creates confusion about what was actually removed and should be cleaned up.
@ -0,0 +1,208 @@package serviceThe comment states 'Fetch rates for all supported currencies with base currency' but the function is called with 'USD' as the target currency for all base currencies. This doesn't actually fetch rates for all supported currencies - it only fetches the base-to-USD rate.
Pull Request Overview
Copilot reviewed 21 out of 23 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@ -0,0 +19,4 @@// supportedCurrencySymbols returns the currencies as a comma-separated string for API callsfunc supportedCurrencySymbols() string {return strings.Join(SupportedCurrencies, ",")The SupportedCurrencies variable is defined in the service package but is also used in the settings service. Consider moving this to a shared package or constants file to avoid potential import cycles and improve maintainability.
@CyberL1 @amitsingh-007 Would you mind if you have the time to review this PR, before I merge. Just want some human eyes instead of an automated code review at this time, quite a bit of change in this one, I tested locally a few times with optimizations coming later. This PR should resolve #43 & #44
Reviewed INR currency support. LGTM
Renewal date updates as requested
Thank you both, you rock!