Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import { MatDialogModule } from '@angular/material/dialog';
import { MatSnackBar, MatSnackBarModule } from '@angular/material/snack-bar';
import { provideRouter } from '@angular/router';
import { Angulartics2Module } from 'angulartics2';
import { of } from 'rxjs';
import { Connection, ConnectionType, DBtype } from 'src/app/models/connection';
import { ConnectionsService } from 'src/app/services/connections.service';
import { TableRowService } from 'src/app/services/table-row.service';
import { TablesService } from 'src/app/services/tables.service';
import { DbTableRowEditComponent } from './db-table-row-edit.component';

Expand Down Expand Up @@ -296,6 +298,40 @@ describe('DbTableRowEditComponent', () => {
component.pageAction = null;
});

describe('onlyTouched filtering', () => {
beforeEach(() => {
component.tableRowValues = {
id: 1,
username: 'original',
email: 'a@x.test',
bio: 'hello',
};
});

it('returns all fields by default', () => {
const result = component.getFormattedUpdatedRow();
expect(Object.keys(result).sort()).toEqual(['bio', 'email', 'id', 'username']);
});

it('returns only touched fields when onlyTouched=true', () => {
component.updateField('renamed', 'username');
const result = component.getFormattedUpdatedRow(true);
expect(result).toEqual({ username: 'renamed' });
});

it('returns empty object when nothing was touched', () => {
const result = component.getFormattedUpdatedRow(true);
expect(result).toEqual({});
});

it('tracks multiple touched fields', () => {
component.updateField('renamed', 'username');
component.updateField('b@x.test', 'email');
const result = component.getFormattedUpdatedRow(true);
expect(result).toEqual({ username: 'renamed', email: 'b@x.test' });
});
});

it('should include password field when it has a value', () => {
component.tableRowValues = {
id: 1,
Expand Down Expand Up @@ -342,4 +378,123 @@ describe('DbTableRowEditComponent', () => {
expect((result as any).password).toBe('');
});
});

// 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.
Comment on lines +382 to +386
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
describe('render + save: untouched FK and binary fields are not sent', () => {
let tableRowService: TableRowService;
let updateTableRowSpy: ReturnType<typeof vi.spyOn>;

beforeEach(async () => {
tableRowService = TestBed.inject(TableRowService);
const tablesService = TestBed.inject(TablesService);

// updateRow's success callback navigates; no routes are registered in this
// harness, so stub navigate to avoid NG04002 unhandled rejections.
vi.spyOn(component.router, 'navigate').mockResolvedValue(true);

updateTableRowSpy = vi
.spyOn(tableRowService, 'updateTableRow')
.mockReturnValue(of({ row: { id: '42' }, primaryColumns: [{ column_name: 'id' }] } as any));

// FK widget loads its current row + autocomplete suggestions via fetchTable.
// Both calls must resolve so its ngOnInit can reach the onFieldChange.emit.
vi.spyOn(tablesService, 'fetchTable').mockReturnValue(
of({
rows: [{ id: 7, name: 'Alice' }],
primaryColumns: [{ column_name: 'id', data_type: 'int' }],
identity_column: 'name',
} as any),
);

vi.spyOn(connectionsService, 'currentConnection', 'get').mockReturnValue({
id: 'conn-1',
database: 'shop',
title: 'Shop',
host: 'localhost',
port: '5432',
sid: null,
type: DBtype.Postgres,
username: 'u',
ssh: false,
ssl: false,
cert: '',
masterEncryption: false,
azure_encryption: false,
connectionType: ConnectionType.Direct,
} as Connection);
vi.spyOn(connectionsService, 'currentConnectionID', 'get').mockReturnValue('conn-1');

// Overwrite whatever the initial (failing) ngOnInit left, then render.
component.connectionID = 'conn-1';
component.tableName = 'orders';
component.hasKeyAttributesFromURL = true;
component.keyAttributesFromURL = { id: '42' };
component.keyAttributesListFromStructure = ['id'];
component.readonlyFields = [];
component.nonModifyingFields = [];
component.pageMode = 'edit';
component.pageAction = null;
component.tableForeignKeys = [
{
column_name: 'CustomerId',
referenced_column_name: 'id',
referenced_table_name: 'customers',
constraint_name: 'fk_customer',
autocomplete_columns: [],
} as any,
];
component.tableRowValues = {
name: 'order-42',
CustomerId: 7,
payload: { type: 'Buffer', data: [1, 2, 3, 4] },
};
component.tableTypes = {
name: 'varchar',
CustomerId: 'foreign key',
payload: 'bytea',
};
component.tableRowRequiredValues = { name: false, CustomerId: false, payload: false };
component.tableRowStructure = {
name: { column_name: 'name', data_type: 'varchar', allow_null: true },
CustomerId: { column_name: 'CustomerId', data_type: 'integer', allow_null: true },
payload: { column_name: 'payload', data_type: 'bytea', allow_null: true },
} as any;
component.fieldsOrdered = ['name', 'CustomerId', 'payload'];
component.tableWidgetsList = [];
component.tableWidgets = {};
component.loading = false;

fixture.detectChanges();
await fixture.whenStable();
fixture.detectChanges();
});

it('sends an empty body when the user did not touch any field', async () => {
component.handleRowSubmitting(false);
await fixture.whenStable();

expect(updateTableRowSpy).toHaveBeenCalledTimes(1);
const body = updateTableRowSpy.mock.calls[0][3];
expect(body).toEqual({});
});

it('sends only the touched text field when the user edits name', async () => {
// simulate the user typing into the text widget — same path the widget's
// (onFieldChange) output would take through the template
component.updateField('order-43', 'name');

component.handleRowSubmitting(false);
await fixture.whenStable();

expect(updateTableRowSpy).toHaveBeenCalledTimes(1);
const body = updateTableRowSpy.mock.calls[0][3];
expect(body).toEqual({ name: 'order-43' });
expect(body).not.toHaveProperty('CustomerId');
expect(body).not.toHaveProperty('payload');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Title } from '@angular/platform-browser';
import { ActivatedRoute, Router, RouterModule } from '@angular/router';
import JsonURL from '@jsonurl/jsonurl';
import JSON5 from 'json5';
import { isEqual } from 'lodash-es';
import { DynamicModule } from 'ng-dynamic-component';
import { SignalComponentIoModule } from 'ng-dynamic-component/signal-component-io';
import { defaultTimestampValues, recordEditTypes, timestampTypes, UIwidgets } from 'src/app/consts/record-edit-types';
Expand Down Expand Up @@ -74,6 +75,7 @@ export class DbTableRowEditComponent implements OnInit {
public tableName: string | null = null;
public dispalyTableName: string | null = null;
public tableRowValues: Record<string, any>;
private touchedFields = new Set<string>();
public tableRowStructure: object;
public tableRowRequiredValues: object;
public identityColumn: string;
Expand Down Expand Up @@ -612,6 +614,9 @@ export class DbTableRowEditComponent implements OnInit {

updateField = (updatedValue: any, field: string) => {
const existing = this.tableRowValues[field];
if (!isEqual(updatedValue, existing)) {
this.touchedFields.add(field);
}
Comment on lines 615 to +619
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
if (
typeof updatedValue === 'object' &&
updatedValue !== null &&
Expand All @@ -631,8 +636,10 @@ export class DbTableRowEditComponent implements OnInit {
}
};

getFormattedUpdatedRow = () => {
let updatedRow = { ...this.tableRowValues };
getFormattedUpdatedRow = (onlyTouched: boolean = false) => {
let updatedRow = onlyTouched
? Object.fromEntries(Object.entries(this.tableRowValues).filter(([key]) => this.touchedFields.has(key)))
: { ...this.tableRowValues };

//crutch, format datetime fields
//if no one edit manually datetime field, we have to remove '.000Z', cuz mysql return this format but it doesn't record it
Expand Down Expand Up @@ -756,12 +763,13 @@ export class DbTableRowEditComponent implements OnInit {
updateRow(continueEditing: boolean) {
this.submitting = true;

const formattedUpdatedRow = this.getFormattedUpdatedRow();
const formattedUpdatedRow = this.getFormattedUpdatedRow(true);

this._tableRow
.updateTableRow(this.connectionID, this.tableName, this.keyAttributesFromURL, formattedUpdatedRow)
.subscribe(
(res) => {
this.touchedFields.clear();
this.ngZone.run(() => {
if (continueEditing) {
if (this.isPrimaryKeyUpdated) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,29 @@ describe('BinaryEditComponent', () => {
expect(component).toBeTruthy();
});

it('parses a server string as char-code-per-byte and emits Buffer-JSON on init', () => {
it('parses a server string as char-code-per-byte on init and does not emit', () => {
vi.spyOn(component.onFieldChange, 'emit');
fixture.componentRef.setInput('value', 'Hello');
component.ngOnInit();
// 'Hello' -> bytes 48 65 6c 6c 6f -> hex '48656c6c6f'
expect(component.hexData).toBe('48656c6c6f');
expect(component.onFieldChange.emit).toHaveBeenCalledWith({
type: 'Buffer',
data: [0x48, 0x65, 0x6c, 0x6c, 0x6f],
});
expect(component.onFieldChange.emit).not.toHaveBeenCalled();
});

it('re-emits an incoming Buffer-JSON value as Buffer-JSON on init', () => {
it('parses an incoming Buffer-JSON value into hex on init without emitting', () => {
vi.spyOn(component.onFieldChange, 'emit');
fixture.componentRef.setInput('value', { type: 'Buffer', data: [0x48, 0x65, 0x6c] });
component.ngOnInit();
expect(component.hexData).toBe('48656c');
expect(component.onFieldChange.emit).toHaveBeenCalledWith({
type: 'Buffer',
data: [0x48, 0x65, 0x6c],
});
expect(component.onFieldChange.emit).not.toHaveBeenCalled();
});

it('emits null and shows empty hex when incoming value is null', () => {
it('shows empty hex when incoming value is null and does not emit on init', () => {
vi.spyOn(component.onFieldChange, 'emit');
fixture.componentRef.setInput('value', null);
component.ngOnInit();
expect(component.hexData).toBe('');
expect(component.onFieldChange.emit).toHaveBeenCalledWith(null);
expect(component.onFieldChange.emit).not.toHaveBeenCalled();
});

it('emits Buffer-JSON when the user types valid hex', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export class BinaryEditComponent extends BaseEditFieldComponent implements OnIni
ngOnInit(): void {
super.ngOnInit();
this.hexData = bytesToHex(parseBinaryValue(this.value()));
this.emitCurrentValue();
}

onHexChange(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ export class ForeignKeyEditComponent extends BaseEditFieldComponent {
[primaeyKey.column_name]: res.rows[0][primaeyKey.column_name],
})),
);
this.onFieldChange.emit(this.currentFieldValue);
}
}

Expand Down
Loading