Feature/row edit send touched fields only#1733
Conversation
Track which columns the user actually modified and filter getFormattedUpdatedRow to those keys on update (add/dup still send all fields). Deep-compare in updateField so widget-mount re-emits from ForeignKeyEditComponent and BinaryEditComponent don't taint the dirty set.
BinaryEditComponent.ngOnInit re-emitted the stored value after a parseBinaryValue → bytesToHex → toBufferJson round-trip. When the server returned the column as a string (or any non-Buffer-JSON shape) the emit silently differed from what was in tableRowValues, tainting the row-edit touched-fields set and causing the column to be rewritten on every save. ForeignKeyEditComponent.ngOnInit had the same pattern with currentFieldValue. Widgets now emit only on actual user input.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the row-edit flow so that update requests (PUT) include only fields the user actually touched, preventing unchanged values (notably FK/binary) from being resent.
Changes:
- Stop Foreign Key and Binary edit widgets from emitting their current value during
ngOnInit. - Track “touched” fields in
DbTableRowEditComponentand filter the outgoing update payload to touched fields only. - Update/add unit tests to verify no init-time emission and touched-only payload behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/components/ui-components/record-edit-fields/foreign-key/foreign-key.component.ts | Removes init-time emit to avoid marking FK as touched without user action. |
| frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.ts | Removes init-time emit to avoid marking binary as touched without user action. |
| frontend/src/app/components/ui-components/record-edit-fields/binary/binary.component.spec.ts | Updates tests to assert init parses without emitting and user edits still emit. |
| frontend/src/app/components/db-table-row-edit/db-table-row-edit.component.ts | Adds touched-field tracking and filters PUT body to touched fields only. |
| frontend/src/app/components/db-table-row-edit/db-table-row-edit.component.spec.ts | Adds tests for touched-only filtering and an integration-style save assertion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| updateField = (updatedValue: any, field: string) => { | ||
| const existing = this.tableRowValues[field]; | ||
| if (!isEqual(updatedValue, existing)) { | ||
| this.touchedFields.add(field); | ||
| } |
There was a problem hiding this comment.
The new touchedFields tracking can still mark fields as “touched” due to init-time widget emissions (not user interaction). For example, BooleanEditComponent emits in ngOnInit after normalizing the incoming value, which will call updateField and add the field to touchedFields even when the user never changed anything. This undermines the goal of sending only user-touched fields. Consider adding an initialization guard (e.g., skip touched tracking until after initial render / first stable tick, or ignore the first emit per field), or changing the contract so widgets don’t emit on ngOnInit.
| // Integration-style: render the edit form with FK and binary widgets, then save. | ||
| // FK widget (foreign-key.component.ts:120) and Binary widget (binary.component.ts:27) | ||
| // both re-emit their current value on ngOnInit. Touched-tracking MUST ignore those | ||
| // init-time emits — otherwise every edit PUT silently re-writes FK/binary columns | ||
| // the user never touched. |
There was a problem hiding this comment.
These comments describe the old behavior (“FK/Binary widgets … re-emit their current value on ngOnInit”), but this PR removes those init-time emits. The comment block should be updated to reflect the current intent (e.g., regression test ensures untouched FK/binary fields are not included in the update payload) so future readers don’t chase a behavior that no longer exists.
| // Integration-style: render the edit form with FK and binary widgets, then save. | |
| // FK widget (foreign-key.component.ts:120) and Binary widget (binary.component.ts:27) | |
| // both re-emit their current value on ngOnInit. Touched-tracking MUST ignore those | |
| // init-time emits — otherwise every edit PUT silently re-writes FK/binary columns | |
| // the user never touched. | |
| // Integration-style regression test: render the edit form with FK and binary widgets, | |
| // then save. Untouched FK/binary fields must not be included in the update payload, | |
| // while genuinely edited fields should still be sent. This protects against | |
| // accidentally re-introducing behavior that marks unchanged FK/binary values as | |
| // updates during form initialization or save preparation. |
Avoids NG04002 unhandled rejection from the updateRow success callback navigating to a route that isn't registered in the test harness.
No description provided.