From b617791a02fa031fad1a60e5c32556c92777b09f Mon Sep 17 00:00:00 2001 From: omprakash Date: Wed, 26 Aug 2026 15:49:33 +0530 Subject: [PATCH] fix(ui): keep single-option preselect off Frappe reserved fields The Select/Link single-option preselect fired for every field, including the ones Frappe owns. Verified against frappe/develop rather than assumed: - amended_from Link -> self, read_only:1, NOT hidden (renders) - auto_repeat Link -> Auto Repeat, read_only:1, NOT hidden (renders) - parent_ Link -> self, neither hidden nor read-only - old_parent Link -> self, hidden:1 (never reaches a widget) - naming_series Select, assigned server-side at insert - name/owner/modified_by are std_fields pseudo-docfields; Frappe's `restricted` tuple refuses them as real DocFields parent_ is the dangerous one: editable and visible on every is_tree DocType, so a tree with one existing node silently acquired a parent the user never picked. naming_series matters too -- get_default_naming_series returns the first TRUTHY option and notes "Empty strings are used to avoid populating forms by default", so the server, not the client, picks it. The nestedset parent fieldname is frappe.scrub(f"Parent {self.name}"). frappe.scrub is only .replace(" ","_").replace("-","_").lower(), which is WEAKER than normalizeDoctypeTableName -- that one also collapses apostrophes, so reusing it would mispredict parent_item's_group as parent_item_s_group and miss the field. Ported separately as frappeScrub. Also fixes three preselect defects found alongside: - A single-option multi-select could not be unchecked. The preselect lived in build(), so the clear (which emits '') re-fired it on the next frame. Now gated on value == null, distinguishing "never set" from "explicitly cleared". With reqd:1 this had produced a required error while _formData still held the value, and formValues.addAll(_formData) let that win on save. - Preselect ignored readOnly/enabled, which is what actually reaches amended_from and auto_repeat. - Duplicate options crashed FormBuilderDropdown ("There should be exactly one item with [DropdownButton]'s value") and defeated preselect by making the count 2. Both Select options and LinkField's host-supplied static list are now deduped, order-preserving. allowPreselect defaults to true so a host constructing these widgets directly is unaffected. FieldFactory carries `doctype` as a mutable instance field rather than a createField parameter, per the extension contract documented on the class -- the override compat test still passes. Not addressed here: the dirty-on-open baseline is only partly improved. That is a dirty-baseline issue, not a preselect one (a Table MultiSelect field or any defaultValue dirties an untouched doc the same way), so it needs its own design call. Tests: 40 new across 6 files; 2453/2453 pass, analyze and format clean. --- lib/src/ui/widgets/fields/field_factory.dart | 21 ++ lib/src/ui/widgets/fields/link_field.dart | 47 +++- lib/src/ui/widgets/fields/select_field.dart | 62 ++++- lib/src/ui/widgets/form_builder.dart | 5 + lib/src/utils/frappe_reserved_fields.dart | 87 +++++++ .../fields/field_factory_preselect_test.dart | 82 ++++++ .../fields/link_field_preselect_test.dart | 211 ++++++++++++++++ .../fields/select_field_preselect_test.dart | 235 ++++++++++++++++++ .../widgets/form_builder_preselect_test.dart | 172 +++++++++++++ test/utils/frappe_reserved_fields_test.dart | 78 ++++++ 10 files changed, 978 insertions(+), 22 deletions(-) create mode 100644 lib/src/utils/frappe_reserved_fields.dart create mode 100644 test/ui/widgets/fields/field_factory_preselect_test.dart create mode 100644 test/ui/widgets/fields/link_field_preselect_test.dart create mode 100644 test/ui/widgets/fields/select_field_preselect_test.dart create mode 100644 test/ui/widgets/form_builder_preselect_test.dart create mode 100644 test/utils/frappe_reserved_fields_test.dart diff --git a/lib/src/ui/widgets/fields/field_factory.dart b/lib/src/ui/widgets/fields/field_factory.dart index 54510483..01a20326 100644 --- a/lib/src/ui/widgets/fields/field_factory.dart +++ b/lib/src/ui/widgets/fields/field_factory.dart @@ -11,6 +11,7 @@ import '../../../services/link_field_coordinator.dart'; import 'attach_field.dart'; import '../../../models/image_pick_source.dart'; import '../../../services/media_resolver.dart'; +import '../../../utils/frappe_reserved_fields.dart'; import '../../../utils/media_store.dart'; import 'base_field.dart'; import 'button_field.dart'; @@ -66,6 +67,7 @@ import 'time_field.dart'; /// - [mediaResolver] /// - [isOfflineMode] /// - [imagePickSource] +/// - [doctype] /// /// **Subclassing is the supported pattern; COMPOSITION is not.** A host that /// wraps an inner `FieldFactory` and delegates to it must forward every field @@ -84,6 +86,17 @@ class FieldFactory { LinkFieldCoordinator? linkFieldCoordinator; FieldStyle? defaultStyle; + /// The DocType whose form this factory is rendering, un-scrubbed (e.g. + /// `Item Group`). Assigned by `FrappeFormBuilder` from `meta.name`. + /// + /// Needed only to recognise the one reserved fieldname that is + /// doctype-dependent — `add_nestedset_fields()` names the tree parent Link + /// `frappe.scrub(f"Parent {self.name}")`. With it unset the fixed reserved + /// names are still caught; only `parent_` is missed, so a + /// host that forgets it degrades rather than breaks. See + /// [isFrappeReservedField]. + String? doctype; + /// When false, drop the length cap on `Data` fields entirely — Frappe stores /// **Single** doctypes as `mediumtext` and exempts them from the cap /// regardless of any explicit `DocField.length`. Default true (cap), matching @@ -250,6 +263,10 @@ class FieldFactory { onChanged: onChanged, enabled: enabled, style: fieldStyle, + allowPreselect: !isFrappeReservedField( + field.fieldname, + doctype: doctype, + ), ); case 'Table MultiSelect': @@ -334,6 +351,10 @@ class FieldFactory { getLinkFilterBuilder: getLinkFilterBuilder, style: fieldStyle, onIsLocalChanged: onIsLocalChanged, + allowPreselect: !isFrappeReservedField( + field.fieldname, + doctype: doctype, + ), ); case 'Table': diff --git a/lib/src/ui/widgets/fields/link_field.dart b/lib/src/ui/widgets/fields/link_field.dart index fb9abe3e..d4733f6d 100644 --- a/lib/src/ui/widgets/fields/link_field.dart +++ b/lib/src/ui/widgets/fields/link_field.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:collection'; import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; @@ -8,6 +9,7 @@ import '../../../models/link_filter_result.dart'; import '../../../services/link_option_service.dart'; import '../../../services/link_field_coordinator.dart'; import '../../../database/entities/link_option_entity.dart'; +import '../../../utils/frappe_reserved_fields.dart'; import '../../../utils/uuid_pattern.dart'; import 'field_helpers.dart'; import 'searchable_select.dart'; @@ -27,6 +29,16 @@ class LinkField extends BaseField { /// form data so [UuidRewriter] can rewrite the value at push time. final ValueChanged? onIsLocalChanged; + /// When false, the single-option preselect never fires for this field. + /// `FieldFactory` sets it from [isFrappeReservedField]. The Link fields + /// this actually protects are the framework-owned ones — `amended_from` + /// (`options: self`, `read_only`), `auto_repeat` (`options: Auto Repeat`, + /// `read_only`) and the `is_tree` `parent_` Link, which + /// is neither hidden nor read-only and so is the one that would otherwise + /// silently acquire a parent the user never picked. Defaults to true so a + /// host constructing this widget directly keeps the previous behaviour. + final bool allowPreselect; + const LinkField({ super.key, required super.field, @@ -41,17 +53,26 @@ class LinkField extends BaseField { this.parentFormData = const {}, this.getLinkFilterBuilder, this.onIsLocalChanged, + this.allowPreselect = true, }); @override Widget buildField(BuildContext context) { // If options are provided directly, use them if (options != null && options!.isNotEmpty) { + // Deduplicated, order-preserving. [options] is a `createField` parameter + // the SDK never populates itself, so the list is whatever the host + // passed — and `DropdownButton` asserts when two `DropdownMenuItem`s + // share the value it is showing ("There should be exactly one item with + // [DropdownButton]'s value"). Deduping also restores the preselect for a + // sole option written twice: the count is 1 again, not 2. Mirrors + // `SelectField._getRawOptions`. + final staticOptions = LinkedHashSet.of(options!).toList(); // Validate initialValue is in options list final initialValueStr = value?.toString(); String? validInitialValue; if (initialValueStr != null && initialValueStr.isNotEmpty) { - if (options!.contains(initialValueStr)) { + if (staticOptions.contains(initialValueStr)) { validInitialValue = initialValueStr; } else { // Value not in options - use null @@ -63,12 +84,15 @@ class LinkField extends BaseField { // Propagate `onIsLocalChanged` so an auto-picked UUID-shaped // option (offline mobile_uuid) flips `__is_local` for // UuidRewriter at push time — matches `_applyOptionsAndAutoSelect`. - if (options!.length == 1 && - (validInitialValue == null || validInitialValue.isEmpty)) { - validInitialValue = options!.first; + if (staticOptions.length == 1 && + (validInitialValue == null || validInitialValue.isEmpty) && + allowPreselect && + enabled && + !field.readOnly) { + validInitialValue = staticOptions.first; WidgetsBinding.instance.addPostFrameCallback((_) { - onChanged?.call(options!.first); - onIsLocalChanged?.call(looksLikeMobileUuid(options!.first)); + onChanged?.call(staticOptions.first); + onIsLocalChanged?.call(looksLikeMobileUuid(staticOptions.first)); }); } @@ -85,14 +109,14 @@ class LinkField extends BaseField { ); return FormBuilderDropdown( autovalidateMode: AutovalidateMode.onUserInteraction, - key: ValueKey('link_${field.fieldname}_${options!.length}'), + key: ValueKey('link_${field.fieldname}_${staticOptions.length}'), name: field.fieldname ?? '', initialValue: validInitialValue, enabled: enabled && !field.readOnly, isExpanded: true, decoration: tap.decoration, padding: tap.padding, - items: options! + items: staticOptions .map( (option) => DropdownMenuItem(value: option, child: Text(option)), ) @@ -138,6 +162,7 @@ class LinkField extends BaseField { style: style, onIsLocalChanged: onIsLocalChanged, errorText: state.errorText, + allowPreselect: allowPreselect, ); }, ); @@ -182,6 +207,7 @@ class _LinkFieldDropdown extends StatefulWidget { final FieldStyle? style; final ValueChanged? onIsLocalChanged; final String? errorText; + final bool allowPreselect; const _LinkFieldDropdown({ required this.field, @@ -198,6 +224,7 @@ class _LinkFieldDropdown extends StatefulWidget { this.style, this.onIsLocalChanged, this.errorText, + this.allowPreselect = true, }); @override @@ -256,7 +283,9 @@ class _LinkFieldDropdownState extends State<_LinkFieldDropdown> { _isLoading = false; _waitingForDependent = false; }); - if (options.length == 1) { + // `widget.enabled` already folds in `field.readOnly` (see the call site in + // [LinkField.buildField]), so this one clause covers both. + if (options.length == 1 && widget.allowPreselect && widget.enabled) { final currentVal = widget.value?.toString(); final hasValidSelection = currentVal != null && diff --git a/lib/src/ui/widgets/fields/select_field.dart b/lib/src/ui/widgets/fields/select_field.dart index b5934d3e..0522d980 100644 --- a/lib/src/ui/widgets/fields/select_field.dart +++ b/lib/src/ui/widgets/fields/select_field.dart @@ -1,11 +1,22 @@ +import 'dart:collection'; + import 'package:flutter/material.dart'; import 'package:flutter_form_builder/flutter_form_builder.dart'; +import '../../../utils/frappe_reserved_fields.dart'; import '../../../utils/translate.dart'; import 'base_field.dart'; import 'field_helpers.dart'; /// Widget for Select field type. Supports single and multi-select (when field.allowMultiple). class SelectField extends BaseField { + /// When false, the single-option preselect below never fires for this + /// field. `FieldFactory` sets it from [isFrappeReservedField] so a + /// framework-owned slot (`naming_series`, `amended_from`, the `is_tree` + /// `parent_` Link, …) is never filled with a value the user did not + /// choose. Defaults to true so a host constructing this widget directly + /// keeps the previous behaviour. + final bool allowPreselect; + const SelectField({ super.key, required super.field, @@ -13,18 +24,41 @@ class SelectField extends BaseField { super.onChanged, super.enabled, super.style, + this.allowPreselect = true, }); /// Raw (untranslated) option keys — used as stored document values. + /// + /// Deduplicated, order-preserving: `DropdownButton` asserts when two + /// `DropdownMenuItem`s share the value it is showing ("There should be + /// exactly one item with [DropdownButton]'s value"), so a DocType whose + /// `options` repeats a line would crash the field outright. Deduping also + /// restores the preselect for a sole option that happens to be written + /// twice — the count is 1 again, not 2. List _getRawOptions() { if (field.options == null || field.options!.isEmpty) return []; - return field.options! - .split('\n') - .map((e) => e.trim()) - .where((e) => e.isNotEmpty) - .toList(); + return LinkedHashSet.of( + field.options! + .split('\n') + .map((e) => e.trim()) + .where((e) => e.isNotEmpty), + ).toList(); } + /// Whether the single-option preselect may fire. + /// + /// `value == null` is the load-bearing clause. The form holds no entry for a + /// field it has never been given a value for, but an EXPLICIT clear stores + /// `''` (multi-select emits `_listToValue([])`). Preselecting on "no valid + /// selection" alone made the two indistinguishable, so unchecking the sole + /// option of a multi-select re-fired the preselect on the very next build + /// and pushed the value back — while `FormBuilderCheckboxGroup`, whose + /// `ValueKey` had not changed, stayed visibly unchecked. The widget and the + /// form data then disagreed, and `_handleSubmit`'s + /// `formValues.addAll(_formData)` let the form data win. + bool get _canPreselect => + allowPreselect && enabled && !field.readOnly && value == null; + /// Translated display labels — used only for rendering. List _getOptions() { final raw = _getRawOptions(); @@ -80,12 +114,13 @@ class SelectField extends BaseField { .where((v) => rawOptions.contains(v)) .toList(); - // Auto-select when exactly one option and no valid selection. - // Use raw English key for the stored value. - final displayList = rawOptions.length == 1 && validInitialList.isEmpty - ? [rawOptions.first] - : validInitialList; - if (rawOptions.length == 1 && validInitialList.isEmpty) { + // Preselect when exactly one option and nothing is selected yet. + // Use raw English key for the stored value. See [_canPreselect] for why + // an explicitly-cleared value ('') is excluded. + final preselect = + rawOptions.length == 1 && validInitialList.isEmpty && _canPreselect; + final displayList = preselect ? [rawOptions.first] : validInitialList; + if (preselect) { WidgetsBinding.instance.addPostFrameCallback((_) { onChanged?.call(_listToValue([rawOptions.first])); }); @@ -132,10 +167,11 @@ class SelectField extends BaseField { } } - // Auto-select when exactly one option and no valid selection. + // Preselect when exactly one option and nothing is selected yet. // Emit raw English key — never a translated label. if (rawOptions.length == 1 && - (validInitialValue == null || validInitialValue.isEmpty)) { + (validInitialValue == null || validInitialValue.isEmpty) && + _canPreselect) { validInitialValue = rawOptions.first; WidgetsBinding.instance.addPostFrameCallback((_) { onChanged?.call(rawOptions.first); diff --git a/lib/src/ui/widgets/form_builder.dart b/lib/src/ui/widgets/form_builder.dart index 60647b6e..27fe6788 100644 --- a/lib/src/ui/widgets/form_builder.dart +++ b/lib/src/ui/widgets/form_builder.dart @@ -463,6 +463,11 @@ class _FrappeFormBuilderState extends State // Frappe stores Single doctypes as mediumtext and exempts them from the // implicit Data varchar(140) cap. _fieldFactory.capDataLength = !widget.meta.isSingle; + // Un-scrubbed DocType name. Lets the factory recognise the `is_tree` + // `parent_` Link — the only reserved fieldname that is + // not the same on every DocType — and keep the single-option preselect off + // it. See [FieldFactory.doctype]. + _fieldFactory.doctype = widget.meta.name; _fieldFactory.errorTextResolver = _inlineTableErrorFor; // The six attachment capabilities below are assigned ONLY when this widget // was actually given one. An unconditional assignment clobbers a host that diff --git a/lib/src/utils/frappe_reserved_fields.dart b/lib/src/utils/frappe_reserved_fields.dart new file mode 100644 index 00000000..71dd8293 --- /dev/null +++ b/lib/src/utils/frappe_reserved_fields.dart @@ -0,0 +1,87 @@ +/// Frappe's reserved / framework-owned fieldnames, and the `frappe.scrub` +/// helper needed to derive the one that is doctype-dependent. +/// +/// Used to keep the single-option preselect (see `SelectField` / `LinkField`) +/// off fields the framework owns: preselecting them writes a value the user +/// never chose into a slot Frappe assigns itself. +library; + +/// Dart port of `frappe.scrub` (frappe/utils/data.py): +/// +/// ```python +/// def scrub(txt: str) -> str: +/// return cstr(txt).replace(" ", "_").replace("-", "_").lower() +/// ``` +/// +/// Deliberately NOT [normalizeDoctypeTableName] (`database/table_name.dart`), +/// which additionally collapses every non-alphanumeric run to a single `_` and +/// strips leading/trailing `_`. That is correct for a SQLite identifier and +/// wrong here: `frappe.scrub` leaves apostrophes, slashes and repeated +/// separators alone, so a DocType named `Item's Group` yields the nestedset +/// parent field `parent_item's_group` — the table-name normalizer would +/// mispredict `parent_item_s_group` and the guard would miss the field. +String frappeScrub(String txt) => + txt.replaceAll(' ', '_').replaceAll('-', '_').toLowerCase(); + +/// Reserved fieldnames that are the same on every DocType. +/// +/// Split by origin, because the two halves reach a form differently: +/// +/// **`std_fields` pseudo-docfields** — `name`, `owner`, `modified_by` are +/// declared `Link` only in `frappe/model/__init__.py`'s `std_fields` list, to +/// render the Report Builder / filter UI. They are not DocFields (on disk they +/// are plain `varchar(140)`), and `Meta.get_link_fields()` — +/// `self.get("fields", {"fieldtype": "Link", "options": ["!=", "[Select]"]})` — +/// scans DocFields only, so they never appear in it. Frappe additionally +/// REFUSES to create a DocField with these names (`scrub_field_names()` throws +/// `InvalidFieldNameError` for a `restricted` tuple that contains all three), +/// so they cannot reach `meta.fields` at all. Listed anyway as a cheap +/// belt-and-braces: the SDK renders whatever meta it is handed. +/// +/// **Conditional real DocFields** — added by `frappe/core/doctype/doctype/ +/// doctype.py` when the DocType opts in, and therefore genuinely present in +/// `meta.fields`: +/// +/// | Field | Fieldtype | Options | Added when | Flags | +/// |----------------|-----------|---------------|-------------------------|--------------| +/// | `amended_from` | Link | self | `is_submittable` | `read_only` | +/// | `old_parent` | Link | self | `is_tree` | `hidden` | +/// | `auto_repeat` | Link | `Auto Repeat` | `allow_auto_repeat` | `read_only` | +/// +/// `naming_series` is a Select and is convention rather than injection — the +/// DocType author declares it — but the name is reserved by that convention. +/// Frappe assigns it SERVER-SIDE at insert (`set_name_by_naming_series` → +/// `get_default_naming_series`, which returns the first TRUTHY option and +/// carries the comment *"Empty strings are used to avoid populating forms by +/// default"*). A client that preselects it defeats that. +/// +/// The `is_tree` `parent_` field is doctype-dependent and so +/// is not in this set — see [isFrappeReservedField]. +const frappeReservedFieldNames = { + // std_fields pseudo-docfields (also in Frappe's `restricted` tuple). + 'name', + 'owner', + 'modified_by', + // Conditional real DocFields. + 'amended_from', + 'old_parent', + 'auto_repeat', + // Convention. + 'naming_series', +}; + +/// Whether [fieldname] is a Frappe reserved / framework-owned field. +/// +/// Pass [doctype] (the DocType's `name`, un-scrubbed) to also catch the +/// nestedset parent Link that `add_nestedset_fields()` adds to every +/// `is_tree` DocType. Frappe builds that fieldname as +/// `frappe.scrub(f"Parent {self.name}")`, so `Item Group` yields +/// `parent_item_group`. Without [doctype] the field is indistinguishable from +/// an ordinary `parent_*` field and is NOT flagged — `parent_company` on a +/// non-tree DocType is a perfectly normal user Link. +bool isFrappeReservedField(String? fieldname, {String? doctype}) { + if (fieldname == null || fieldname.isEmpty) return false; + if (frappeReservedFieldNames.contains(fieldname)) return true; + if (doctype == null || doctype.trim().isEmpty) return false; + return fieldname == frappeScrub('Parent $doctype'); +} diff --git a/test/ui/widgets/fields/field_factory_preselect_test.dart b/test/ui/widgets/fields/field_factory_preselect_test.dart new file mode 100644 index 00000000..38d74db8 --- /dev/null +++ b/test/ui/widgets/fields/field_factory_preselect_test.dart @@ -0,0 +1,82 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_field.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/fields/field_factory.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/fields/link_field.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/fields/select_field.dart'; + +DocField _select(String fieldname) => DocField( + fieldname: fieldname, + fieldtype: 'Select', + label: fieldname, + options: 'Only', +); + +DocField _link(String fieldname) => DocField( + fieldname: fieldname, + fieldtype: 'Link', + label: fieldname, + options: 'Some DocType', +); + +void main() { + group('FieldFactory threads the reserved-field decision into the widget', () { + test('an ordinary Select is allowed to preselect', () { + final f = FieldFactory()..doctype = 'Item Group'; + final w = f.createField(field: _select('status')) as SelectField; + expect(w.allowPreselect, isTrue); + }); + + test('naming_series is not', () { + final f = FieldFactory()..doctype = 'Item Group'; + final w = f.createField(field: _select('naming_series')) as SelectField; + expect(w.allowPreselect, isFalse); + }); + + test('amended_from is not', () { + final f = FieldFactory()..doctype = 'Item Group'; + final w = + f.createField(field: _link('amended_from'), linkOptions: ['X']) + as LinkField; + expect(w.allowPreselect, isFalse); + }); + + test('auto_repeat is not', () { + final f = FieldFactory()..doctype = 'Item Group'; + final w = + f.createField(field: _link('auto_repeat'), linkOptions: ['X']) + as LinkField; + expect(w.allowPreselect, isFalse); + }); + + test('the nestedset parent Link of THIS doctype is not', () { + final f = FieldFactory()..doctype = 'Item Group'; + final w = + f.createField(field: _link('parent_item_group'), linkOptions: ['X']) + as LinkField; + expect(w.allowPreselect, isFalse); + }); + + test('a parent_* Link of a DIFFERENT doctype is allowed', () { + final f = FieldFactory()..doctype = 'Sales Order'; + final w = + f.createField(field: _link('parent_item_group'), linkOptions: ['X']) + as LinkField; + expect(w.allowPreselect, isTrue); + }); + + test('with no doctype set, only the fixed reserved names are caught', () { + final f = FieldFactory(); + expect( + (f.createField(field: _select('naming_series')) as SelectField) + .allowPreselect, + isFalse, + ); + expect( + (f.createField(field: _link('parent_item_group'), linkOptions: ['X']) + as LinkField) + .allowPreselect, + isTrue, + ); + }); + }); +} diff --git a/test/ui/widgets/fields/link_field_preselect_test.dart b/test/ui/widgets/fields/link_field_preselect_test.dart new file mode 100644 index 00000000..b3534a8e --- /dev/null +++ b/test/ui/widgets/fields/link_field_preselect_test.dart @@ -0,0 +1,211 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_form_builder/flutter_form_builder.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/frappe_mobile_sdk.dart'; + +class _FakeLinkOptionService extends LinkOptionService { + final Completer> _completer = Completer(); + _FakeLinkOptionService() : super.withoutResolver(); + + @override + Future> getLinkOptions( + String doctype, { + List>? filters, + }) => _completer.future; + + void resolve(List options) { + if (!_completer.isCompleted) _completer.complete(options); + } +} + +LinkOptionEntity _opt(String name) => LinkOptionEntity( + doctype: 'TestDocType', + name: name, + label: name, + lastUpdated: 0, +); + +Widget _wrap(Widget child) => MaterialApp( + home: Scaffold(body: FormBuilder(child: child)), +); + +DocField _field({bool readOnly = false, String options = 'TestDocType'}) => + DocField( + fieldname: 'test_link', + fieldtype: 'Link', + label: 'Test Link', + options: options, + readOnly: readOnly, + ); + +void main() { + group('static options branch', () { + testWidgets('does not preselect when allowPreselect is false', ( + tester, + ) async { + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + options: const ['Only'], + allowPreselect: false, + onChanged: emissions.add, + ), + ), + ); + await tester.pumpAndSettle(); + expect(emissions, isEmpty); + }); + + testWidgets('does not preselect a readOnly field', (tester) async { + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(readOnly: true), + options: const ['Only'], + onChanged: emissions.add, + ), + ), + ); + await tester.pumpAndSettle(); + expect(emissions, isEmpty); + }); + + testWidgets('does not preselect a disabled field', (tester) async { + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + options: const ['Only'], + enabled: false, + onChanged: emissions.add, + ), + ), + ); + await tester.pumpAndSettle(); + expect(emissions, isEmpty); + }); + + testWidgets('still preselects an ordinary editable field', (tester) async { + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + options: const ['Only'], + onChanged: emissions.add, + ), + ), + ); + await tester.pumpAndSettle(); + expect(emissions, ['Only']); + }); + }); + + group('service-loaded options branch', () { + testWidgets('does not preselect when allowPreselect is false', ( + tester, + ) async { + final svc = _FakeLinkOptionService(); + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + linkOptionService: svc, + allowPreselect: false, + onChanged: emissions.add, + ), + ), + ); + svc.resolve([_opt('Only')]); + await tester.pumpAndSettle(); + expect(emissions, isEmpty); + }); + + testWidgets('does not preselect a readOnly field', (tester) async { + final svc = _FakeLinkOptionService(); + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(readOnly: true), + linkOptionService: svc, + onChanged: emissions.add, + ), + ), + ); + svc.resolve([_opt('Only')]); + await tester.pumpAndSettle(); + expect(emissions, isEmpty); + }); + + testWidgets('still preselects an ordinary editable field', (tester) async { + final svc = _FakeLinkOptionService(); + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + linkOptionService: svc, + onChanged: emissions.add, + ), + ), + ); + svc.resolve([_opt('Only')]); + await tester.pumpAndSettle(); + expect(emissions, ['Only']); + }); + }); + + _duplicateOptionTests(); +} + +// --------------------------------------------------------------------------- +// Duplicate host-supplied static options. `linkOptions` is a `createField` +// parameter the SDK never populates itself, so the list is whatever the host +// passes — and it reaches the same FormBuilderDropdown that asserts +// "There should be exactly one item with [DropdownButton]'s value". +// --------------------------------------------------------------------------- +void _duplicateOptionTests() { + group('duplicate static options', () { + testWidgets('do not crash the dropdown', (tester) async { + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + value: 'A', + options: const ['A', 'A', 'B'], + onChanged: (_) {}, + ), + ), + ); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + final w = tester.widget>( + find.byType(FormBuilderDropdown), + ); + expect(w.items, hasLength(2)); + }); + + testWidgets('a duplicated sole option still preselects', (tester) async { + final emissions = []; + await tester.pumpWidget( + _wrap( + LinkField( + field: _field(), + options: const ['Only', 'Only'], + onChanged: emissions.add, + ), + ), + ); + await tester.pumpAndSettle(); + expect(emissions, ['Only']); + }); + }); +} diff --git a/test/ui/widgets/fields/select_field_preselect_test.dart b/test/ui/widgets/fields/select_field_preselect_test.dart new file mode 100644 index 00000000..6a018797 --- /dev/null +++ b/test/ui/widgets/fields/select_field_preselect_test.dart @@ -0,0 +1,235 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_form_builder/flutter_form_builder.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_field.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/fields/select_field.dart'; + +/// Mirrors the real host (`FrappeFormBuilder`): stores whatever the field +/// emits and rebuilds the field with it. Without the round-trip, a preselect +/// that re-fires on every build looks identical to one that fires once. +class _Host extends StatefulWidget { + const _Host({ + super.key, + required this.field, + this.initial, + this.enabled = true, + this.allowPreselect = true, + }); + + final DocField field; + final dynamic initial; + final bool enabled; + final bool allowPreselect; + + @override + State<_Host> createState() => _HostState(); +} + +class _HostState extends State<_Host> { + late dynamic value = widget.initial; + final List emissions = []; + + @override + Widget build(BuildContext context) { + return MaterialApp( + home: Scaffold( + body: FormBuilder( + child: SelectField( + field: widget.field, + value: value, + enabled: widget.enabled, + allowPreselect: widget.allowPreselect, + onChanged: (v) { + emissions.add(v); + setState(() => value = v); + }, + ), + ), + ), + ); + } +} + +DocField _select({ + required String options, + bool multi = false, + bool readOnly = false, + bool reqd = false, +}) => DocField( + fieldname: 'status', + fieldtype: 'Select', + label: 'Status', + options: options, + allowMultiple: multi, + readOnly: readOnly, + reqd: reqd, +); + +void main() { + group('preselect is suppressed for a field the host excludes', () { + testWidgets('single-select does not emit when allowPreselect is false', ( + tester, + ) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only'), + allowPreselect: false, + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, isEmpty); + }); + + testWidgets('multi-select does not emit when allowPreselect is false', ( + tester, + ) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only', multi: true), + allowPreselect: false, + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, isEmpty); + }); + }); + + group('preselect respects readOnly and enabled', () { + testWidgets('a readOnly single-option select does not emit', ( + tester, + ) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only', readOnly: true), + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, isEmpty); + }); + + testWidgets('a disabled single-option select does not emit', ( + tester, + ) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only'), + enabled: false, + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, isEmpty); + }); + + testWidgets('a readOnly single-option MULTI select does not emit', ( + tester, + ) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only', multi: true, readOnly: true), + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, isEmpty); + }); + }); + + group('preselect fires once, not on every rebuild', () { + testWidgets('unchecking the sole option sticks', (tester) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only', multi: true), + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, [ + 'Only', + ], reason: 'preselect must fire once on mount'); + + await tester.tap(find.byType(Checkbox).first); + await tester.pumpAndSettle(); + + expect( + key.currentState!.emissions, + ['Only', ''], + reason: 'the uncheck must not be followed by a re-preselect', + ); + expect( + key.currentState!.value, + '', + reason: 'form data must agree with the unchecked box', + ); + expect( + tester.widget(find.byType(Checkbox).first).value, + isFalse, + ); + }); + + testWidgets('an explicitly cleared value is not re-preselected on mount', ( + tester, + ) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + initial: '', + field: _select(options: 'Only', multi: true), + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, isEmpty); + }); + + testWidgets('an absent value IS preselected on mount', (tester) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only'), + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, ['Only']); + }); + }); + + group('duplicate options', () { + testWidgets('a duplicated sole option still preselects', (tester) async { + final key = GlobalKey<_HostState>(); + await tester.pumpWidget( + _Host( + key: key, + field: _select(options: 'Only\nOnly'), + ), + ); + await tester.pumpAndSettle(); + expect(key.currentState!.emissions, ['Only']); + }); + + testWidgets('duplicate options do not crash the dropdown', (tester) async { + await tester.pumpWidget( + _Host( + initial: 'A', + field: _select(options: 'A\nA\nB'), + ), + ); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + final w = tester.widget>( + find.byType(FormBuilderDropdown), + ); + expect(w.items, hasLength(2)); + }); + }); +} diff --git a/test/ui/widgets/form_builder_preselect_test.dart b/test/ui/widgets/form_builder_preselect_test.dart new file mode 100644 index 00000000..77fcd6f3 --- /dev/null +++ b/test/ui/widgets/form_builder_preselect_test.dart @@ -0,0 +1,172 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_field.dart'; +import 'package:frappe_mobile_sdk/src/models/doc_type_meta.dart'; +import 'package:frappe_mobile_sdk/src/ui/form/form_controller.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/form_builder.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/fields/link_field.dart'; +import 'package:frappe_mobile_sdk/src/ui/widgets/fields/select_field.dart'; + +DocTypeMeta _meta(String name, List fields) => + DocTypeMeta(name: name, fields: fields); + +DocField _select(String fieldname) => DocField( + fieldname: fieldname, + fieldtype: 'Select', + label: fieldname, + options: 'Only', +); + +DocField _link(String fieldname) => DocField( + fieldname: fieldname, + fieldtype: 'Link', + label: fieldname, + options: 'Item Group', +); + +void main() { + testWidgets( + 'FrappeFormBuilder passes meta.name down so the tree parent Link and ' + 'naming_series are excluded from preselect', + (tester) async { + final changed = {}; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: FrappeFormBuilder( + meta: _meta('Item Group', [ + _select('naming_series'), + _select('status'), + _link('parent_item_group'), + ]), + onFieldChange: (name, value, data, {source = ChangeSource.user}) { + changed[name] = value; + return null; + }, + ), + ), + ), + ); + await tester.pumpAndSettle(); + + SelectField selectFor(String fieldname) => tester + .widgetList(find.byType(SelectField)) + .firstWhere((w) => w.field.fieldname == fieldname); + + expect(selectFor('naming_series').allowPreselect, isFalse); + expect(selectFor('status').allowPreselect, isTrue); + + final parentLink = tester + .widgetList(find.byType(LinkField)) + .firstWhere((w) => w.field.fieldname == 'parent_item_group'); + expect(parentLink.allowPreselect, isFalse); + + // The ordinary Select still preselects; the reserved one stays untouched. + expect(changed['status'], 'Only'); + expect(changed.containsKey('naming_series'), isFalse); + expect(changed.containsKey('parent_item_group'), isFalse); + }, + ); + + testWidgets( + 'unchecking a preselected sole option survives the real form pipeline', + (tester) async { + // The stub host in select_field_preselect_test.dart feeds the emitted + // value straight back. The real path also runs + // `patchValue({tags: FieldNormalizer.normalize(field, '')})` -> `[]` + // into FormBuilderCheckboxGroup, which is the half of the original bug + // that made the widget and _formData disagree. This exercises it. + Map? submitted; + void Function()? submitFn; + final changed = {}; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: FrappeFormBuilder( + meta: _meta('Item Group', [ + DocField( + fieldname: 'tags', + fieldtype: 'Select', + label: 'Tags', + options: 'Only', + allowMultiple: true, + ), + ]), + onSubmit: (data) => submitted = data, + registerSubmit: (fn) => submitFn = fn, + onFieldChange: (name, value, data, {source = ChangeSource.user}) { + changed[name] = value; + return null; + }, + ), + ), + ), + ); + await tester.pumpAndSettle(); + expect(changed['tags'], 'Only', reason: 'preselect fires once on mount'); + + await tester.tap(find.byType(Checkbox).first); + await tester.pumpAndSettle(); + + expect( + tester.widget(find.byType(Checkbox).first).value, + isFalse, + reason: 'the box the user unchecked must stay unchecked', + ); + expect( + changed['tags'], + '', + reason: 'the last change the form saw must be the clear', + ); + + submitFn!.call(); + await tester.pumpAndSettle(); + expect(submitted, isNotNull); + expect( + submitted!['tags'], + anyOf(isNull, '', isEmpty), + reason: 'form data must not resurrect the preselected value on save', + ); + }, + ); + + testWidgets( + 'reactive mode goes through the same factory, so the gate still applies', + (tester) async { + // The reactive build path has its OWN createField call site + // (form_builder.dart:2292) separate from the legacy one at 1455. Both + // read the same _fieldFactory, but only `build` configures it — this + // pins that the reactive path is not missed. + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: FrappeFormBuilder( + mode: FormBuilderMode.reactive, + meta: _meta('Item Group', [ + _select('naming_series'), + _select('status'), + _link('parent_item_group'), + ]), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + SelectField selectFor(String fieldname) => tester + .widgetList(find.byType(SelectField)) + .firstWhere((w) => w.field.fieldname == fieldname); + + expect(selectFor('naming_series').allowPreselect, isFalse); + expect(selectFor('status').allowPreselect, isTrue); + expect( + tester + .widgetList(find.byType(LinkField)) + .firstWhere((w) => w.field.fieldname == 'parent_item_group') + .allowPreselect, + isFalse, + ); + }, + ); +} diff --git a/test/utils/frappe_reserved_fields_test.dart b/test/utils/frappe_reserved_fields_test.dart new file mode 100644 index 00000000..63cb97ca --- /dev/null +++ b/test/utils/frappe_reserved_fields_test.dart @@ -0,0 +1,78 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:frappe_mobile_sdk/src/utils/frappe_reserved_fields.dart'; + +void main() { + group('frappeScrub', () { + // Mirrors frappe/utils/data.py: + // return cstr(txt).replace(" ", "_").replace("-", "_").lower() + test('replaces spaces and hyphens with underscore and lowercases', () { + expect(frappeScrub('Sales Order'), 'sales_order'); + expect(frappeScrub('Cost-Center'), 'cost_center'); + expect(frappeScrub('Parent Item Group'), 'parent_item_group'); + }); + + test( + 'leaves other punctuation intact, unlike the table-name normalizer', + () { + // normalizeDoctypeTableName() collapses "'" to "_"; frappe.scrub does + // not. The nestedset parent fieldname follows frappe.scrub. + expect(frappeScrub("Item's Group"), "item's_group"); + }, + ); + + test('does not collapse repeated separators', () { + expect(frappeScrub('A B'), 'a__b'); + }); + }); + + group('isFrappeReservedField', () { + test('flags the std_fields pseudo-docfields', () { + expect(isFrappeReservedField('name'), isTrue); + expect(isFrappeReservedField('owner'), isTrue); + expect(isFrappeReservedField('modified_by'), isTrue); + }); + + test('flags the conditional real DocFields', () { + expect(isFrappeReservedField('amended_from'), isTrue); + expect(isFrappeReservedField('old_parent'), isTrue); + expect(isFrappeReservedField('auto_repeat'), isTrue); + }); + + test('flags naming_series', () { + expect(isFrappeReservedField('naming_series'), isTrue); + }); + + test('flags the nestedset parent field for the given doctype', () { + expect( + isFrappeReservedField('parent_item_group', doctype: 'Item Group'), + isTrue, + ); + expect( + isFrappeReservedField('parent_cost_center', doctype: 'Cost-Center'), + isTrue, + ); + }); + + test('does not flag the nestedset parent field of a DIFFERENT doctype', () { + expect( + isFrappeReservedField('parent_item_group', doctype: 'Sales Order'), + isFalse, + ); + }); + + test('does not flag the nestedset parent field with no doctype given', () { + expect(isFrappeReservedField('parent_item_group'), isFalse); + }); + + test('does not flag ordinary fieldnames', () { + expect(isFrappeReservedField('status'), isFalse); + expect(isFrappeReservedField('customer'), isFalse); + expect(isFrappeReservedField('parent_company'), isFalse); + }); + + test('does not flag null or empty', () { + expect(isFrappeReservedField(null), isFalse); + expect(isFrappeReservedField(''), isFalse); + }); + }); +}