-
Notifications
You must be signed in to change notification settings - Fork 19
Fix service rate fee persistence by using proper Ember Data records #43
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove @Tracked perDropFees array that never synced to rate_fees relationship - Remove syncPerDropFees() method that was causing fees to be wiped on updates - Update addPerDropRateFee() to create proper service-rate-fee records and add to rate_fees - Update removePerDropFee() to accept model instance and call destroyRecord() - Update resetPerDropFees() to work with Ember Data records instead of plain objects This fixes the bug where service rate fees were being set to 0 after updates because the plain JavaScript array was not syncing to the Ember Data relationship.
- Refactor rateFees computed property to be read-only (no side effects) - Create generateFixedRateFees() action to explicitly manage fee records - Remove unsaved (local) fees before creating new ones to prevent duplicates - Only work with saved fees (those with IDs) to preserve values after refresh - Filter by distance >= 0 to prevent negative distance display - syncServiceRateFees() now calls generateFixedRateFees() for backward compatibility Fixes: 1. Duplicated rate_fees on frontend after create (server response + local records) 2. Empty/reset fee values after refresh (now preserves saved values) 3. Distance display showing -1 km (filtered out negative distances)
- Import observer from @ember/object - Add maxDistanceObserver to watch max_distance and max_distance_unit - Automatically calls generateFixedRateFees() when values change - Restores core functionality: fees now generate in real-time as user types This fixes the regression where fees stopped generating when max_distance was changed in the form after refactoring the computed property.
- Remove deprecated observer pattern - Remove generateFixedRateFees action from model - Remove syncServiceRateFees action from model - Keep only rateFees computed property for display (pure function) - Business logic moved to service-rate-actions service Model now only responsible for: - Data attributes and relationships - Simple computed properties for display - Per-drop fee management (separate concern) This follows Ember best practices and proper separation of concerns.
Move duplicate cleanup logic from controllers to serializer where it belongs. **Why Serializer:** - Serializer is responsible for data normalization and store lifecycle - normalizeSaveResponse() hook runs after save completes - Proper place to clean up relationships after backend response - Follows Ember Data best practices **Implementation:** - Override normalizeSaveResponse() to detect service-rate saves - Schedule cleanup after store has been updated with backend data - Add _cleanupDuplicateRateFees() helper method - Removes unsaved records that duplicate saved records **Benefits:** - ✅ Separation of concerns (data layer handles data) - ✅ No manual cleanup in controllers - ✅ Automatic for all service-rate saves - ✅ Proper Ember Data lifecycle management **Result:** - No duplicates after save - Clean rate_fees relationship - Follows framework conventions
Much simpler approach: directly set serviceRate.rate_fees to only the
saved records from the backend response.
**Previous Approach:**
- Iterate through existing records
- Separate saved vs unsaved
- Remove unsaved that duplicate saved
- Unload records individually
**New Approach:**
- Get saved rate_fees IDs from normalized response
- Fetch those records from store
- Directly set relationship: serviceRate.set('rate_fees', savedFees)
**Why This Works:**
- rateFees computed property reads from rate_fees relationship
- If rate_fees only has saved records, no duplicates
- Simple assignment replaces complex iteration
- Ember Data handles the rest
**Benefits:**
- ✅ Much simpler code
- ✅ More direct and explicit
- ✅ Easier to understand
- ✅ Same result: no duplicates
Thanks to user feedback for pointing out the simpler solution!
**Issue:**
After save, distance=0 fee (0-1 km) was missing from the details view.
**Root Cause:**
Using serviceRate.set('rate_fees', savedFees) doesn't properly handle
hasMany relationships in Ember Data. The set() method might not correctly
replace the relationship content.
**Solution:**
Use proper hasMany relationship methods:
- rateFees.clear() to remove all existing records
- rateFees.pushObjects(savedFees) to add only saved records
**Why This Works:**
- clear() properly removes all references from the relationship
- pushObjects() adds the new records correctly
- Ember Data handles all the internal bookkeeping
- All fees including distance=0 are preserved
**Result:**
- All fees display correctly including 0-1 km ✅
- No duplicates ✅
- Proper Ember Data API usage ✅
**Issue 1: Distance 0 saved as null** - distance attribute was defined as @attr('string') - When setting distance: 0 (number), it serialized as null - Changed to @attr('number') to properly handle numeric values - Now distance=0 saves correctly as 0, not null - 0-1 km fee now saves and displays ✅ **Issue 2: New fees not saving on update** - Serializer was clearing ALL rate_fees and only adding saved ones - When changing max_distance (e.g., 0→4), new local fees were created - After save, serializer cleared them because backend didn't know about them - Changed to only remove duplicate unsaved fees, not all unsaved fees - New fees are preserved and sent to backend on next save ✅ **Changes:** - addon/models/service-rate-fee.js: distance type string→number - addon/serializers/service-rate.js: selective cleanup instead of clear() **Result:** - 0-1 km fee saves correctly ✅ - New fees when changing max_distance are preserved ✅ - No duplicates ✅
**Issue:**
Using setTimeout() is a code smell in Ember applications.
Should use proper Ember run loop APIs instead.
**Solution:**
- Import { next } from '@ember/runloop'
- Replace setTimeout(() => {}, 0) with next(() => {})
**Why next():**
- next() schedules work for the next run loop iteration
- Same behavior as setTimeout with 0 delay
- Proper Ember API that integrates with the run loop
- Better for testing (can use run loop control in tests)
- Avoids potential timing issues
- More explicit about intent
**Benefits:**
- ✅ Follows Ember best practices
- ✅ Better integration with Ember's run loop
- ✅ Easier to test
- ✅ More maintainable
- ✅ No code smells
Thanks to user feedback for catching this!
**Issue:** Database error when saving rate_fees: "Incorrect integer value: '\xE2\x82\xAE2,000' for column 'fee'" Fee was being sent as formatted string "₮2,000" instead of plain number 2000. **Root Cause:** - fee attribute was defined as @attr('string') - MoneyInput component formats the display value - When bound to string attribute, it stores the formatted string - Backend expects integer, receives formatted string → error **Solution:** Changed fee attribute from @attr('string') to @attr('number') **How It Works:** - MoneyInput displays formatted value (₮2,000) - MoneyInput binds to number attribute - Stores plain number (2000) in model - Serializes as plain number to backend - Backend receives integer → success ✅ **Same Fix as distance:** This is the same issue we fixed with distance attribute. Both distance and fee need to be numbers, not strings. **Result:** - Fee values save correctly ✅ - No database errors ✅ - Display still shows formatted values ✅ - Backend receives plain numbers ✅
**Issues:**
1. Per-Drop rate_fees duplicated after save (same as Fixed Rate had)
2. Parcel fees not displaying after save
**Root Cause:**
Serializer cleanup was only running for Fixed Rate:
```javascript
if (serviceRate && serviceRate.isFixedRate) {
// cleanup only for fixed rate
}
```
This meant:
- Per-Drop rates: No cleanup → duplicates ❌
- Parcel fees: No cleanup at all → not displaying ❌
**Solution:**
1. Removed isFixedRate check
- Cleanup now applies to ALL rate types (Fixed, Per-Drop, Per-Meter)
2. Added parcel_fees cleanup
- Same pattern as rate_fees cleanup
- Checks for duplicates based on size/dimensions
- Removes unsaved parcel fees that duplicate saved ones
**How It Works:**
rate_fees cleanup:
- Get all rate_fees (saved + unsaved)
- Map saved fees by distance
- Remove unsaved fees that have same distance as saved fees
parcel_fees cleanup:
- Get all parcel_fees (saved + unsaved)
- Check for duplicates based on size, length, width, height
- Remove unsaved parcel fees that match saved ones
**Result:**
- ✅ Per-Drop: No more duplicates after save
- ✅ Fixed Rate: Still works (no regression)
- ✅ Parcel fees: Display correctly after save
- ✅ All rate types: Consistent cleanup behavior
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the bug where service rate fees were being set to 0 after updates because the plain JavaScript array was not syncing to the Ember Data relationship.