From 01ca6da166c8f45d23c633dbbad65591d2744cdd Mon Sep 17 00:00:00 2001 From: Ashwin Date: Fri, 7 Aug 2026 20:11:01 +0530 Subject: [PATCH] Add unit tests for custom fields and macros functionality - Implemented comprehensive tests for custom fields, covering key validation, draft validation, payload creation, and parsing. - Added tests for macros, including permission checks, removal actions, draft validation, and payload structure. - Established a fake HTTP client to simulate API responses for testing purposes. - Ensured that all edge cases and expected behaviors are thoroughly validated in both custom fields and macros. --- mobile/lib/config/api_config.dart | 26 +- .../data/models/custom_field_definition.dart | 231 ++++++- mobile/lib/data/models/macro.dart | 179 ++++++ mobile/lib/providers/auth_provider.dart | 10 + mobile/lib/providers/lookup_provider.dart | 9 + mobile/lib/providers/settings_provider.dart | 360 +++++++++++ mobile/lib/routes/app_router.dart | 28 + .../settings/custom_field_form_sheet.dart | 453 ++++++++++++++ .../settings/custom_fields_screen.dart | 486 +++++++++++++++ .../screens/settings/macro_form_sheet.dart | 247 ++++++++ .../lib/screens/settings/macros_screen.dart | 436 +++++++++++++ mobile/lib/screens/settings/more_screen.dart | 103 +-- .../screens/settings/settings_hub_screen.dart | 92 +++ .../screens/tickets/macro_picker_sheet.dart | 262 ++++++++ .../screens/tickets/ticket_detail_screen.dart | 28 + mobile/lib/widgets/common/common.dart | 1 + mobile/lib/widgets/common/menu_row.dart | 74 +++ mobile/test/screens/phone_viewport_test.dart | 465 ++++++++++++++ mobile/test/unit/custom_fields_test.dart | 585 ++++++++++++++++++ mobile/test/unit/macros_test.dart | 454 ++++++++++++++ 20 files changed, 4441 insertions(+), 88 deletions(-) create mode 100644 mobile/lib/data/models/macro.dart create mode 100644 mobile/lib/providers/settings_provider.dart create mode 100644 mobile/lib/screens/settings/custom_field_form_sheet.dart create mode 100644 mobile/lib/screens/settings/custom_fields_screen.dart create mode 100644 mobile/lib/screens/settings/macro_form_sheet.dart create mode 100644 mobile/lib/screens/settings/macros_screen.dart create mode 100644 mobile/lib/screens/settings/settings_hub_screen.dart create mode 100644 mobile/lib/screens/tickets/macro_picker_sheet.dart create mode 100644 mobile/lib/widgets/common/menu_row.dart create mode 100644 mobile/test/unit/custom_fields_test.dart create mode 100644 mobile/test/unit/macros_test.dart diff --git a/mobile/lib/config/api_config.dart b/mobile/lib/config/api_config.dart index 733e89a9..7d8edc75 100644 --- a/mobile/lib/config/api_config.dart +++ b/mobile/lib/config/api_config.dart @@ -385,9 +385,33 @@ class ApiConfig { static String get tags => '$apiBaseUrl/tags/'; /// Custom field definitions (per-org schema for entities like Case/Lead/...). - /// Query with `?target_model=Case&active_only=true`. + /// Query with `?target_model=Case&active_only=true`. Pass + /// `include_counts=false` unless you need `records_missing_value`: computing + /// it costs a full scan of the org's records per definition. static String get customFieldDefinitions => '$apiBaseUrl/custom-fields/'; + /// Saved replies. GET lists the ones visible to the caller: every org macro + /// plus their own personal ones, never anybody else's. `?active=true` for + /// the reply-box picker, unfiltered for the settings screen, which has to + /// see the turned-off rows to offer turning one back on. + static String get macros => '$apiBaseUrl/macros/'; + + /// One saved reply. PATCH edits it (PUT runs the serializer non-partial and + /// 400s without both title and body). DELETE turns an org macro off and + /// deletes a personal one for good, decided server-side from the row. + static String macro(String id) => '$apiBaseUrl/macros/$id/'; + + /// Substitute the `%token%` placeholders against a ticket and return the + /// text: POST `{"case_id": ""}`. Server-side on purpose, so the + /// supported token set lives in one place and no client can drift from it. + static String macroRender(String id) => '$apiBaseUrl/macros/$id/render/'; + + /// One definition: PUT edits it, DELETE turns it off. DELETE is a soft + /// delete (`CustomFieldDefinitionDetailView.delete` flips `is_active` and + /// leaves stored values readable), so nothing here says "delete" to a user. + /// Both verbs are admin-only server-side. + static String customField(String id) => '$apiBaseUrl/custom-fields/$id/'; + /// People in the org: GET lists active and inactive, POST invites. /// Admin-only server-side, 403 for everyone else on both verbs. static String get orgUsers => '$apiBaseUrl/users/'; diff --git a/mobile/lib/data/models/custom_field_definition.dart b/mobile/lib/data/models/custom_field_definition.dart index d97ffb4a..b2c85beb 100644 --- a/mobile/lib/data/models/custom_field_definition.dart +++ b/mobile/lib/data/models/custom_field_definition.dart @@ -11,9 +11,24 @@ class CustomFieldDefinition { final CustomFieldType fieldType; final List options; final bool isRequired; + + /// Whether the field is offered as a list filter. Read by the settings + /// screen only; the form renderer has no use for it. + final bool isFilterable; + final int displayOrder; final bool isActive; + /// How many records of [targetModel] predate this field and carry no value + /// for it. Computed server-side and only present when the request did not + /// pass `include_counts=false`, so it is 0 on the form-rendering path. + /// + /// This is the number the settings screen exists to show: marking a field + /// required binds writes from that moment on and backfills nothing, so + /// "Required" beside a non-zero count is a promise the stored data does not + /// keep. + final int recordsMissingValue; + const CustomFieldDefinition({ required this.id, required this.targetModel, @@ -22,8 +37,10 @@ class CustomFieldDefinition { required this.fieldType, this.options = const [], this.isRequired = false, + this.isFilterable = false, this.displayOrder = 0, this.isActive = true, + this.recordsMissingValue = 0, }); factory CustomFieldDefinition.fromJson(Map json) { @@ -51,8 +68,10 @@ class CustomFieldDefinition { fieldType: CustomFieldType.fromString(json['field_type'] as String?), options: parsedOptions, isRequired: json['is_required'] as bool? ?? false, + isFilterable: json['is_filterable'] as bool? ?? false, displayOrder: json['display_order'] as int? ?? 0, isActive: json['is_active'] as bool? ?? true, + recordsMissingValue: json['records_missing_value'] as int? ?? 0, ); } } @@ -64,15 +83,20 @@ class CustomFieldOption { } enum CustomFieldType { - text('text'), - textarea('textarea'), - number('number'), - dropdown('dropdown'), - date('date'), - checkbox('checkbox'); + text('text', 'Text'), + textarea('textarea', 'Long text'), + number('number', 'Number'), + dropdown('dropdown', 'Dropdown'), + date('date', 'Date'), + checkbox('checkbox', 'Checkbox'); final String value; - const CustomFieldType(this.value); + + /// Matches `FIELD_TYPE_LABEL` in `frontend/src/lib/v2/enums.js`, so the two + /// clients name the same type the same way. + final String label; + + const CustomFieldType(this.value, this.label); static CustomFieldType fromString(String? value) { return CustomFieldType.values.firstWhere( @@ -81,3 +105,196 @@ enum CustomFieldType { ); } } + +/// The record types the backend will attach a custom field to, with the names +/// this app already uses for them elsewhere (`Case` is a ticket, `Opportunity` +/// is a deal). +/// +/// Mirrors `SUPPORTED_TARGETS` in `backend/common/custom_fields.py` and +/// `TARGET_MODEL_LABEL` in `frontend/src/lib/v2/enums.js`. +/// `CustomFieldDefinitionSerializer.validate_target_model` rejects anything +/// outside this set, so offering a wider list would only produce a 400. +const Map customFieldTargets = { + 'Account': 'Accounts', + 'Case': 'Tickets', + 'Contact': 'Contacts', + 'Estimate': 'Estimates', + 'Invoice': 'Invoices', + 'Lead': 'Leads', + 'Opportunity': 'Deals', + 'RecurringInvoice': 'Recurring invoices', + 'Task': 'Tasks', +}; + +String customFieldTargetLabel(String targetModel) => + customFieldTargets[targetModel] ?? targetModel; + +/// Whether [key] is a key the server will accept. +/// +/// `CustomFieldDefinitionSerializer.validate_key` requires +/// `[a-z][a-z0-9_]*`. Hyphens are the trap: they read as natural in a slug and +/// are refused, so the form says so rather than letting the save fail. +bool isValidCustomFieldKey(String key) => + RegExp(r'^[a-z][a-z0-9_]*$').hasMatch(key); + +/// A starting key derived from the label, for the create form only. +/// +/// Typing a lowercase underscored slug on a phone keyboard is several mode +/// switches, so the field is pre-filled and still editable. It is a default, +/// not a rule: whatever is in the box at submit time is what gets sent, and +/// [isValidCustomFieldKey] is what decides whether it may be. +/// +/// Returns `''` when nothing usable survives (a label of only punctuation, or +/// one starting with a digit, which the server's leading-letter rule refuses), +/// leaving the admin to type one. +String suggestCustomFieldKey(String label) { + final slug = label + .toLowerCase() + .replaceAll(RegExp(r'[^a-z0-9]+'), '_') + .replaceAll(RegExp(r'^_+|_+$'), ''); + if (slug.isEmpty) return ''; + return isValidCustomFieldKey(slug) ? slug : ''; +} + +/// "Very High" to "very-high", for a NEW dropdown option only. +/// +/// An existing option's value is stored on every record that uses it, so a +/// relabel must leave the value untouched. This mirrors `slugifyOptionValue` +/// in `frontend/src/lib/server/v2/custom-fields.js`, hyphens included: option +/// values are free-form on the server and only the field `key` is slug-checked. +String slugifyOptionValue(String label) => label + .toLowerCase() + .trim() + .replaceAll(RegExp(r'[^a-z0-9]+'), '-') + .replaceAll(RegExp(r'^-+|-+$'), ''); + +/// What a custom-field form has to say before it can be submitted, or `null`. +/// +/// Extracted from the form so it is testable without driving the widget, and +/// so the create and edit paths cannot drift: both call this. +/// +/// [isCreate] decides whether [key] and [targetModel] are checked at all. They +/// are frozen after creation (`CustomFieldDefinitionSerializer.validate`), so +/// the edit form does not offer them and has nothing to validate. +String? validateCustomFieldDraft({ + required bool isCreate, + required String label, + required String key, + required String targetModel, + required CustomFieldType fieldType, + required List options, +}) { + if (label.trim().isEmpty) return 'Give the field a label.'; + if (isCreate) { + if (key.trim().isEmpty) return 'Give the field a key.'; + if (!isValidCustomFieldKey(key.trim())) { + return 'A key is lowercase letters, numbers and underscores, ' + 'starting with a letter. No hyphens or spaces.'; + } + if (!customFieldTargets.containsKey(targetModel)) { + return 'Choose the record type this field belongs to.'; + } + } + if (fieldType == CustomFieldType.dropdown) { + final rows = options.where((o) => o.label.trim().isNotEmpty).toList(); + if (rows.isEmpty) return 'A dropdown needs at least one option.'; + final seen = {}; + for (final row in rows) { + final value = row.value.isNotEmpty + ? row.value + : slugifyOptionValue(row.label); + if (value.isEmpty) { + return 'An option needs at least one letter or number in its name.'; + } + if (!seen.add(value)) { + return 'Two options would both be stored as "$value". Rename one.'; + } + } + } + return null; +} + +/// The body for `POST /custom-fields/`. +/// +/// `org` is set from the JWT server-side and `records_missing_value` is +/// computed, so neither appears here. `is_active` is left off too: the model +/// defaults it to true and a new field is always on. +/// +/// `options` rides only on a dropdown. `validate_definition_options` refuses a +/// non-empty options list on any other type, so sending one turns a valid text +/// field into a 400. +Map customFieldCreatePayload({ + required String targetModel, + required String key, + required String label, + required CustomFieldType fieldType, + required bool isRequired, + required bool isFilterable, + required int displayOrder, + required List options, +}) { + final body = { + 'target_model': targetModel, + 'key': key.trim(), + 'label': label.trim(), + 'field_type': fieldType.value, + 'is_required': isRequired, + 'is_filterable': isFilterable, + 'display_order': displayOrder, + }; + if (fieldType == CustomFieldType.dropdown) { + body['options'] = _optionRows(options); + } + return body; +} + +/// The body for `PUT /custom-fields//`. +/// +/// `key`, `target_model` and `field_type` are absent on purpose. +/// `CustomFieldDefinitionSerializer.validate` refuses a change to any of the +/// three with a 400, and it is right to: values live in each record's +/// `custom_fields` JSON keyed by `key`, so changing the key orphans them, +/// changing the target leaves them on the old entity, and changing the type +/// reinterprets values written under the old one. +Map customFieldUpdatePayload({ + required String label, + required CustomFieldType fieldType, + required bool isRequired, + required bool isFilterable, + required int displayOrder, + required List options, +}) { + final body = { + 'label': label.trim(), + 'is_required': isRequired, + 'is_filterable': isFilterable, + 'display_order': displayOrder, + }; + if (fieldType == CustomFieldType.dropdown) { + body['options'] = _optionRows(options); + } + return body; +} + +/// The body for turning a field back on. +/// +/// Only `is_active`, and that is the whole point. Reusing the edit payload +/// would send `label` too, and a reactivate control has no label to send: the +/// field would come back with its name blanked. The web learned this the same +/// way, which is why its `activate` action calls `updateCustomField` directly +/// instead of going through its form reader. +Map customFieldActivatePayload() => {'is_active': true}; + +List> _optionRows(List options) { + final rows = >[]; + for (final option in options) { + final label = option.label.trim(); + if (label.isEmpty) continue; + final value = option.value.isNotEmpty + ? option.value + : slugifyOptionValue(label); + if (value.isEmpty) continue; + rows.add({'value': value, 'label': label}); + } + return rows; +} diff --git a/mobile/lib/data/models/macro.dart b/mobile/lib/data/models/macro.dart new file mode 100644 index 00000000..13b0e015 --- /dev/null +++ b/mobile/lib/data/models/macro.dart @@ -0,0 +1,179 @@ +/// A saved reply, from `MacroSerializer`. +/// +/// Two scopes with genuinely different rules, and the difference is not +/// cosmetic. An `org` macro is shared, admin-managed, and `DELETE` only turns +/// it off. A `personal` macro belongs to one person and `DELETE` removes it for +/// good. Both go through the same endpoint and the server decides which +/// happens from the row's own scope, so the client's only job is to say the +/// right thing before asking. +class Macro { + const Macro({ + required this.id, + this.title = '', + this.body = '', + this.scope = scopeOrg, + this.ownerEmail, + this.isActive = true, + this.usageCount = 0, + this.unknownPlaceholders = const [], + }); + + static const String scopeOrg = 'org'; + static const String scopePersonal = 'personal'; + + final String id; + final String title; + final String body; + final String scope; + + /// `owner_name`, which the serializer fills from the owner's email because + /// `User` carries no display name. Null on an org macro, which has no owner. + final String? ownerEmail; + + final bool isActive; + final int usageCount; + + /// `%token%` placeholders the server's renderer does not expand, computed + /// server-side by `find_unknown_placeholders`. + /// + /// Passed through rather than recomputed. Recomputing here would mean this + /// app deciding which tokens are broken, and it would drift from the set the + /// renderer actually expands the first time either changed. Unknown tokens + /// are not a save-blocker: they render literally into the reply, which is + /// why a macro carrying one is worth flagging. + final List unknownPlaceholders; + + bool get isPersonal => scope == scopePersonal; + + /// "Everyone" / "Just you", matching `MACRO_SCOPE_LABEL` in + /// `frontend/src/lib/v2/enums.js`. + String get scopeLabel => isPersonal ? 'Just you' : 'Everyone'; + + factory Macro.fromJson(Map json) { + final unknown = json['unknown_placeholders']; + return Macro( + id: json['id']?.toString() ?? '', + title: json['title'] as String? ?? '', + body: json['body'] as String? ?? '', + scope: json['scope']?.toString() ?? scopeOrg, + ownerEmail: (json['owner_name'] as String?)?.trim().isEmpty ?? true + ? null + : json['owner_name'] as String?, + isActive: json['is_active'] as bool? ?? true, + usageCount: json['usage_count'] as int? ?? 0, + unknownPlaceholders: unknown is List + ? unknown.map((t) => t.toString()).toList(growable: false) + : const [], + ); + } +} + +/// Whether the signed-in user may edit or remove this macro. +/// +/// Mirrors `MacroDetailView._get_writable`, which is the only authority: an +/// org macro needs an admin, a personal macro needs its owner, and an admin +/// gets no exception on someone else's personal macro (that answers 404, not +/// 403, so the id space cannot be used to discover whose private macros +/// exist). +/// +/// Ownership is compared on email rather than assumed from the list. The list +/// endpoint does already exclude other people's personal macros, so today +/// every personal row belongs to the caller, but that is a property of the +/// query and this is a rule about the row. Deriving one from the other would +/// put an authorization answer at the mercy of a filter change. +/// +/// Returns false when either email is unknown, so an unparsed payload hides +/// the action rather than offering a write that answers 404. +bool canWriteMacro({ + required bool isAdmin, + required String scope, + required String? ownerEmail, + required String? myEmail, +}) { + if (scope == Macro.scopeOrg) return isAdmin; + final owner = ownerEmail?.trim().toLowerCase(); + final me = myEmail?.trim().toLowerCase(); + if (owner == null || owner.isEmpty || me == null || me.isEmpty) return false; + return owner == me; +} + +/// What removing this macro actually does, in the words the confirm dialog +/// needs. +/// +/// `MacroDetailView.delete` soft-deletes an org macro (`is_active` flips, the +/// row stays and stays counted) and hard-deletes a personal one. One endpoint, +/// two outcomes, decided server-side. A dialog that says "Delete" over a soft +/// delete, or "Turn off" over a permanent one, is the same defect either way. +class MacroRemoval { + const MacroRemoval({ + required this.actionLabel, + required this.title, + required this.detail, + required this.isPermanent, + }); + + final String actionLabel; + final String title; + final String detail; + final bool isPermanent; +} + +MacroRemoval macroRemoval(Macro macro) { + if (macro.isPersonal) { + return MacroRemoval( + actionLabel: 'Delete', + title: 'Delete ${macro.title}?', + detail: + 'This is one of your own saved replies, so it is removed for ' + 'good. Nobody else could see it.', + isPermanent: true, + ); + } + return MacroRemoval( + actionLabel: 'Turn off', + title: 'Turn off ${macro.title}?', + detail: + 'It stops being offered in the reply box for everyone. The saved ' + 'reply is kept and you can turn it back on.', + isPermanent: false, + ); +} + +/// What a macro form has to say before it can be submitted, or `null`. +/// +/// `MacroSerializer` requires both `title` and `body`, and +/// `_resolve_scope_and_owner` rejects any scope outside the two. This is a +/// fast fail so a blank body does not cost a round trip. +String? validateMacroDraft({ + required String title, + required String body, + required String scope, +}) { + if (title.trim().isEmpty) return 'Give the saved reply a title.'; + if (body.trim().isEmpty) return 'A saved reply needs something to say.'; + if (scope != Macro.scopeOrg && scope != Macro.scopePersonal) { + return 'Choose who this reply is for.'; + } + return null; +} + +/// The body for `POST /macros/` and `PATCH /macros//`. +/// +/// `owner` is absent and must stay absent. It is `read_only` on the serializer +/// and re-derived by `_resolve_scope_and_owner` from `request.profile`; a +/// client that could name one could file a saved reply as somebody else. +/// `org` is a JWT claim for the same reason. `usage_count` is server-owned. +Map macroPayload({ + required String title, + required String body, + required String scope, +}) { + return {'title': title.trim(), 'body': body.trim(), 'scope': scope}; +} + +/// The body for turning an org macro back on. +/// +/// Only `is_active`, and it goes as a PATCH. Reusing [macroPayload] would need +/// a title and a body the reactivate control does not have. `_get_writable` +/// still applies, so a non-admin reactivating an org macro is refused. +Map macroActivatePayload() => {'is_active': true}; diff --git a/mobile/lib/providers/auth_provider.dart b/mobile/lib/providers/auth_provider.dart index 2e5b9920..2c0d2162 100644 --- a/mobile/lib/providers/auth_provider.dart +++ b/mobile/lib/providers/auth_provider.dart @@ -340,6 +340,16 @@ final isOrgAdminProvider = Provider((ref) { return (org?.role ?? '').toUpperCase() == 'ADMIN'; }); +/// The signed-in user's email, or null. +/// +/// The one identifier this app and the API agree on for a person. Several +/// serializers surface an owner or author as an email rather than an id +/// (`User` carries no display name), so an ownership comparison in the UI has +/// to be made on this. UI affordances only; see `core/permissions.dart`. +final myEmailProvider = Provider((ref) { + return ref.watch(authProvider).user?.email; +}); + /// Provider for checking if org selection is needed final needsOrgSelectionProvider = Provider((ref) { return ref.watch(authProvider).needsOrgSelection; diff --git a/mobile/lib/providers/lookup_provider.dart b/mobile/lib/providers/lookup_provider.dart index 65d2ef2b..4f5360b7 100644 --- a/mobile/lib/providers/lookup_provider.dart +++ b/mobile/lib/providers/lookup_provider.dart @@ -313,6 +313,14 @@ final ticketEntityOptionsProvider = Provider>>( /// /// Each form watches the family for its entity; results are cached per-target /// for the lifetime of the provider scope so reopening the same form is cheap. +/// +/// `include_counts=false` matters. Without it the endpoint also computes +/// `records_missing_value` for every definition the org has, which is one +/// COUNT over the org's records per target model plus a +/// `custom_fields ? key` scan per field. A form that only wants labels and +/// types does not need those numbers, and paying for them on every lead, deal +/// and task form open puts full scans on the record path. The settings screen +/// is the one caller that wants them and asks separately. final customFieldDefinitionsProvider = FutureProvider.family, String>(( ref, @@ -323,6 +331,7 @@ final customFieldDefinitionsProvider = queryParameters: { 'target_model': targetModel, 'active_only': 'true', + 'include_counts': 'false', }, ) .toString(); diff --git a/mobile/lib/providers/settings_provider.dart b/mobile/lib/providers/settings_provider.dart new file mode 100644 index 00000000..d52ff52f --- /dev/null +++ b/mobile/lib/providers/settings_provider.dart @@ -0,0 +1,360 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../config/api_config.dart'; +import '../data/api_envelope.dart'; +import '../data/models/custom_field_definition.dart'; +import '../data/models/macro.dart'; +import '../services/api_service.dart'; + +/// The org settings cluster. +/// +/// Custom fields is the first of the cluster's pages to reach the phone. The +/// rest (routing, escalation, inbound email, ticket approvals, macros, tags, +/// business hours, reopen rules) share the same shape, so they will land +/// beside this rather than in files of their own. +/// +/// Every write here is admin-only server-side. The screens hide the controls +/// from everyone else, which is UX: `CustomFieldDefinitionListCreateView.post` +/// and `CustomFieldDefinitionDetailView.put`/`.delete` each call +/// `is_org_admin` first and answer 403, so hiding a button is not what keeps a +/// member out. + +/// The server's own message where there is one. These endpoints refuse for +/// reasons a user can act on ("Admin access required", "key must be a +/// lowercase slug starting with a letter"). +String _message(dynamic response) { + final errors = response.data?['errors']; + if (errors is Map) { + for (final value in errors.values) { + if (value is List && value.isNotEmpty) return value.first.toString(); + if (value is String && value.trim().isNotEmpty) return value; + } + } + if (errors is String && errors.trim().isNotEmpty) return errors; + final detail = response.data?['message']; + if (detail is String && detail.trim().isNotEmpty) return detail; + final raw = response.message as String?; + return (raw == null || raw.trim().isEmpty) ? 'Something went wrong' : raw; +} + +/// The definition list plus the counts the settings screen shows above it. +class CustomFieldsState { + const CustomFieldsState({ + this.fields = const [], + this.count = 0, + this.active = 0, + this.modelsExtended = 0, + this.requiredWithGaps = 0, + }); + + final List fields; + + /// From the server's `totals` block, which is computed over the org's whole + /// definition set rather than the rows on screen. Recomputing these from + /// `fields` would give the same answer today and quietly diverge the moment + /// the list is filtered. + final int count; + final int active; + final int modelsExtended; + + /// Required fields that some records have no value for. The banner exists + /// because marking a field required binds writes from that point on and + /// backfills nothing. + final int requiredWithGaps; + + /// Fields grouped by the record type they extend, since "what do we collect + /// on a ticket" is how anyone looks for one. Groups are ordered by the + /// target's display name; fields inside a group by `display_order`, then + /// label, matching the order the record forms render them in. + List>> get grouped { + final byTarget = >{}; + for (final field in fields) { + byTarget.putIfAbsent(field.targetModel, () => []).add(field); + } + final entries = byTarget.entries.toList() + ..sort( + (a, b) => customFieldTargetLabel( + a.key, + ).compareTo(customFieldTargetLabel(b.key)), + ); + for (final entry in entries) { + entry.value.sort((a, b) { + final byOrder = a.displayOrder.compareTo(b.displayOrder); + return byOrder != 0 ? byOrder : a.label.compareTo(b.label); + }); + } + return entries; + } +} + +class CustomFieldsNotifier extends AsyncNotifier { + final ApiService _api = ApiService(); + + @override + Future build() => _fetch(); + + Future refresh() async { + state = const AsyncValue.loading(); + state = await AsyncValue.guard(_fetch); + } + + /// No `active_only`, because this is the screen that turns a field back on + /// and cannot offer that for a row it never fetched. No `include_counts` + /// either: `records_missing_value` is the number this page is here to show, + /// so it is the one caller that wants the scan. + Future _fetch() async { + final response = await _api.get(ApiConfig.customFieldDefinitions); + if (!response.success || response.data == null) { + throw Exception(response.message ?? 'Failed to load custom fields'); + } + final body = response.data!; + final fields = listFromEnvelope(body, const [ + 'definitions', + ]).map(CustomFieldDefinition.fromJson).toList(growable: false); + final totals = body['totals']; + return CustomFieldsState( + fields: fields, + count: _int(totals, 'count', fields.length), + active: _int(totals, 'active', fields.where((f) => f.isActive).length), + modelsExtended: _int( + totals, + 'models_extended', + fields + .where((f) => f.isActive) + .map((f) => f.targetModel) + .toSet() + .length, + ), + requiredWithGaps: _int( + totals, + 'required_with_gaps', + fields.where((f) => f.isRequired && f.recordsMissingValue > 0).length, + ), + ); + } + + Future createField(Map payload) async { + final response = await _api.post(ApiConfig.customFieldDefinitions, payload); + if (!response.success) return _message(response); + await refresh(); + return null; + } + + Future updateField(String id, Map payload) async { + final response = await _api.put(ApiConfig.customField(id), payload); + if (!response.success) return _message(response); + await refresh(); + return null; + } + + /// Turn a field off. `DELETE` is a soft delete server-side: it flips + /// `is_active` and leaves every stored value readable, which is why nothing + /// on the screen calls this Delete. + Future deactivateField(String id) async { + final response = await _api.delete(ApiConfig.customField(id)); + if (!response.success) return _message(response); + await refresh(); + return null; + } + + /// Turn a field back on. There is no reactivate endpoint, so this is the + /// same PUT carrying only `is_active`. See [customFieldActivatePayload] for + /// why it must not reuse the edit body. + Future activateField(String id) => + updateField(id, customFieldActivatePayload()); +} + +final customFieldsProvider = + AsyncNotifierProvider( + CustomFieldsNotifier.new, + ); + +// --------------------------------------------------------------------------- +// Macros, the saved replies +// --------------------------------------------------------------------------- + +/// The macros visible to the caller, plus the placeholder reference. +/// +/// Visibility is the server's decision: `_visible_qs` returns every org macro +/// plus the caller's own personal ones, so nothing here filters rows. A +/// stranger's personal macro never arrives. +class MacrosState { + const MacrosState({ + this.macros = const [], + this.placeholders = const [], + this.orgCount = 0, + this.personalCount = 0, + this.inactiveCount = 0, + this.brokenCount = 0, + }); + + final List macros; + + /// `{token, resolves}` pairs from `SUPPORTED_PLACEHOLDERS`. Server-owned, so + /// the reference the form shows is exactly what the renderer expands. + final List placeholders; + + final int orgCount; + final int personalCount; + final int inactiveCount; + + /// Macros carrying a `%token%` the renderer does not know. Those render + /// literally into a customer reply, which is why they are counted. + final int brokenCount; +} + +class MacroPlaceholder { + const MacroPlaceholder({required this.token, required this.resolves}); + + final String token; + final String resolves; +} + +class MacrosNotifier extends AsyncNotifier { + final ApiService _api = ApiService(); + + @override + Future build() => _fetch(); + + Future refresh() async { + state = const AsyncValue.loading(); + state = await AsyncValue.guard(_fetch); + } + + /// Unfiltered on purpose. The reply-box picker asks for `?active=true`, but + /// this screen is the one that turns a macro back on and cannot offer that + /// for a row it never fetched. + Future _fetch() async { + final response = await _api.get(ApiConfig.macros); + if (!response.success || response.data == null) { + throw Exception(response.message ?? 'Failed to load saved replies'); + } + final body = response.data!; + final macros = listFromEnvelope(body, const [ + 'results', + ]).map(Macro.fromJson).toList(growable: false); + final totals = body['totals']; + final rawPlaceholders = body['placeholders']; + return MacrosState( + macros: macros, + placeholders: rawPlaceholders is List + ? [ + for (final p in rawPlaceholders) + if (p is Map) + MacroPlaceholder( + token: p['token']?.toString() ?? '', + resolves: p['resolves']?.toString() ?? '', + ), + ] + : const [], + orgCount: _int(totals, 'org', macros.where((m) => !m.isPersonal).length), + personalCount: _int( + totals, + 'personal', + macros.where((m) => m.isPersonal).length, + ), + inactiveCount: _int( + totals, + 'inactive', + macros.where((m) => !m.isActive).length, + ), + brokenCount: _int( + totals, + 'with_unknown_placeholders', + macros.where((m) => m.unknownPlaceholders.isNotEmpty).length, + ), + ); + } + + Future createMacro(Map payload) async { + final response = await _api.post(ApiConfig.macros, payload); + if (!response.success) return _macroMessage(response); + await refresh(); + return null; + } + + /// PATCH, not PUT. `MacroDetailView.put` runs the serializer with + /// `partial=False` and `MacroSerializer` requires both `title` and `body`, + /// so a PUT missing either is a 400 for no reason the user could see. + Future updateMacro(String id, Map payload) async { + final response = await _api.patch(ApiConfig.macro(id), payload); + if (!response.success) return _macroMessage(response); + await refresh(); + return null; + } + + /// Turn an org macro off, or delete a personal one for good. One endpoint; + /// which happens is the row's scope, decided server-side. + Future removeMacro(String id) async { + final response = await _api.delete(ApiConfig.macro(id)); + if (!response.success) return _macroMessage(response); + await refresh(); + return null; + } + + Future activateMacro(String id) => + updateMacro(id, macroActivatePayload()); +} + +final macrosProvider = AsyncNotifierProvider( + MacrosNotifier.new, +); + +/// The active macros, for the reply box. +/// +/// A separate read from [macrosProvider] rather than a filter over it: the +/// picker wants `?active=true` server-side and must not pull the turned-off +/// rows onto a ticket screen at all, and the two are opened from different +/// places at different times. +final activeMacrosProvider = FutureProvider>((ref) async { + final url = Uri.parse( + ApiConfig.macros, + ).replace(queryParameters: {'active': 'true'}).toString(); + final response = await ApiService().get(url); + if (!response.success || response.data == null) { + throw Exception(response.message ?? 'Failed to load saved replies'); + } + return listFromEnvelope(response.data!, const [ + 'results', + ]).map(Macro.fromJson).toList(growable: false); +}); + +/// Expand a macro against a ticket and return the text to insert. +/// +/// Server-side substitution: `render_macro` owns the supported token set, and +/// an unknown token is left literal so the agent sees it rather than sending a +/// reply with a silent hole in it. Returns the message on failure instead of +/// throwing, matching the write methods above. +Future<({String? text, String? error})> renderMacro({ + required String macroId, + required String ticketId, +}) async { + final response = await ApiService().post(ApiConfig.macroRender(macroId), { + 'case_id': ticketId, + }); + if (!response.success) { + return (text: null, error: _macroMessage(response)); + } + final rendered = response.data?['rendered_body']; + if (rendered is! String) { + return (text: null, error: 'The saved reply came back empty.'); + } + return (text: rendered, error: null); +} + +/// The macro endpoints answer `{"error": "..."}` and `{"detail": "..."}` where +/// the rest of the app answers `{"errors": ...}`, so [_message] alone would +/// miss both. The wording matters here: "Only admins can manage org-scope +/// macros" is the difference between a bug and a rule. +String _macroMessage(dynamic response) { + final error = response.data?['error']; + if (error is String && error.trim().isNotEmpty) return error; + final detail = response.data?['detail']; + if (detail is String && detail.trim().isNotEmpty) return detail; + return _message(response); +} + +int _int(dynamic totals, String key, int fallback) { + if (totals is Map && totals[key] is int) return totals[key] as int; + return fallback; +} diff --git a/mobile/lib/routes/app_router.dart b/mobile/lib/routes/app_router.dart index cfb34b1b..182bfae0 100644 --- a/mobile/lib/routes/app_router.dart +++ b/mobile/lib/routes/app_router.dart @@ -35,8 +35,11 @@ import '../screens/tasks/board_screen.dart'; import '../screens/tasks/tasks_list_screen.dart'; import '../screens/tasks/task_detail_screen.dart'; import '../screens/tasks/task_form_screen.dart'; +import '../screens/settings/custom_fields_screen.dart'; +import '../screens/settings/macros_screen.dart'; import '../screens/settings/more_screen.dart'; import '../screens/settings/profile_screen.dart'; +import '../screens/settings/settings_hub_screen.dart'; import '../screens/settings/team_screen.dart'; import '../screens/solutions/solutions_list_screen.dart'; import '../screens/solutions/solution_detail_screen.dart'; @@ -99,6 +102,13 @@ class AppRoutes { static const String profile = '/more/profile'; static const String team = '/more/team'; + /// The org settings cluster. Two segments deep under `/more`, matching + /// `profile` and `team`, so nothing here can be mistaken for a record id the + /// way `/invoices/estimates` once could. + static const String settings = '/more/settings'; + static const String settingsCustomFields = '/more/settings/custom-fields'; + static const String settingsMacros = '/more/settings/macros'; + /// Your own week of logged time. Not under `/more`: it is a workspace /// destination reached from the dashboard as often as from the menu, and a /// path that says where it lives is the one that survives being linked to. @@ -404,6 +414,24 @@ final appRouterProvider = Provider((ref) { parentNavigatorKey: _rootNavigatorKey, builder: (context, state) => const TeamScreen(), ), + GoRoute( + path: AppRoutes.settings, + name: 'settings', + parentNavigatorKey: _rootNavigatorKey, + builder: (context, state) => const SettingsHubScreen(), + ), + GoRoute( + path: AppRoutes.settingsCustomFields, + name: 'settingsCustomFields', + parentNavigatorKey: _rootNavigatorKey, + builder: (context, state) => const CustomFieldsScreen(), + ), + GoRoute( + path: AppRoutes.settingsMacros, + name: 'settingsMacros', + parentNavigatorKey: _rootNavigatorKey, + builder: (context, state) => const MacrosScreen(), + ), GoRoute( path: AppRoutes.orgCreate, name: 'orgCreate', diff --git a/mobile/lib/screens/settings/custom_field_form_sheet.dart b/mobile/lib/screens/settings/custom_field_form_sheet.dart new file mode 100644 index 00000000..2ea44069 --- /dev/null +++ b/mobile/lib/screens/settings/custom_field_form_sheet.dart @@ -0,0 +1,453 @@ +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../core/theme/theme.dart'; +import '../../data/models/custom_field_definition.dart'; + +/// Add or edit a custom field definition. +/// +/// Returns the request body to send, or `null` if the sheet was dismissed. It +/// returns a payload rather than a `CustomFieldDefinition` because create and +/// edit send genuinely different bodies: the edit body must not carry `key`, +/// `target_model` or `field_type`, and a model instance cannot express that +/// absence. See [customFieldCreatePayload] and [customFieldUpdatePayload]. +/// +/// Every rule checked here is also checked by +/// `CustomFieldDefinitionSerializer`. This is a courtesy so a hyphen in the key +/// is caught before a round trip, not a guard: the server's 400 still arrives +/// and is shown when it disagrees. +Future?> showCustomFieldFormSheet( + BuildContext context, + CustomFieldDefinition? existing, +) { + return showModalBottomSheet>( + context: context, + isScrollControlled: true, + builder: (context) => _CustomFieldFormSheet(existing: existing), + ); +} + +class _CustomFieldFormSheet extends StatefulWidget { + const _CustomFieldFormSheet({this.existing}); + + final CustomFieldDefinition? existing; + + @override + State<_CustomFieldFormSheet> createState() => _CustomFieldFormSheetState(); +} + +class _CustomFieldFormSheetState extends State<_CustomFieldFormSheet> { + late final TextEditingController _label; + late final TextEditingController _key; + late final TextEditingController _order; + late String _targetModel; + late CustomFieldType _fieldType; + late bool _isRequired; + late bool _isFilterable; + + /// Option rows for a dropdown. An existing row keeps its stored `value` + /// untouched, because every record already holding that option refers to it; + /// a row added here has an empty value and gets one slugified from its label + /// at submit time. + late List<_OptionDraft> _options; + + /// True once the admin edits the key by hand, after which the label stops + /// writing into it. Without this, correcting the label would silently + /// overwrite a key they had deliberately chosen. + bool _keyTouched = false; + + String? _error; + + bool get _isCreate => widget.existing == null; + + @override + void initState() { + super.initState(); + final f = widget.existing; + _label = TextEditingController(text: f?.label ?? ''); + _key = TextEditingController(text: f?.key ?? ''); + _order = TextEditingController(text: '${f?.displayOrder ?? 0}'); + _targetModel = f?.targetModel ?? customFieldTargets.keys.first; + _fieldType = f?.fieldType ?? CustomFieldType.text; + _isRequired = f?.isRequired ?? false; + _isFilterable = f?.isFilterable ?? false; + _options = [ + for (final o in f?.options ?? const []) + _OptionDraft(value: o.value, label: o.label), + ]; + } + + @override + void dispose() { + _label.dispose(); + _key.dispose(); + _order.dispose(); + for (final option in _options) { + option.controller.dispose(); + } + super.dispose(); + } + + void _onLabelChanged(String value) { + if (!_isCreate || _keyTouched) return; + final suggestion = suggestCustomFieldKey(value); + if (suggestion == _key.text) return; + _key.value = TextEditingValue( + text: suggestion, + selection: TextSelection.collapsed(offset: suggestion.length), + ); + } + + void _addOption() { + setState(() => _options.add(_OptionDraft(value: '', label: ''))); + } + + void _removeOption(int index) { + setState(() => _options.removeAt(index).controller.dispose()); + } + + void _submit() { + final options = [ + for (final o in _options) + CustomFieldOption(value: o.value, label: o.controller.text), + ]; + final problem = validateCustomFieldDraft( + isCreate: _isCreate, + label: _label.text, + key: _key.text, + targetModel: _targetModel, + fieldType: _fieldType, + options: options, + ); + if (problem != null) { + setState(() => _error = problem); + return; + } + + final displayOrder = int.tryParse(_order.text.trim()) ?? 0; + Navigator.of(context).pop( + _isCreate + ? customFieldCreatePayload( + targetModel: _targetModel, + key: _key.text, + label: _label.text, + fieldType: _fieldType, + isRequired: _isRequired, + isFilterable: _isFilterable, + displayOrder: displayOrder, + options: options, + ) + : customFieldUpdatePayload( + label: _label.text, + fieldType: _fieldType, + isRequired: _isRequired, + isFilterable: _isFilterable, + displayOrder: displayOrder, + options: options, + ), + ); + } + + @override + Widget build(BuildContext context) { + return Padding( + padding: EdgeInsets.only( + left: 16, + right: 16, + top: 16, + bottom: MediaQuery.of(context).viewInsets.bottom + 16, + ), + child: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + _isCreate ? 'New custom field' : 'Edit ${widget.existing!.label}', + style: AppTypography.h3.copyWith(fontWeight: FontWeight.w600), + ), + const SizedBox(height: 16), + TextField( + controller: _label, + onChanged: _onLabelChanged, + textCapitalization: TextCapitalization.sentences, + maxLength: 128, + decoration: const InputDecoration( + labelText: 'Label', + helperText: 'What the field is called on the record', + border: OutlineInputBorder(), + counterText: '', + ), + ), + const SizedBox(height: 12), + if (_isCreate) + TextField( + controller: _key, + onChanged: (_) => _keyTouched = true, + maxLength: 64, + inputFormatters: [ + FilteringTextInputFormatter.allow(RegExp(r'[a-z0-9_]')), + ], + decoration: const InputDecoration( + labelText: 'Key', + helperText: + 'Lowercase letters, numbers and underscores. ' + 'Cannot be changed later.', + helperMaxLines: 2, + border: OutlineInputBorder(), + counterText: '', + ), + ) + else + _Frozen( + label: 'Key', + value: widget.existing!.key, + note: + 'Fixed after creation. Every value already stored is filed ' + 'under this key, and changing it would leave them behind.', + ), + const SizedBox(height: 12), + if (_isCreate) + DropdownButtonFormField( + initialValue: _targetModel, + isExpanded: true, + decoration: const InputDecoration( + labelText: 'On record type', + border: OutlineInputBorder(), + ), + items: [ + for (final entry in customFieldTargets.entries) + DropdownMenuItem( + value: entry.key, + child: Text(entry.value), + ), + ], + onChanged: (v) => + setState(() => _targetModel = v ?? _targetModel), + ) + else + _Frozen( + label: 'On record type', + value: customFieldTargetLabel(widget.existing!.targetModel), + note: 'Fixed after creation.', + ), + const SizedBox(height: 12), + if (_isCreate) + DropdownButtonFormField( + initialValue: _fieldType, + isExpanded: true, + decoration: const InputDecoration( + labelText: 'Type', + border: OutlineInputBorder(), + ), + items: [ + for (final type in CustomFieldType.values) + DropdownMenuItem(value: type, child: Text(type.label)), + ], + onChanged: (v) => setState(() => _fieldType = v ?? _fieldType), + ) + else + _Frozen( + label: 'Type', + value: widget.existing!.fieldType.label, + note: + 'Fixed after creation. Values already stored were written ' + 'as this type and a new one would reinterpret them.', + ), + if (_fieldType == CustomFieldType.dropdown) ...[ + const SizedBox(height: 16), + _OptionsEditor( + options: _options, + onAdd: _addOption, + onRemove: _removeOption, + ), + ], + const SizedBox(height: 12), + TextField( + controller: _order, + keyboardType: TextInputType.number, + inputFormatters: [FilteringTextInputFormatter.digitsOnly], + decoration: const InputDecoration( + labelText: 'Position', + helperText: 'Lower numbers come first on the record', + border: OutlineInputBorder(), + ), + ), + const SizedBox(height: 4), + // Material, not a bare Container: a SwitchListTile inside a + // ColoredBox asserts in debug because its ink splash would be + // painted over and never seen. + Material( + color: Colors.transparent, + child: Column( + children: [ + SwitchListTile( + contentPadding: EdgeInsets.zero, + value: _isRequired, + onChanged: (v) => setState(() => _isRequired = v), + title: const Text('Required'), + subtitle: const Text( + 'Binds new saves only. Records saved before this keep ' + 'their gap.', + ), + ), + SwitchListTile( + contentPadding: EdgeInsets.zero, + value: _isFilterable, + onChanged: (v) => setState(() => _isFilterable = v), + title: const Text('Offer as a filter'), + ), + ], + ), + ), + if (_error != null) ...[ + const SizedBox(height: 8), + Text( + _error!, + style: AppTypography.caption.copyWith( + color: AppColors.danger600, + ), + ), + ], + const SizedBox(height: 16), + Row( + children: [ + Expanded( + child: SizedBox( + height: 48, + child: OutlinedButton( + onPressed: () => Navigator.of(context).pop(), + child: const Text('Cancel'), + ), + ), + ), + const SizedBox(width: 12), + Expanded( + child: SizedBox( + height: 48, + child: FilledButton( + onPressed: _submit, + child: Text(_isCreate ? 'Add field' : 'Save field'), + ), + ), + ), + ], + ), + ], + ), + ), + ); + } +} + +/// One dropdown option being edited. +/// +/// [value] is what records store. It is empty for a row added in this sheet +/// and is left exactly as it was for a row that came back from the server, +/// because relabelling an option must not move the records that hold it. +class _OptionDraft { + _OptionDraft({required this.value, required String label}) + : controller = TextEditingController(text: label); + + final String value; + final TextEditingController controller; +} + +class _OptionsEditor extends StatelessWidget { + const _OptionsEditor({ + required this.options, + required this.onAdd, + required this.onRemove, + }); + + final List<_OptionDraft> options; + final VoidCallback onAdd; + final void Function(int index) onRemove; + + @override + Widget build(BuildContext context) { + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + 'Options', + style: AppTypography.body.copyWith(fontWeight: FontWeight.w600), + ), + const SizedBox(height: 4), + Text( + 'Renaming an option keeps the records that already use it.', + style: AppTypography.caption.copyWith(color: AppColors.textSecondary), + ), + const SizedBox(height: 8), + for (var i = 0; i < options.length; i++) + Padding( + padding: const EdgeInsets.only(bottom: 8), + child: Row( + children: [ + Expanded( + child: TextField( + controller: options[i].controller, + textCapitalization: TextCapitalization.sentences, + decoration: InputDecoration( + isDense: true, + hintText: 'Option ${i + 1}', + border: const OutlineInputBorder(), + ), + ), + ), + IconButton( + onPressed: () => onRemove(i), + tooltip: 'Remove option', + icon: Icon( + LucideIcons.x, + size: 18, + color: AppColors.textSecondary, + ), + ), + ], + ), + ), + SizedBox( + height: 44, + child: OutlinedButton.icon( + onPressed: onAdd, + icon: const Icon(LucideIcons.plus, size: 16), + label: const Text('Add option'), + ), + ), + ], + ); + } +} + +/// A value the server will not let change after creation, shown as text with +/// the reason. Offering the input and letting the save fail would teach the +/// rule the expensive way. +class _Frozen extends StatelessWidget { + const _Frozen({required this.label, required this.value, required this.note}); + + final String label; + final String value; + final String note; + + @override + Widget build(BuildContext context) { + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + label, + style: AppTypography.caption.copyWith(color: AppColors.textSecondary), + ), + const SizedBox(height: 2), + Text(value, style: AppTypography.body), + const SizedBox(height: 2), + Text( + note, + style: AppTypography.caption.copyWith(color: AppColors.textTertiary), + ), + ], + ); + } +} diff --git a/mobile/lib/screens/settings/custom_fields_screen.dart b/mobile/lib/screens/settings/custom_fields_screen.dart new file mode 100644 index 00000000..46011786 --- /dev/null +++ b/mobile/lib/screens/settings/custom_fields_screen.dart @@ -0,0 +1,486 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../core/theme/theme.dart'; +import '../../data/models/custom_field_definition.dart'; +import '../../providers/auth_provider.dart'; +import '../../providers/settings_provider.dart'; +import '../../widgets/common/badge.dart'; +import 'custom_field_form_sheet.dart'; + +/// Fields this organization added to records that shipped without them. +/// +/// Grouped by the record they extend, because that is how anyone looks for +/// one: "what do we collect on a ticket", not "show me all forty definitions". +/// +/// The number that earns its place is `records_missing_value`. Marking a field +/// required binds writes from that moment and backfills nothing, so "Required" +/// on a field twenty-three records have no value for is a promise the stored +/// data does not keep, and any report on that field will quietly exclude or +/// mis-bucket those twenty-three. +/// +/// Reading is open to every member; create, edit and turn off are admin-only +/// and the server answers 403 to anyone else. The controls follow that rule so +/// a member is not offered an action that cannot work. +class CustomFieldsScreen extends ConsumerWidget { + const CustomFieldsScreen({super.key}); + + Future _edit( + BuildContext context, + WidgetRef ref, + CustomFieldDefinition? existing, + ) async { + final draft = await showCustomFieldFormSheet(context, existing); + if (draft == null || !context.mounted) return; + final notifier = ref.read(customFieldsProvider.notifier); + final error = existing == null + ? await notifier.createField(draft) + : await notifier.updateField(existing.id, draft); + if (!context.mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + error ?? (existing == null ? 'Field added' : 'Field saved'), + ), + ), + ); + } + + Future _turnOff( + BuildContext context, + WidgetRef ref, + CustomFieldDefinition field, + ) async { + final confirmed = await showDialog( + context: context, + builder: (context) => AlertDialog( + title: Text('Turn off ${field.label}?'), + // Deliberately not "delete". The server soft-deletes: it flips + // is_active and every value already stored stays readable on the + // records that carry it. Saying Delete would promise a removal that + // does not happen. + content: const Text( + 'It stops appearing on new records and stops accepting new values. ' + 'Values already stored are kept, and you can turn it back on.', + ), + actions: [ + TextButton( + onPressed: () => Navigator.of(context).pop(false), + child: const Text('Leave it on'), + ), + TextButton( + onPressed: () => Navigator.of(context).pop(true), + style: TextButton.styleFrom(foregroundColor: AppColors.danger600), + child: const Text('Turn off'), + ), + ], + ), + ); + if (confirmed != true || !context.mounted) return; + final error = await ref + .read(customFieldsProvider.notifier) + .deactivateField(field.id); + if (!context.mounted) return; + ScaffoldMessenger.of( + context, + ).showSnackBar(SnackBar(content: Text(error ?? 'Field turned off'))); + } + + Future _turnOn( + BuildContext context, + WidgetRef ref, + CustomFieldDefinition field, + ) async { + final error = await ref + .read(customFieldsProvider.notifier) + .activateField(field.id); + if (!context.mounted) return; + ScaffoldMessenger.of( + context, + ).showSnackBar(SnackBar(content: Text(error ?? 'Field turned on'))); + } + + @override + Widget build(BuildContext context, WidgetRef ref) { + final async = ref.watch(customFieldsProvider); + final isAdmin = ref.watch(isOrgAdminProvider); + + return Scaffold( + backgroundColor: AppColors.surfaceDim, + appBar: AppBar( + title: const Text('Custom fields'), + backgroundColor: AppColors.surface, + elevation: 0, + scrolledUnderElevation: 1, + actions: [ + if (isAdmin) + IconButton( + icon: const Icon(LucideIcons.plus), + tooltip: 'New custom field', + onPressed: () => _edit(context, ref, null), + ), + ], + ), + body: async.when( + loading: () => const Center(child: CircularProgressIndicator()), + error: (_, _) => _ErrorState( + onRetry: () => ref.read(customFieldsProvider.notifier).refresh(), + ), + data: (state) { + if (state.fields.isEmpty) { + return _EmptyState(isAdmin: isAdmin); + } + final groups = state.grouped; + return RefreshIndicator( + onRefresh: () => ref.read(customFieldsProvider.notifier).refresh(), + child: ListView( + padding: const EdgeInsets.only(bottom: 96), + children: [ + _Summary(state: state), + for (final group in groups) ...[ + _GroupHeader( + label: customFieldTargetLabel(group.key), + count: group.value.length, + ), + for (final field in group.value) + _FieldRow( + field: field, + canEdit: isAdmin, + onEdit: () => _edit(context, ref, field), + onTurnOff: () => _turnOff(context, ref, field), + onTurnOn: () => _turnOn(context, ref, field), + ), + ], + ], + ), + ); + }, + ), + ); + } +} + +/// The counts, and the one warning worth interrupting for. +class _Summary extends StatelessWidget { + const _Summary({required this.state}); + + final CustomFieldsState state; + + @override + Widget build(BuildContext context) { + return Container( + color: AppColors.surface, + padding: const EdgeInsets.fromLTRB(16, 14, 16, 14), + margin: const EdgeInsets.only(bottom: 1), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + // Wrap, not Row: at 390px three stat pairs plus their labels do not + // fit on one line, and a Row would overflow rather than wrap. + Wrap( + spacing: 20, + runSpacing: 10, + children: [ + _Stat(value: '${state.active}', label: 'in use'), + _Stat( + value: '${state.count - state.active}', + label: 'turned off', + ), + _Stat( + value: '${state.modelsExtended}', + label: state.modelsExtended == 1 + ? 'record type' + : 'record types', + ), + ], + ), + if (state.requiredWithGaps > 0) ...[ + const SizedBox(height: 14), + Container( + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: AppColors.warning50, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: AppColors.warning200), + ), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Icon( + LucideIcons.triangleAlert, + size: 18, + color: AppColors.warning600, + ), + const SizedBox(width: 10), + Expanded( + child: Text( + state.requiredWithGaps == 1 + ? 'One required field has records with no value for ' + 'it. Marking a field required only binds new ' + 'saves, so older records keep the gap.' + : '${state.requiredWithGaps} required fields have ' + 'records with no value for them. Marking a ' + 'field required only binds new saves, so older ' + 'records keep the gap.', + style: AppTypography.caption.copyWith( + color: AppColors.textPrimary, + ), + ), + ), + ], + ), + ), + ], + ], + ), + ); + } +} + +class _Stat extends StatelessWidget { + const _Stat({required this.value, required this.label}); + + final String value; + final String label; + + @override + Widget build(BuildContext context) { + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + Text( + value, + style: AppTypography.h2.copyWith(fontWeight: FontWeight.w600), + ), + Text( + label, + style: AppTypography.caption.copyWith(color: AppColors.textSecondary), + ), + ], + ); + } +} + +class _GroupHeader extends StatelessWidget { + const _GroupHeader({required this.label, required this.count}); + + final String label; + final int count; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.fromLTRB(16, 22, 16, 8), + child: Row( + children: [ + Expanded( + child: Text( + label.toUpperCase(), + style: AppTypography.overline.copyWith( + color: AppColors.textSecondary, + letterSpacing: 1.2, + ), + ), + ), + Text( + '$count', + style: AppTypography.caption.copyWith( + color: AppColors.textTertiary, + ), + ), + ], + ), + ); + } +} + +class _FieldRow extends StatelessWidget { + const _FieldRow({ + required this.field, + required this.canEdit, + required this.onEdit, + required this.onTurnOff, + required this.onTurnOn, + }); + + final CustomFieldDefinition field; + final bool canEdit; + final VoidCallback onEdit; + final VoidCallback onTurnOff; + final VoidCallback onTurnOn; + + @override + Widget build(BuildContext context) { + final missing = field.recordsMissingValue; + return Container( + color: AppColors.surface, + margin: const EdgeInsets.only(bottom: 1), + padding: const EdgeInsets.fromLTRB(16, 12, 16, 12), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + field.label, + style: AppTypography.body.copyWith( + fontWeight: FontWeight.w600, + color: field.isActive + ? AppColors.textPrimary + : AppColors.textSecondary, + ), + ), + const SizedBox(height: 2), + Text( + field.key, + style: AppTypography.caption.copyWith( + color: AppColors.textTertiary, + fontFamily: 'monospace', + ), + ), + const SizedBox(height: 8), + Wrap( + spacing: 8, + runSpacing: 6, + crossAxisAlignment: WrapCrossAlignment.center, + children: [ + StatusBadge( + label: field.fieldType.label, + color: AppColors.gray500, + ), + if (!field.isActive) + StatusBadge(label: 'Turned off', color: AppColors.gray500), + if (field.isRequired) + StatusBadge(label: 'Required', color: AppColors.primary600), + if (field.isFilterable) + StatusBadge(label: 'Filter', color: AppColors.gray500), + if (field.fieldType == CustomFieldType.dropdown) + Text( + '${field.options.length} option' + '${field.options.length == 1 ? '' : 's'}', + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + ], + ), + // Only worth saying when it is a problem. A count on an optional + // field is just "records saved before this existed", which is not + // news; on a required one it is the gap the banner warns about. + if (field.isRequired && missing > 0) ...[ + const SizedBox(height: 8), + Text( + '$missing record${missing == 1 ? '' : 's'} ' + '${missing == 1 ? 'has' : 'have'} no value for this', + style: AppTypography.caption.copyWith( + color: AppColors.warning600, + ), + ), + ], + if (canEdit) ...[ + const SizedBox(height: 10), + Wrap( + spacing: 8, + runSpacing: 8, + children: [ + SizedBox( + height: 40, + child: OutlinedButton( + onPressed: onEdit, + child: const Text('Edit'), + ), + ), + SizedBox( + height: 40, + child: field.isActive + ? OutlinedButton( + onPressed: onTurnOff, + style: OutlinedButton.styleFrom( + foregroundColor: AppColors.danger600, + ), + child: const Text('Turn off'), + ) + : OutlinedButton( + onPressed: onTurnOn, + child: const Text('Turn on'), + ), + ), + ], + ), + ], + ], + ), + ); + } +} + +class _EmptyState extends StatelessWidget { + const _EmptyState({required this.isAdmin}); + + final bool isAdmin; + + @override + Widget build(BuildContext context) { + return Center( + child: Padding( + padding: const EdgeInsets.all(32), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Icon(LucideIcons.listPlus, size: 40, color: AppColors.textTertiary), + const SizedBox(height: 16), + Text( + 'No custom fields yet', + style: AppTypography.h3, + textAlign: TextAlign.center, + ), + const SizedBox(height: 8), + Text( + isAdmin + ? 'Add one to collect something the built-in fields do not ' + 'cover, on any record type.' + : 'An administrator adds these.', + style: AppTypography.body.copyWith( + color: AppColors.textSecondary, + ), + textAlign: TextAlign.center, + ), + ], + ), + ), + ); + } +} + +class _ErrorState extends StatelessWidget { + const _ErrorState({required this.onRetry}); + + final VoidCallback onRetry; + + @override + Widget build(BuildContext context) { + return Center( + child: Padding( + padding: const EdgeInsets.all(32), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + LucideIcons.triangleAlert, + size: 40, + color: AppColors.textTertiary, + ), + const SizedBox(height: 16), + Text( + 'Could not load the custom fields', + style: AppTypography.body, + textAlign: TextAlign.center, + ), + const SizedBox(height: 16), + OutlinedButton(onPressed: onRetry, child: const Text('Try again')), + ], + ), + ), + ); + } +} diff --git a/mobile/lib/screens/settings/macro_form_sheet.dart b/mobile/lib/screens/settings/macro_form_sheet.dart new file mode 100644 index 00000000..90e33116 --- /dev/null +++ b/mobile/lib/screens/settings/macro_form_sheet.dart @@ -0,0 +1,247 @@ +import 'package:flutter/material.dart'; + +import '../../core/theme/theme.dart'; +import '../../data/models/macro.dart'; +import '../../providers/settings_provider.dart'; + +/// Write or edit a saved reply. +/// +/// Returns the request body, or `null` if dismissed. [canCreateOrg] only +/// decides whether the "Everyone" choice is offered. +/// `_resolve_scope_and_owner` re-derives admin status from `request.profile` +/// and is what actually turns a non-admin's org-scope attempt into a 403. +Future?> showMacroFormSheet( + BuildContext context, { + Macro? existing, + required bool canCreateOrg, + required List placeholders, +}) { + return showModalBottomSheet>( + context: context, + isScrollControlled: true, + builder: (context) => _MacroFormSheet( + existing: existing, + canCreateOrg: canCreateOrg, + placeholders: placeholders, + ), + ); +} + +class _MacroFormSheet extends StatefulWidget { + const _MacroFormSheet({ + this.existing, + required this.canCreateOrg, + required this.placeholders, + }); + + final Macro? existing; + final bool canCreateOrg; + final List placeholders; + + @override + State<_MacroFormSheet> createState() => _MacroFormSheetState(); +} + +class _MacroFormSheetState extends State<_MacroFormSheet> { + late final TextEditingController _title; + late final TextEditingController _body; + late String _scope; + String? _error; + + bool get _isCreate => widget.existing == null; + + @override + void initState() { + super.initState(); + final m = widget.existing; + _title = TextEditingController(text: m?.title ?? ''); + _body = TextEditingController(text: m?.body ?? ''); + // A member has one option, so default to it rather than to a scope the + // server would refuse. + _scope = + m?.scope ?? + (widget.canCreateOrg ? Macro.scopeOrg : Macro.scopePersonal); + } + + @override + void dispose() { + _title.dispose(); + _body.dispose(); + super.dispose(); + } + + /// Drop a token into the body at the cursor. Typing `%customer_name%` on a + /// phone keyboard is four mode switches, and a typo is not caught at save + /// time: an unknown token renders literally into a reply to a customer. + void _insert(String token) { + final selection = _body.selection; + final text = _body.text; + final at = selection.isValid ? selection.start : text.length; + final end = selection.isValid ? selection.end : text.length; + final next = text.replaceRange(at, end, token); + _body.value = TextEditingValue( + text: next, + selection: TextSelection.collapsed(offset: at + token.length), + ); + } + + void _submit() { + final problem = validateMacroDraft( + title: _title.text, + body: _body.text, + scope: _scope, + ); + if (problem != null) { + setState(() => _error = problem); + return; + } + Navigator.of( + context, + ).pop(macroPayload(title: _title.text, body: _body.text, scope: _scope)); + } + + @override + Widget build(BuildContext context) { + return Padding( + padding: EdgeInsets.only( + left: 16, + right: 16, + top: 16, + bottom: MediaQuery.of(context).viewInsets.bottom + 16, + ), + child: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + _isCreate ? 'New saved reply' : 'Edit ${widget.existing!.title}', + style: AppTypography.h3.copyWith(fontWeight: FontWeight.w600), + ), + const SizedBox(height: 16), + TextField( + controller: _title, + textCapitalization: TextCapitalization.sentences, + maxLength: 255, + decoration: const InputDecoration( + labelText: 'Title', + helperText: 'What you will look for in the reply box', + border: OutlineInputBorder(), + counterText: '', + ), + ), + const SizedBox(height: 12), + if (widget.canCreateOrg) + DropdownButtonFormField( + initialValue: _scope, + isExpanded: true, + decoration: const InputDecoration( + labelText: 'Who can use it', + border: OutlineInputBorder(), + ), + items: const [ + DropdownMenuItem( + value: Macro.scopeOrg, + child: Text('Everyone in the organization'), + ), + DropdownMenuItem( + value: Macro.scopePersonal, + child: Text('Just you'), + ), + ], + onChanged: (v) => setState(() => _scope = v ?? _scope), + ) + else + // Not a disabled control: a member has exactly one option, and + // showing a greyed "Everyone" would advertise an action the + // server answers 403 to. + Container( + width: double.infinity, + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: AppColors.gray100, + borderRadius: BorderRadius.circular(8), + ), + child: Text( + 'Saved for you alone. An administrator writes the replies ' + 'the whole organization shares.', + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + ), + const SizedBox(height: 12), + TextField( + controller: _body, + textCapitalization: TextCapitalization.sentences, + minLines: 5, + maxLines: 12, + decoration: const InputDecoration( + labelText: 'Reply', + alignLabelWithHint: true, + border: OutlineInputBorder(), + ), + ), + if (widget.placeholders.isNotEmpty) ...[ + const SizedBox(height: 12), + Text( + 'Tap to insert', + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + const SizedBox(height: 6), + Wrap( + spacing: 8, + runSpacing: 8, + children: [ + for (final placeholder in widget.placeholders) + Tooltip( + message: placeholder.resolves, + child: ActionChip( + label: Text(placeholder.token), + onPressed: () => _insert(placeholder.token), + ), + ), + ], + ), + ], + if (_error != null) ...[ + const SizedBox(height: 8), + Text( + _error!, + style: AppTypography.caption.copyWith( + color: AppColors.danger600, + ), + ), + ], + const SizedBox(height: 16), + Row( + children: [ + Expanded( + child: SizedBox( + height: 48, + child: OutlinedButton( + onPressed: () => Navigator.of(context).pop(), + child: const Text('Cancel'), + ), + ), + ), + const SizedBox(width: 12), + Expanded( + child: SizedBox( + height: 48, + child: FilledButton( + onPressed: _submit, + child: Text(_isCreate ? 'Save reply' : 'Save changes'), + ), + ), + ), + ], + ), + ], + ), + ), + ); + } +} diff --git a/mobile/lib/screens/settings/macros_screen.dart b/mobile/lib/screens/settings/macros_screen.dart new file mode 100644 index 00000000..20a2541b --- /dev/null +++ b/mobile/lib/screens/settings/macros_screen.dart @@ -0,0 +1,436 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../core/theme/theme.dart'; +import '../../data/models/macro.dart'; +import '../../providers/auth_provider.dart'; +import '../../providers/settings_provider.dart'; +import '../../widgets/common/badge.dart'; +import 'macro_form_sheet.dart'; + +/// Saved replies, the canned answers offered in a ticket's reply box. +/// +/// **The one settings page every member can write to.** Anyone may keep their +/// own personal replies; only an admin may write one the whole organization +/// shares. Both rules are enforced by `_resolve_scope_and_owner` and +/// `MacroDetailView._get_writable`, and this screen mirrors them so nobody is +/// offered an action that answers 403. +/// +/// Personal rows belonging to other people never arrive: `_visible_qs` filters +/// them out. The edit control still asks about ownership rather than assuming +/// it, because that is the rule the server checks. +class MacrosScreen extends ConsumerWidget { + const MacrosScreen({super.key}); + + Future _edit( + BuildContext context, + WidgetRef ref, + Macro? existing, + ) async { + final state = ref.read(macrosProvider).value; + final payload = await showMacroFormSheet( + context, + existing: existing, + canCreateOrg: ref.read(isOrgAdminProvider), + placeholders: state?.placeholders ?? const [], + ); + if (payload == null || !context.mounted) return; + final notifier = ref.read(macrosProvider.notifier); + final error = existing == null + ? await notifier.createMacro(payload) + : await notifier.updateMacro(existing.id, payload); + if (!context.mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + error ?? (existing == null ? 'Reply saved' : 'Changes saved'), + ), + ), + ); + } + + Future _remove(BuildContext context, WidgetRef ref, Macro macro) async { + // The dialog's words come from the row's scope, because the outcome does: + // an org macro is turned off and a personal one is gone for good. + final removal = macroRemoval(macro); + final confirmed = await showDialog( + context: context, + builder: (context) => AlertDialog( + title: Text(removal.title), + content: Text(removal.detail), + actions: [ + TextButton( + onPressed: () => Navigator.of(context).pop(false), + child: const Text('Keep it'), + ), + TextButton( + onPressed: () => Navigator.of(context).pop(true), + style: TextButton.styleFrom(foregroundColor: AppColors.danger600), + child: Text(removal.actionLabel), + ), + ], + ), + ); + if (confirmed != true || !context.mounted) return; + final error = await ref.read(macrosProvider.notifier).removeMacro(macro.id); + if (!context.mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + error ?? (removal.isPermanent ? 'Reply deleted' : 'Reply turned off'), + ), + ), + ); + } + + Future _turnOn(BuildContext context, WidgetRef ref, Macro macro) async { + final error = await ref + .read(macrosProvider.notifier) + .activateMacro(macro.id); + if (!context.mounted) return; + ScaffoldMessenger.of( + context, + ).showSnackBar(SnackBar(content: Text(error ?? 'Reply turned on'))); + } + + @override + Widget build(BuildContext context, WidgetRef ref) { + final async = ref.watch(macrosProvider); + final isAdmin = ref.watch(isOrgAdminProvider); + final myEmail = ref.watch(myEmailProvider); + + return Scaffold( + backgroundColor: AppColors.surfaceDim, + appBar: AppBar( + title: const Text('Saved replies'), + backgroundColor: AppColors.surface, + elevation: 0, + scrolledUnderElevation: 1, + actions: [ + // No admin gate: every member may keep their own. + IconButton( + icon: const Icon(LucideIcons.plus), + tooltip: 'New saved reply', + onPressed: () => _edit(context, ref, null), + ), + ], + ), + body: async.when( + loading: () => const Center(child: CircularProgressIndicator()), + error: (_, _) => _ErrorState( + onRetry: () => ref.read(macrosProvider.notifier).refresh(), + ), + data: (state) { + if (state.macros.isEmpty) { + return const _EmptyState(); + } + return RefreshIndicator( + onRefresh: () => ref.read(macrosProvider.notifier).refresh(), + child: ListView( + padding: const EdgeInsets.only(bottom: 96), + children: [ + _Summary(state: state), + for (final macro in state.macros) + _MacroRow( + macro: macro, + canWrite: canWriteMacro( + isAdmin: isAdmin, + scope: macro.scope, + ownerEmail: macro.ownerEmail, + myEmail: myEmail, + ), + onEdit: () => _edit(context, ref, macro), + onRemove: () => _remove(context, ref, macro), + onTurnOn: () => _turnOn(context, ref, macro), + ), + ], + ), + ); + }, + ), + ); + } +} + +class _Summary extends StatelessWidget { + const _Summary({required this.state}); + + final MacrosState state; + + @override + Widget build(BuildContext context) { + return Container( + color: AppColors.surface, + padding: const EdgeInsets.fromLTRB(16, 14, 16, 14), + margin: const EdgeInsets.only(bottom: 1), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Wrap( + spacing: 20, + runSpacing: 10, + children: [ + _Stat(value: '${state.orgCount}', label: 'shared'), + _Stat(value: '${state.personalCount}', label: 'just yours'), + if (state.inactiveCount > 0) + _Stat(value: '${state.inactiveCount}', label: 'turned off'), + ], + ), + if (state.brokenCount > 0) ...[ + const SizedBox(height: 14), + Container( + padding: const EdgeInsets.all(12), + decoration: BoxDecoration( + color: AppColors.warning50, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: AppColors.warning200), + ), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Icon( + LucideIcons.triangleAlert, + size: 18, + color: AppColors.warning600, + ), + const SizedBox(width: 10), + Expanded( + child: Text( + state.brokenCount == 1 + ? 'One reply uses a placeholder that is not ' + 'recognized. It goes out to the customer ' + 'exactly as written.' + : '${state.brokenCount} replies use a placeholder ' + 'that is not recognized. Those go out to the ' + 'customer exactly as written.', + style: AppTypography.caption.copyWith( + color: AppColors.textPrimary, + ), + ), + ), + ], + ), + ), + ], + ], + ), + ); + } +} + +class _Stat extends StatelessWidget { + const _Stat({required this.value, required this.label}); + + final String value; + final String label; + + @override + Widget build(BuildContext context) { + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + Text( + value, + style: AppTypography.h2.copyWith(fontWeight: FontWeight.w600), + ), + Text( + label, + style: AppTypography.caption.copyWith(color: AppColors.textSecondary), + ), + ], + ); + } +} + +class _MacroRow extends StatelessWidget { + const _MacroRow({ + required this.macro, + required this.canWrite, + required this.onEdit, + required this.onRemove, + required this.onTurnOn, + }); + + final Macro macro; + final bool canWrite; + final VoidCallback onEdit; + final VoidCallback onRemove; + final VoidCallback onTurnOn; + + @override + Widget build(BuildContext context) { + final broken = macro.unknownPlaceholders; + return Container( + color: AppColors.surface, + margin: const EdgeInsets.only(bottom: 1), + padding: const EdgeInsets.fromLTRB(16, 12, 16, 12), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + macro.title, + style: AppTypography.body.copyWith( + fontWeight: FontWeight.w600, + color: macro.isActive + ? AppColors.textPrimary + : AppColors.textSecondary, + ), + ), + const SizedBox(height: 4), + Text( + macro.body, + maxLines: 2, + overflow: TextOverflow.ellipsis, + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + const SizedBox(height: 8), + Wrap( + spacing: 8, + runSpacing: 6, + crossAxisAlignment: WrapCrossAlignment.center, + children: [ + StatusBadge( + label: macro.scopeLabel, + color: macro.isPersonal + ? AppColors.gray500 + : AppColors.primary600, + ), + if (!macro.isActive) + StatusBadge(label: 'Turned off', color: AppColors.gray500), + Text( + macro.usageCount == 0 + ? 'not used yet' + : 'used ${macro.usageCount} time' + '${macro.usageCount == 1 ? '' : 's'}', + style: AppTypography.caption.copyWith( + color: AppColors.textTertiary, + ), + ), + ], + ), + if (broken.isNotEmpty) ...[ + const SizedBox(height: 8), + Text( + // Named, not counted: the author needs to know which token to + // fix, and a typo like %custmer_name% is only obvious beside the + // right spelling. + broken.length == 1 + ? '${broken.single} is not recognized and goes out as written' + : '${broken.join(', ')} are not recognized and go out as ' + 'written', + style: AppTypography.caption.copyWith( + color: AppColors.warning600, + ), + ), + ], + if (canWrite) ...[ + const SizedBox(height: 10), + Wrap( + spacing: 8, + runSpacing: 8, + children: [ + SizedBox( + height: 40, + child: OutlinedButton( + onPressed: onEdit, + child: const Text('Edit'), + ), + ), + SizedBox( + height: 40, + child: macro.isActive + ? OutlinedButton( + onPressed: onRemove, + style: OutlinedButton.styleFrom( + foregroundColor: AppColors.danger600, + ), + child: Text(macroRemoval(macro).actionLabel), + ) + : OutlinedButton( + onPressed: onTurnOn, + child: const Text('Turn on'), + ), + ), + ], + ), + ], + ], + ), + ); + } +} + +class _EmptyState extends StatelessWidget { + const _EmptyState(); + + @override + Widget build(BuildContext context) { + return Center( + child: Padding( + padding: const EdgeInsets.all(32), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + LucideIcons.messageSquareQuote, + size: 40, + color: AppColors.textTertiary, + ), + const SizedBox(height: 16), + Text( + 'No saved replies yet', + style: AppTypography.h3, + textAlign: TextAlign.center, + ), + const SizedBox(height: 8), + Text( + 'Write the answer you keep typing once, then insert it into a ' + 'ticket reply in two taps.', + style: AppTypography.body.copyWith( + color: AppColors.textSecondary, + ), + textAlign: TextAlign.center, + ), + ], + ), + ), + ); + } +} + +class _ErrorState extends StatelessWidget { + const _ErrorState({required this.onRetry}); + + final VoidCallback onRetry; + + @override + Widget build(BuildContext context) { + return Center( + child: Padding( + padding: const EdgeInsets.all(32), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + LucideIcons.triangleAlert, + size: 40, + color: AppColors.textTertiary, + ), + const SizedBox(height: 16), + Text( + 'Could not load the saved replies', + style: AppTypography.body, + textAlign: TextAlign.center, + ), + const SizedBox(height: 16), + OutlinedButton(onPressed: onRetry, child: const Text('Try again')), + ], + ), + ), + ); + } +} diff --git a/mobile/lib/screens/settings/more_screen.dart b/mobile/lib/screens/settings/more_screen.dart index c5669e8a..f1ce13be 100644 --- a/mobile/lib/screens/settings/more_screen.dart +++ b/mobile/lib/screens/settings/more_screen.dart @@ -38,37 +38,37 @@ class _MoreScreenState extends ConsumerState { // Workspace Section. Destinations that aren't on the bottom nav. _buildSectionHeader('Workspace'), - _MenuItem( + MenuRow( icon: LucideIcons.checkSquare, label: 'Tasks', description: 'Your to-dos across leads, deals and tickets', onTap: () => context.go(AppRoutes.tasks), ), - _MenuItem( + MenuRow( icon: LucideIcons.building2, label: 'Accounts', description: 'The companies you sell to', onTap: () => context.push(AppRoutes.accounts), ), - _MenuItem( + MenuRow( icon: LucideIcons.users, label: 'Contacts', description: 'The people at those companies', onTap: () => context.push(AppRoutes.contacts), ), - _MenuItem( + MenuRow( icon: LucideIcons.clock, label: 'Timesheet', description: 'The hours you logged this week', onTap: () => context.push(AppRoutes.timesheet), ), - _MenuItem( + MenuRow( icon: LucideIcons.bell, label: 'Notifications', description: 'Mentions and comments on your tickets', onTap: () => context.push(AppRoutes.notifications), ), - _MenuItem( + MenuRow( icon: LucideIcons.receipt, label: 'Invoices', description: 'What is owed, and what has been paid', @@ -77,7 +77,7 @@ class _MoreScreenState extends ConsumerState { // Account Section _buildSectionHeader('Account'), - _MenuItem( + MenuRow( icon: LucideIcons.user, label: 'Profile Settings', onTap: () => context.push(AppRoutes.profile), @@ -87,7 +87,7 @@ class _MoreScreenState extends ConsumerState { // worked; nothing in the app reached them once an org was chosen, // so the only way to change org was to sign out. if ((ref.watch(authProvider).organizations?.length ?? 0) > 1) - _MenuItem( + MenuRow( icon: LucideIcons.building2, label: 'Switch Organization', description: ref.watch(selectedOrgProvider)?.name, @@ -96,12 +96,23 @@ class _MoreScreenState extends ConsumerState { // Team Section _buildSectionHeader('Team'), - _MenuItem( + MenuRow( icon: LucideIcons.users, label: 'Team', description: 'Who is in this org, and what they can do', onTap: () => context.push(AppRoutes.team), ), + // Not admin-gated, matching the web sidebar and the API: the + // settings reads are open to any member and only the writes are + // admin-only. Team above is the opposite and is genuinely + // admin-only, which is why the two sit in separate rows rather + // than one "Administration" block. + MenuRow( + icon: LucideIcons.slidersHorizontal, + label: 'Organization settings', + description: 'Custom fields and other org-wide rules', + onTap: () => context.push(AppRoutes.settings), + ), // Preferences used to be a section here, holding a Dark Mode // switch that repainted nothing and then announced "Dark mode @@ -112,18 +123,18 @@ class _MoreScreenState extends ConsumerState { // Support Section _buildSectionHeader('Support'), - _MenuItem( + MenuRow( icon: LucideIcons.helpCircle, label: 'Help Center', onTap: () => _openOnTheWeb('/docs', 'Help Center'), ), - _MenuItem( + MenuRow( icon: LucideIcons.fileText, label: 'Terms of Service', onTap: () => _openOnTheWeb('/terms-of-service', 'Terms of Service'), ), - _MenuItem( + MenuRow( icon: LucideIcons.shield, label: 'Privacy Policy', onTap: () => _openOnTheWeb('/privacy-policy', 'Privacy Policy'), @@ -327,71 +338,3 @@ class _MoreScreenState extends ConsumerState { ); } } - -/// Menu Item Widget -class _MenuItem extends StatelessWidget { - final IconData icon; - final String label; - final String? description; - final VoidCallback onTap; - - const _MenuItem({ - required this.icon, - required this.label, - this.description, - required this.onTap, - }); - - @override - Widget build(BuildContext context) { - return InkWell( - onTap: onTap, - child: Container( - padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 14), - decoration: BoxDecoration( - color: AppColors.surface, - border: Border(bottom: BorderSide(color: AppColors.gray100)), - ), - child: Row( - children: [ - // Icon - Icon(icon, size: 22, color: AppColors.textSecondary), - - const SizedBox(width: 14), - - // Label + Description - Expanded( - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text( - label, - style: AppTypography.body.copyWith( - fontWeight: FontWeight.w500, - ), - ), - if (description != null) ...[ - const SizedBox(height: 2), - Text( - description!, - style: AppTypography.caption.copyWith( - color: AppColors.textSecondary, - ), - ), - ], - ], - ), - ), - - // Chevron - Icon( - LucideIcons.chevronRight, - size: 20, - color: AppColors.textTertiary, - ), - ], - ), - ), - ); - } -} diff --git a/mobile/lib/screens/settings/settings_hub_screen.dart b/mobile/lib/screens/settings/settings_hub_screen.dart new file mode 100644 index 00000000..1d3bb8fa --- /dev/null +++ b/mobile/lib/screens/settings/settings_hub_screen.dart @@ -0,0 +1,92 @@ +import 'package:flutter/material.dart'; +import 'package:go_router/go_router.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../core/theme/theme.dart'; +import '../../routes/app_router.dart'; +import '../../widgets/common/common.dart'; + +/// The launcher for the org settings cluster, mirroring `/settings` on the web. +/// +/// **Not admin-gated, on purpose.** The reads underneath it are open to any +/// member: `CustomFieldDefinitionListCreateView.get` carries only +/// `IsAuthenticated, HasOrgContext`, and the admin check sits on POST, PUT and +/// DELETE. The web's sidebar says the same in as many words ("the hub is +/// readable by any member"). Gating the read here would hide from a member the +/// definitions that shape the forms they fill in every day, and would be this +/// app disagreeing with the API about who may look. +/// +/// Each page hides its own write controls from a non-admin, which is UX rather +/// than a guard: the server answers 403 either way. +/// +/// Pages arrive here as they are built for the phone. The web has thirteen; +/// listing the unbuilt ones as greyed rows would only advertise a gap the +/// parity tracker already records. +class SettingsHubScreen extends StatelessWidget { + const SettingsHubScreen({super.key}); + + @override + Widget build(BuildContext context) { + return Scaffold( + backgroundColor: AppColors.surfaceDim, + appBar: AppBar( + title: const Text('Organization settings'), + backgroundColor: AppColors.surface, + elevation: 0, + scrolledUnderElevation: 1, + ), + body: SingleChildScrollView( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const _SectionHeader('Records'), + MenuRow( + icon: LucideIcons.listPlus, + label: 'Custom fields', + description: 'Extra fields on leads, deals, tickets and the rest', + onTap: () => context.push(AppRoutes.settingsCustomFields), + ), + const _SectionHeader('Tickets'), + MenuRow( + icon: LucideIcons.messageSquareQuote, + label: 'Saved replies', + description: 'Canned answers to insert into a ticket reply', + onTap: () => context.push(AppRoutes.settingsMacros), + ), + const SizedBox(height: 24), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 16), + child: Text( + 'The rest of the settings are on the web app for now.', + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + ), + const SizedBox(height: 48), + ], + ), + ), + ); + } +} + +class _SectionHeader extends StatelessWidget { + const _SectionHeader(this.title); + + final String title; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.fromLTRB(16, 24, 16, 8), + child: Text( + title.toUpperCase(), + style: AppTypography.overline.copyWith( + color: AppColors.textSecondary, + letterSpacing: 1.2, + ), + ), + ); + } +} diff --git a/mobile/lib/screens/tickets/macro_picker_sheet.dart b/mobile/lib/screens/tickets/macro_picker_sheet.dart new file mode 100644 index 00000000..ed3d7032 --- /dev/null +++ b/mobile/lib/screens/tickets/macro_picker_sheet.dart @@ -0,0 +1,262 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../core/theme/theme.dart'; +import '../../data/models/macro.dart'; +import '../../providers/settings_provider.dart'; +import '../../widgets/common/badge.dart'; + +/// Pick a saved reply and expand it against this ticket. +/// +/// Returns the rendered text, or `null` if nothing was chosen. This is the +/// point of the whole feature on a phone: the alternative is typing a +/// paragraph of boilerplate on a phone keyboard. +/// +/// The expansion is a server round trip (`POST /macros//render/`), not a +/// local search and replace. `macros/render.py` owns the supported token set, +/// substituting here would drift from it, and the server is also where the +/// usage count is kept. +Future showMacroPickerSheet(BuildContext context, String ticketId) { + return showModalBottomSheet( + context: context, + isScrollControlled: true, + builder: (context) => _MacroPickerSheet(ticketId: ticketId), + ); +} + +class _MacroPickerSheet extends ConsumerStatefulWidget { + const _MacroPickerSheet({required this.ticketId}); + + final String ticketId; + + @override + ConsumerState<_MacroPickerSheet> createState() => _MacroPickerSheetState(); +} + +class _MacroPickerSheetState extends ConsumerState<_MacroPickerSheet> { + final _search = TextEditingController(); + String _query = ''; + String? _applyingId; + String? _error; + + @override + void dispose() { + _search.dispose(); + super.dispose(); + } + + Future _apply(Macro macro) async { + setState(() { + _applyingId = macro.id; + _error = null; + }); + final result = await renderMacro( + macroId: macro.id, + ticketId: widget.ticketId, + ); + if (!mounted) return; + if (result.error != null) { + setState(() { + _applyingId = null; + _error = result.error; + }); + return; + } + Navigator.of(context).pop(result.text); + } + + /// Filtered on the client, over a list the server already scoped to the + /// active macros this person may see. The list is small (org macros plus + /// your own), so a round trip per keystroke would buy nothing. + List _filter(List macros) { + final q = _query.trim().toLowerCase(); + if (q.isEmpty) return macros; + return macros + .where( + (m) => + m.title.toLowerCase().contains(q) || + m.body.toLowerCase().contains(q), + ) + .toList(growable: false); + } + + @override + Widget build(BuildContext context) { + final async = ref.watch(activeMacrosProvider); + + return Padding( + padding: EdgeInsets.only( + bottom: MediaQuery.of(context).viewInsets.bottom, + ), + child: SizedBox( + // Half the screen: enough to show several replies, little enough that + // the ticket stays visible behind it. + height: MediaQuery.of(context).size.height * 0.6, + child: Column( + children: [ + Padding( + padding: const EdgeInsets.fromLTRB(16, 16, 16, 8), + child: Row( + children: [ + Expanded( + child: Text( + 'Saved replies', + style: AppTypography.h3.copyWith( + fontWeight: FontWeight.w600, + ), + ), + ), + IconButton( + tooltip: 'Close', + icon: const Icon(LucideIcons.x, size: 20), + onPressed: () => Navigator.of(context).pop(), + ), + ], + ), + ), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 16), + child: TextField( + controller: _search, + onChanged: (v) => setState(() => _query = v), + textInputAction: TextInputAction.search, + decoration: const InputDecoration( + hintText: 'Search saved replies', + prefixIcon: Icon(LucideIcons.search, size: 18), + isDense: true, + border: OutlineInputBorder(), + ), + ), + ), + if (_error != null) + Padding( + padding: const EdgeInsets.fromLTRB(16, 8, 16, 0), + child: Text( + _error!, + style: AppTypography.caption.copyWith( + color: AppColors.danger600, + ), + ), + ), + const SizedBox(height: 8), + Expanded( + child: async.when( + loading: () => const Center(child: CircularProgressIndicator()), + error: (_, _) => Center( + child: Padding( + padding: const EdgeInsets.all(24), + child: Text( + 'Could not load the saved replies', + style: AppTypography.body.copyWith( + color: AppColors.textSecondary, + ), + textAlign: TextAlign.center, + ), + ), + ), + data: (macros) { + final rows = _filter(macros); + if (rows.isEmpty) { + return Center( + child: Padding( + padding: const EdgeInsets.all(24), + child: Text( + macros.isEmpty + ? 'No saved replies yet. You can write one under ' + 'More, Organization settings.' + : 'No saved reply matches that search', + style: AppTypography.body.copyWith( + color: AppColors.textSecondary, + ), + textAlign: TextAlign.center, + ), + ), + ); + } + return ListView.builder( + itemCount: rows.length, + itemBuilder: (context, i) => _PickerRow( + macro: rows[i], + busy: _applyingId != null, + applying: _applyingId == rows[i].id, + onTap: () => _apply(rows[i]), + ), + ); + }, + ), + ), + ], + ), + ), + ); + } +} + +class _PickerRow extends StatelessWidget { + const _PickerRow({ + required this.macro, + required this.busy, + required this.applying, + required this.onTap, + }); + + final Macro macro; + final bool busy; + final bool applying; + final VoidCallback onTap; + + @override + Widget build(BuildContext context) { + return InkWell( + onTap: busy ? null : onTap, + child: Container( + decoration: BoxDecoration( + border: Border(bottom: BorderSide(color: AppColors.gray100)), + ), + padding: const EdgeInsets.fromLTRB(16, 12, 16, 12), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + macro.title, + style: AppTypography.body.copyWith( + fontWeight: FontWeight.w600, + ), + ), + const SizedBox(height: 2), + Text( + macro.body, + maxLines: 2, + overflow: TextOverflow.ellipsis, + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + const SizedBox(height: 6), + StatusBadge( + label: macro.scopeLabel, + color: macro.isPersonal + ? AppColors.gray500 + : AppColors.primary600, + ), + ], + ), + ), + const SizedBox(width: 12), + if (applying) + const SizedBox( + width: 18, + height: 18, + child: CircularProgressIndicator(strokeWidth: 2), + ), + ], + ), + ), + ); + } +} diff --git a/mobile/lib/screens/tickets/ticket_detail_screen.dart b/mobile/lib/screens/tickets/ticket_detail_screen.dart index 7c76cf8c..4fdb2700 100644 --- a/mobile/lib/screens/tickets/ticket_detail_screen.dart +++ b/mobile/lib/screens/tickets/ticket_detail_screen.dart @@ -7,6 +7,7 @@ import '../../core/theme/theme.dart'; import '../../data/models/attachment.dart'; import '../../data/models/ticket.dart'; import '../../data/models/comment.dart'; +import 'macro_picker_sheet.dart'; import '../../data/models/email_message.dart'; import '../../providers/auth_provider.dart'; import '../../providers/lookup_provider.dart'; @@ -583,6 +584,12 @@ class _TicketDetailScreenState extends ConsumerState color: AppColors.textSecondary, onPressed: _isAddingComment ? null : _pickMention, ), + IconButton( + tooltip: 'Saved reply', + icon: const Icon(LucideIcons.messageSquareQuote, size: 20), + color: AppColors.textSecondary, + onPressed: _isAddingComment ? null : _pickMacro, + ), Expanded( child: TextField( controller: _commentController, @@ -959,6 +966,27 @@ class _TicketDetailScreenState extends ConsumerState ); } + /// Insert a saved reply, expanded against this ticket. + /// + /// Inserted at the cursor rather than replacing the box, so a macro can be + /// dropped into a reply already part-written. Nothing is sent: the agent + /// still reads it and presses send, which is the point of expanding it into + /// the composer instead of posting it. + Future _pickMacro() async { + final rendered = await showMacroPickerSheet(context, widget.ticketId); + if (rendered == null || rendered.isEmpty || !mounted) return; + + final text = _commentController.text; + final sel = _commentController.selection; + final insertAt = sel.isValid ? sel.start : text.length; + final insertEnd = sel.isValid ? sel.end : text.length; + final next = text.replaceRange(insertAt, insertEnd, rendered); + _commentController.value = TextEditingValue( + text: next, + selection: TextSelection.collapsed(offset: insertAt + rendered.length), + ); + } + Future _toggleWatch() async { final current = _watchers?.isCurrentUserWatching ?? false; final ok = await ref diff --git a/mobile/lib/widgets/common/common.dart b/mobile/lib/widgets/common/common.dart index 9634fc1f..3680e911 100644 --- a/mobile/lib/widgets/common/common.dart +++ b/mobile/lib/widgets/common/common.dart @@ -10,3 +10,4 @@ export 'badge.dart'; export 'buttons.dart'; export 'input_field.dart'; export 'loading.dart'; +export 'menu_row.dart'; diff --git a/mobile/lib/widgets/common/menu_row.dart b/mobile/lib/widgets/common/menu_row.dart new file mode 100644 index 00000000..93aa0877 --- /dev/null +++ b/mobile/lib/widgets/common/menu_row.dart @@ -0,0 +1,74 @@ +import 'package:flutter/material.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../core/theme/theme.dart'; + +/// One tappable row in a menu list: icon, label, optional description, chevron. +/// +/// Lives here rather than privately in `more_screen.dart` because the settings +/// hub is a second list of exactly these, and two copies would drift on the +/// first change to the row height or the divider. +/// +/// The whole row is the tap target and it is 48pt tall at the smallest, so a +/// thumb finds it without aiming. +class MenuRow extends StatelessWidget { + const MenuRow({ + super.key, + required this.icon, + required this.label, + this.description, + required this.onTap, + }); + + final IconData icon; + final String label; + final String? description; + final VoidCallback onTap; + + @override + Widget build(BuildContext context) { + return InkWell( + onTap: onTap, + child: Container( + padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 14), + decoration: BoxDecoration( + color: AppColors.surface, + border: Border(bottom: BorderSide(color: AppColors.gray100)), + ), + child: Row( + children: [ + Icon(icon, size: 22, color: AppColors.textSecondary), + const SizedBox(width: 14), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + label, + style: AppTypography.body.copyWith( + fontWeight: FontWeight.w500, + ), + ), + if (description != null) ...[ + const SizedBox(height: 2), + Text( + description!, + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + ], + ], + ), + ), + Icon( + LucideIcons.chevronRight, + size: 20, + color: AppColors.textTertiary, + ), + ], + ), + ), + ); + } +} diff --git a/mobile/test/screens/phone_viewport_test.dart b/mobile/test/screens/phone_viewport_test.dart index a2ec1d86..515ebaa6 100644 --- a/mobile/test/screens/phone_viewport_test.dart +++ b/mobile/test/screens/phone_viewport_test.dart @@ -1,4 +1,6 @@ import 'package:bottle_crm/data/models/app_notification.dart'; +import 'package:bottle_crm/data/models/custom_field_definition.dart'; +import 'package:bottle_crm/data/models/macro.dart'; import 'package:bottle_crm/data/models/invoice.dart'; import 'package:bottle_crm/data/models/time_entry.dart'; import 'package:bottle_crm/data/models/timesheet.dart'; @@ -7,7 +9,9 @@ import 'package:bottle_crm/data/models/product.dart'; import 'package:bottle_crm/data/models/recurring_invoice.dart'; import 'package:bottle_crm/providers/invoice_extras_provider.dart'; import 'package:bottle_crm/providers/invoices_provider.dart'; +import 'package:bottle_crm/providers/auth_provider.dart'; import 'package:bottle_crm/providers/notifications_provider.dart'; +import 'package:bottle_crm/providers/settings_provider.dart'; import 'package:bottle_crm/providers/timesheet_provider.dart'; import 'package:bottle_crm/screens/invoices/estimates_list_screen.dart'; import 'package:bottle_crm/screens/invoices/invoices_list_screen.dart'; @@ -16,6 +20,10 @@ import 'package:bottle_crm/screens/invoices/new_recurring_screen.dart'; import 'package:bottle_crm/screens/invoices/products_list_screen.dart'; import 'package:bottle_crm/screens/invoices/recurring_list_screen.dart'; import 'package:bottle_crm/screens/notifications/notifications_screen.dart'; +import 'package:bottle_crm/screens/settings/custom_fields_screen.dart'; +import 'package:bottle_crm/screens/settings/macros_screen.dart'; +import 'package:bottle_crm/screens/tickets/macro_picker_sheet.dart'; +import 'package:bottle_crm/screens/settings/settings_hub_screen.dart'; import 'package:bottle_crm/screens/timesheet/timesheet_screen.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -67,6 +75,74 @@ void main() { child: routed(const TimesheetScreen()), ); + Widget settingsHubApp() => + const ProviderScope(child: MaterialApp(home: SettingsHubScreen())); + + /// [isAdmin] is overridden rather than left to fall out of a signed-in + /// profile, because both answers matter here: the read is open to a member + /// and every write is admin-only. + Widget customFieldsApp({required bool isAdmin}) => ProviderScope( + overrides: [ + customFieldsProvider.overrideWith(_FakeCustomFields.new), + isOrgAdminProvider.overrideWithValue(isAdmin), + ], + child: const MaterialApp(home: CustomFieldsScreen()), + ); + + /// [myEmail] decides ownership of the personal rows, which is what the edit + /// control turns on. `myEmailProvider` normally reads the signed-in user, so + /// it is overridden here to drive both answers. + Widget macrosApp({required bool isAdmin, String? myEmail}) => ProviderScope( + overrides: [ + macrosProvider.overrideWith(_FakeMacros.new), + isOrgAdminProvider.overrideWithValue(isAdmin), + myEmailProvider.overrideWithValue(myEmail), + ], + child: const MaterialApp(home: MacrosScreen()), + ); + + /// A button that opens the picker, so the sheet is rendered the way it is + /// really reached: over a screen, with the keyboard inset in play. + Widget macroPickerApp({List? macros}) => ProviderScope( + overrides: [ + activeMacrosProvider.overrideWith( + (ref) async => + macros ?? + [ + Macro.fromJson(const { + 'id': 'm1', + 'title': 'Password reset, standard wording agreed with legal', + 'body': + 'Hi there, follow the link below to reset your password ' + 'and let us know if it does not arrive within ten minutes.', + 'scope': 'org', + 'is_active': true, + }), + Macro.fromJson(const { + 'id': 'm2', + 'title': 'My follow-up', + 'body': 'Just checking in on this one.', + 'scope': 'personal', + 'owner_name': 'me@example.com', + 'is_active': true, + }), + ], + ), + ], + child: MaterialApp( + home: Scaffold( + body: Builder( + builder: (context) => Center( + child: ElevatedButton( + onPressed: () => showMacroPickerSheet(context, 't42'), + child: const Text('open'), + ), + ), + ), + ), + ), + ); + Widget notificationsApp() => ProviderScope( overrides: [notificationsProvider.overrideWith(_FakeNotifications.new)], child: routed(const NotificationsScreen()), @@ -473,6 +549,298 @@ void main() { expect(find.textContaining('monthly, before tax'), findsOneWidget); }); }); + + group('the settings cluster at 390px', () { + testWidgets('the hub renders without overflowing', (tester) async { + await pump(tester, settingsHubApp()); + expect(tester.takeException(), isNull); + }); + + testWidgets('the hub opens to a member, not just an admin', (tester) async { + // isOrgAdminProvider is false in this test's state. The reads under the + // hub are open to any member server-side, and the web sidebar keeps the + // entry for everyone for that reason, so a lock screen here would be + // this app disagreeing with the API about who may look. + await pump(tester, settingsHubApp()); + expect(find.text('Custom fields'), findsOneWidget); + }); + + testWidgets('custom fields render without overflowing', (tester) async { + await pump(tester, customFieldsApp(isAdmin: true)); + expect(tester.takeException(), isNull); + }); + + testWidgets('custom fields fit with the system font scaled up', ( + tester, + ) async { + // The row that earns this: a Wrap of up to four badges plus an option + // count, over a long label, with two buttons under it. + await pump(tester, customFieldsApp(isAdmin: true), textScale: 1.5); + expect(tester.takeException(), isNull); + }); + + testWidgets('a member sees the fields but no write controls', ( + tester, + ) async { + await pump(tester, customFieldsApp(isAdmin: false)); + + expect(find.text('Severity'), findsOneWidget, reason: 'the read is open'); + expect(find.widgetWithText(OutlinedButton, 'Edit'), findsNothing); + expect(find.widgetWithText(OutlinedButton, 'Turn off'), findsNothing); + expect(find.byTooltip('New custom field'), findsNothing); + }); + + testWidgets('a turned-off field offers Turn on, not Turn off', ( + tester, + ) async { + await pump(tester, customFieldsApp(isAdmin: true)); + + expect(find.widgetWithText(OutlinedButton, 'Turn on'), findsOneWidget); + expect(find.text('Turned off'), findsOneWidget); + }); + + testWidgets('a required field with gaps says so on the row', ( + tester, + ) async { + await pump(tester, customFieldsApp(isAdmin: true)); + + // Marking a field required binds new saves only, so the count is the + // thing that makes "Required" honest. + expect(find.textContaining('23 records have no value'), findsOneWidget); + // One gap, so the singular branch of the banner. The fake sets + // requiredWithGaps to 1 deliberately: the plural string is the one that + // reads naturally when written, and the singular is where "1 required + // fields have" slips through. + expect( + find.textContaining('One required field has records'), + findsOneWidget, + ); + }); + + testWidgets('the new-field sheet renders without overflowing', ( + tester, + ) async { + await pump(tester, customFieldsApp(isAdmin: true)); + await tester.tap(find.byTooltip('New custom field')); + await tester.pumpAndSettle(); + + expect(find.text('New custom field'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + + testWidgets('the sheet fills the key in from the label', (tester) async { + await pump(tester, customFieldsApp(isAdmin: true)); + await tester.tap(find.byTooltip('New custom field')); + await tester.pumpAndSettle(); + + await tester.enterText( + find.widgetWithText(TextField, 'Label'), + 'Deal source', + ); + await tester.pumpAndSettle(); + + final key = tester.widget( + find.widgetWithText(TextField, 'Key'), + ); + expect(key.controller?.text, 'deal_source'); + }); + + testWidgets('the sheet asks for options only on a dropdown', ( + tester, + ) async { + await pump(tester, customFieldsApp(isAdmin: true)); + await tester.tap(find.byTooltip('New custom field')); + await tester.pumpAndSettle(); + + expect(find.text('Add option'), findsNothing); + + await tester.tap(find.text('Text').last); + await tester.pumpAndSettle(); + await tester.tap(find.text('Dropdown').last); + await tester.pumpAndSettle(); + + expect(find.text('Add option'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + + testWidgets('editing a field shows the frozen three as text', ( + tester, + ) async { + await pump(tester, customFieldsApp(isAdmin: true)); + await tester.tap(find.widgetWithText(OutlinedButton, 'Edit').first); + await tester.pumpAndSettle(); + + // The server refuses a change to any of the three with a 400. Offering + // the input and letting the save fail teaches the rule the hard way. + expect(find.widgetWithText(TextField, 'Key'), findsNothing); + expect(find.textContaining('Fixed after creation'), findsWidgets); + expect(tester.takeException(), isNull); + }); + }); + + group('saved replies at 390px', () { + testWidgets('render without overflowing', (tester) async { + await pump(tester, macrosApp(isAdmin: true, myEmail: 'me@example.com')); + expect(tester.takeException(), isNull); + }); + + testWidgets('fit with the system font scaled up', (tester) async { + await pump( + tester, + macrosApp(isAdmin: true, myEmail: 'me@example.com'), + textScale: 1.5, + ); + expect(tester.takeException(), isNull); + }); + + testWidgets('a member may still write their own', (tester) async { + // The only settings page with a write open to everyone. Hiding the add + // button from a member would remove a feature the API grants them. + await pump(tester, macrosApp(isAdmin: false, myEmail: 'me@example.com')); + expect(find.byTooltip('New saved reply'), findsOneWidget); + }); + + testWidgets('a member gets no controls on a shared reply', (tester) async { + await pump(tester, macrosApp(isAdmin: false, myEmail: 'me@example.com')); + + // Two rows are mine (one personal, one turned-off personal), one is the + // org macro a member may read and not touch. + expect(find.widgetWithText(OutlinedButton, 'Turn off'), findsNothing); + expect(find.widgetWithText(OutlinedButton, 'Delete'), findsOneWidget); + }); + + testWidgets('an admin gets Turn off on the shared reply', (tester) async { + await pump(tester, macrosApp(isAdmin: true, myEmail: 'me@example.com')); + + expect(find.widgetWithText(OutlinedButton, 'Turn off'), findsOneWidget); + expect(find.widgetWithText(OutlinedButton, 'Delete'), findsOneWidget); + }); + + testWidgets('an admin gets nothing on somebody else personal reply', ( + tester, + ) async { + // The one place admin is not an override: the server answers 404 there. + await pump( + tester, + macrosApp(isAdmin: true, myEmail: 'other@example.com'), + ); + + expect(find.widgetWithText(OutlinedButton, 'Delete'), findsNothing); + expect(find.widgetWithText(OutlinedButton, 'Edit'), findsOneWidget); + }); + + testWidgets('a broken placeholder is named, not counted', (tester) async { + await pump(tester, macrosApp(isAdmin: true, myEmail: 'me@example.com')); + + // The author has to know which token to fix. A typo is only obvious + // beside the right spelling. Two widgets carry the token, the body + // preview and the warning; this asserts the warning. + expect( + find.text('%custmer_name% is not recognized and goes out as written'), + findsOneWidget, + ); + }); + + testWidgets('the form offers the scope choice only to an admin', ( + tester, + ) async { + await pump(tester, macrosApp(isAdmin: false, myEmail: 'me@example.com')); + await tester.tap(find.byTooltip('New saved reply')); + await tester.pumpAndSettle(); + + // Not a greyed control: showing a disabled "Everyone" would advertise + // an action the server answers 403 to. + expect(find.text('Who can use it'), findsNothing); + expect(find.textContaining('Saved for you alone'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + + testWidgets('the form inserts a placeholder at the cursor', (tester) async { + await pump(tester, macrosApp(isAdmin: true, myEmail: 'me@example.com')); + await tester.tap(find.byTooltip('New saved reply')); + await tester.pumpAndSettle(); + + await tester.enterText(find.widgetWithText(TextField, 'Reply'), 'Hi '); + await tester.tap(find.text('%customer_name%')); + await tester.pumpAndSettle(); + + final body = tester.widget( + find.widgetWithText(TextField, 'Reply'), + ); + expect(body.controller?.text, 'Hi %customer_name%'); + expect(tester.takeException(), isNull); + }); + }); + + group('the saved-reply picker at 390px', () { + testWidgets('renders without overflowing', (tester) async { + await pump(tester, macroPickerApp()); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + + expect(find.text('Saved replies'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + + testWidgets('fits with the system font scaled up', (tester) async { + await pump(tester, macroPickerApp(), textScale: 1.5); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + }); + + testWidgets('says where to write one when there are none', (tester) async { + // An empty picker with no explanation reads as broken. The settings + // page is two taps away and a member can use it. + await pump(tester, macroPickerApp(macros: const [])); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + + expect(find.textContaining('Organization settings'), findsOneWidget); + }); + + testWidgets('filters as you type', (tester) async { + await pump(tester, macroPickerApp()); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + + // "follow-up", not "follow": the search reads the body as well as the + // title, and the shared reply's body says "follow the link below". That + // is deliberate, since the words you remember are usually in the reply + // rather than in its name, and it is why the narrower term is used here. + await tester.enterText(find.byType(TextField).first, 'follow-up'); + await tester.pumpAndSettle(); + + expect(find.text('My follow-up'), findsOneWidget); + expect(find.textContaining('Password reset'), findsNothing); + }); + + testWidgets('searches the body, not just the title', (tester) async { + await pump(tester, macroPickerApp()); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + + await tester.enterText(find.byType(TextField).first, 'ten minutes'); + await tester.pumpAndSettle(); + + expect(find.textContaining('Password reset'), findsOneWidget); + expect(find.text('My follow-up'), findsNothing); + }); + + testWidgets('marks which replies are shared and which are yours', ( + tester, + ) async { + await pump(tester, macroPickerApp()); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + + // Sending a personal draft believing it is the agreed org wording is the + // mistake this label prevents. + expect(find.text('Everyone'), findsOneWidget); + expect(find.text('Just you'), findsOneWidget); + }); + }); } /// One estimate still to convert, one already billed. @@ -730,3 +1098,100 @@ class _FakeNotifications extends NotificationsNotifier { ], ); } + +/// Three fields across two record types: one required with a real gap, one +/// dropdown, and one already turned off. Between them they light every branch +/// the row and the summary have. +class _FakeCustomFields extends CustomFieldsNotifier { + @override + Future build() async => CustomFieldsState( + fields: [ + CustomFieldDefinition.fromJson(const { + 'id': 'f1', + 'target_model': 'Case', + 'key': 'severity', + 'label': 'Severity', + 'field_type': 'text', + 'is_required': true, + 'is_filterable': true, + 'display_order': 0, + 'is_active': true, + 'records_missing_value': 23, + }), + CustomFieldDefinition.fromJson(const { + 'id': 'f2', + 'target_model': 'Case', + 'key': 'root_cause', + 'label': 'Root cause, as agreed with the customer at close', + 'field_type': 'dropdown', + 'display_order': 1, + 'is_active': true, + 'options': [ + {'value': 'process', 'label': 'Process'}, + {'value': 'people', 'label': 'People'}, + ], + }), + CustomFieldDefinition.fromJson(const { + 'id': 'f3', + 'target_model': 'Lead', + 'key': 'campaign', + 'label': 'Campaign', + 'field_type': 'text', + 'is_active': false, + }), + ], + count: 3, + active: 2, + modelsExtended: 2, + requiredWithGaps: 1, + ); +} + +/// One shared reply, one of mine, and one of mine already turned off. Between +/// them they drive both answers of every control on the screen. +class _FakeMacros extends MacrosNotifier { + @override + Future build() async => MacrosState( + macros: [ + Macro.fromJson(const { + 'id': 'm1', + 'title': 'Password reset, standard wording agreed with legal', + 'body': 'Hi %custmer_name%, follow the link to reset your password.', + 'scope': 'org', + 'owner_name': null, + 'is_active': true, + 'usage_count': 41, + 'unknown_placeholders': ['%custmer_name%'], + }), + Macro.fromJson(const { + 'id': 'm2', + 'title': 'My follow-up', + 'body': 'Just checking in on this one.', + 'scope': 'personal', + 'owner_name': 'me@example.com', + 'is_active': true, + 'usage_count': 0, + }), + Macro.fromJson(const { + 'id': 'm3', + 'title': 'Old holiday notice', + 'body': 'We are closed until January.', + 'scope': 'personal', + 'owner_name': 'me@example.com', + 'is_active': false, + 'usage_count': 7, + }), + ], + placeholders: const [ + MacroPlaceholder( + token: '%customer_name%', + resolves: "The case's first contact", + ), + MacroPlaceholder(token: '%case_id%', resolves: 'The case number'), + ], + orgCount: 1, + personalCount: 2, + inactiveCount: 1, + brokenCount: 1, + ); +} diff --git a/mobile/test/unit/custom_fields_test.dart b/mobile/test/unit/custom_fields_test.dart new file mode 100644 index 00000000..a5a667b3 --- /dev/null +++ b/mobile/test/unit/custom_fields_test.dart @@ -0,0 +1,585 @@ +import 'dart:convert'; + +import 'package:bottle_crm/data/models/custom_field_definition.dart'; +import 'package:bottle_crm/providers/lookup_provider.dart'; +import 'package:bottle_crm/providers/settings_provider.dart'; +import 'package:bottle_crm/services/api_service.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; + +/// Custom fields, the first page of the settings cluster on the phone. +/// +/// What is worth pinning here is mostly about what does NOT go on the wire: +/// +/// - The edit body must not carry `key`, `target_model` or `field_type`. The +/// server freezes all three because values live in each record's +/// `custom_fields` JSON keyed by `key`, so a change orphans, relocates or +/// reinterprets every value already stored. +/// - The reactivate body must carry `is_active` and nothing else. Reusing the +/// edit body would send a `label` the reactivate control does not have, and +/// the field would come back with its name blanked. +/// - The record-form fetch must send `include_counts=false`. Without it every +/// lead, deal and task form open pays for a full scan of the org's records +/// per definition, to compute a number no form displays. +class _FakeClient extends http.BaseClient { + int status = 200; + String body = '{}'; + final List sent = []; + final List bodies = []; + + @override + Future send(http.BaseRequest request) async { + sent.add(request); + final bytes = await request.finalize().toBytes(); + bodies.add(utf8.decode(bytes)); + return http.StreamedResponse( + Stream.value(utf8.encode(body)), + status, + request: request, + ); + } +} + +CustomFieldDefinition _field({ + String id = 'f1', + String targetModel = 'Case', + String key = 'severity', + String label = 'Severity', + CustomFieldType type = CustomFieldType.text, + int order = 0, +}) { + return CustomFieldDefinition( + id: id, + targetModel: targetModel, + key: key, + label: label, + fieldType: type, + displayOrder: order, + ); +} + +void main() { + group('key rules', () { + test('accepts what the serializer accepts', () { + // `validate_key` requires [a-z][a-z0-9_]*. + expect(isValidCustomFieldKey('severity'), isTrue); + expect(isValidCustomFieldKey('region_2'), isTrue); + expect(isValidCustomFieldKey('a'), isTrue); + }); + + test('rejects the four shapes the server rejects', () { + expect(isValidCustomFieldKey('Severity'), isFalse, reason: 'uppercase'); + expect( + isValidCustomFieldKey('2region'), + isFalse, + reason: 'leading digit', + ); + expect(isValidCustomFieldKey('re-gion'), isFalse, reason: 'hyphen'); + expect(isValidCustomFieldKey('re gion'), isFalse, reason: 'space'); + expect(isValidCustomFieldKey(''), isFalse); + }); + }); + + group('suggested key', () { + test('slugs a label into a valid key', () { + expect(suggestCustomFieldKey('Deal source'), 'deal_source'); + expect(suggestCustomFieldKey('Region / Zone'), 'region_zone'); + expect(suggestCustomFieldKey(' Severity '), 'severity'); + }); + + test('gives up rather than suggest a key the server would refuse', () { + // A leading digit fails the server's leading-letter rule, so suggesting + // it would hand the admin a key that cannot be saved. + expect(suggestCustomFieldKey('2nd contact'), ''); + expect(suggestCustomFieldKey('???'), ''); + expect(suggestCustomFieldKey(''), ''); + }); + + test('every non-empty suggestion is a key the server accepts', () { + const labels = [ + 'Deal source', + 'Region / Zone', + 'Renewal date!', + 'ARR (USD)', + 'Tier-1 account', + ]; + for (final label in labels) { + final suggestion = suggestCustomFieldKey(label); + if (suggestion.isEmpty) continue; + expect(isValidCustomFieldKey(suggestion), isTrue, reason: label); + } + }); + }); + + group('draft validation', () { + String? validate({ + bool isCreate = true, + String label = 'Severity', + String key = 'severity', + String targetModel = 'Case', + CustomFieldType type = CustomFieldType.text, + List options = const [], + }) { + return validateCustomFieldDraft( + isCreate: isCreate, + label: label, + key: key, + targetModel: targetModel, + fieldType: type, + options: options, + ); + } + + test('a complete create passes', () { + expect(validate(), isNull); + }); + + test('a blank label is refused on both paths', () { + expect(validate(label: ' '), contains('label')); + expect(validate(isCreate: false, label: ' '), contains('label')); + }); + + test('a blank key is refused on create', () { + expect(validate(key: ''), contains('key')); + }); + + test('a hyphenated key names the hyphen', () { + // The trap worth a specific message: a hyphen reads as natural in a slug + // and is the one character the server refuses without saying why. + expect(validate(key: 'deal-source'), contains('hyphen')); + }); + + test('a target the server does not support is refused', () { + expect(validate(targetModel: 'Widget'), contains('record type')); + }); + + test('the edit path does not check key or target', () { + // Both are frozen after creation, so the edit form does not offer them + // and has nothing to validate. Checking them anyway would block an edit + // over values the form never collected. + expect(validate(isCreate: false, key: '', targetModel: ''), isNull); + }); + + test('a dropdown needs at least one option', () { + expect( + validate(type: CustomFieldType.dropdown), + contains('at least one option'), + ); + expect( + validate( + type: CustomFieldType.dropdown, + options: const [CustomFieldOption(value: '', label: ' ')], + ), + contains('at least one option'), + reason: 'a blank row is not an option', + ); + }); + + test( + 'two options that would collide are refused before the round trip', + () { + final problem = validate( + type: CustomFieldType.dropdown, + options: const [ + CustomFieldOption(value: '', label: 'Very High'), + CustomFieldOption(value: '', label: 'very high'), + ], + ); + expect(problem, contains('very-high')); + }, + ); + + test('an option of only punctuation is refused', () { + expect( + validate( + type: CustomFieldType.dropdown, + options: const [CustomFieldOption(value: '', label: '???')], + ), + contains('letter or number'), + ); + }); + + test('options are ignored on a non-dropdown', () { + // The server refuses a non-empty options list on any other type, but + // that is about the payload, not the draft: leaving stale rows behind + // after switching the type back must not block the save. + expect( + validate( + type: CustomFieldType.text, + options: const [CustomFieldOption(value: '', label: 'Stale')], + ), + isNull, + ); + }); + }); + + group('create payload', () { + Map build({ + CustomFieldType type = CustomFieldType.text, + List options = const [], + }) { + return customFieldCreatePayload( + targetModel: 'Case', + key: ' severity ', + label: ' Severity ', + fieldType: type, + isRequired: true, + isFilterable: false, + displayOrder: 3, + options: options, + ); + } + + test('carries the three fields that are only settable at creation', () { + final body = build(); + expect(body['target_model'], 'Case'); + expect(body['key'], 'severity'); + expect(body['field_type'], 'text'); + }); + + test('trims the key and label', () { + expect(build()['key'], 'severity'); + expect(build()['label'], 'Severity'); + }); + + test('sends no options on a non-dropdown', () { + // `validate_definition_options` answers 400 for a non-empty options list + // on any type but dropdown, so sending one turns a valid text field into + // a rejection. + expect( + build( + options: const [CustomFieldOption(value: 'a', label: 'A')], + ), + isNot(contains('options')), + ); + }); + + test('sends no org and no computed count', () { + // `org` comes from the JWT server-side and `records_missing_value` is + // computed. A payload carrying either would imply the client decided it. + final body = build(); + expect(body.containsKey('org'), isFalse); + expect(body.containsKey('records_missing_value'), isFalse); + expect(body.containsKey('id'), isFalse); + }); + + test('a new dropdown option gets a slugified value', () { + final body = build( + type: CustomFieldType.dropdown, + options: const [CustomFieldOption(value: '', label: 'Very High')], + ); + expect(body['options'], [ + {'value': 'very-high', 'label': 'Very High'}, + ]); + }); + + test('a blank option row is dropped rather than sent', () { + final body = build( + type: CustomFieldType.dropdown, + options: const [ + CustomFieldOption(value: '', label: 'Real'), + CustomFieldOption(value: '', label: ' '), + ], + ); + expect((body['options'] as List), hasLength(1)); + }); + }); + + group('update payload', () { + Map build({ + CustomFieldType type = CustomFieldType.text, + List options = const [], + }) { + return customFieldUpdatePayload( + label: 'Renamed', + fieldType: type, + isRequired: false, + isFilterable: true, + displayOrder: 1, + options: options, + ); + } + + test('carries none of the three frozen fields', () { + // This is the assertion the whole edit path rests on. The server answers + // 400 for a change to any of them, and sending the unchanged value would + // pass only by luck: `validate` compares against the instance, so a + // stale value from a list fetched before someone else edited the row + // would be a rejection the admin could not explain. + final body = build(); + expect(body.containsKey('key'), isFalse); + expect(body.containsKey('target_model'), isFalse); + expect(body.containsKey('field_type'), isFalse); + }); + + test('carries what an edit may change', () { + final body = build(); + expect(body['label'], 'Renamed'); + expect(body['is_required'], false); + expect(body['is_filterable'], true); + expect(body['display_order'], 1); + }); + + test('an existing option keeps its stored value when relabelled', () { + // Every record holding this option refers to it by value. Re-slugifying + // on a relabel would leave all of them pointing at an option that no + // longer exists. + final body = build( + type: CustomFieldType.dropdown, + options: const [ + CustomFieldOption(value: 'very-high', label: 'Critical'), + ], + ); + expect(body['options'], [ + {'value': 'very-high', 'label': 'Critical'}, + ]); + }); + }); + + group('activate payload', () { + test('is is_active and nothing else', () { + // There is no reactivate endpoint, so this rides the edit PUT. If it + // carried the edit body's `label`, a reactivate would blank the field's + // name, because the control that sends it has no label to send. + expect(customFieldActivatePayload(), {'is_active': true}); + }); + }); + + group('parsing', () { + test('reads the counts the settings screen shows', () { + final field = CustomFieldDefinition.fromJson({ + 'id': 'f1', + 'target_model': 'Lead', + 'key': 'source', + 'label': 'Source', + 'field_type': 'dropdown', + 'is_required': true, + 'is_filterable': true, + 'display_order': 2, + 'is_active': false, + 'records_missing_value': 23, + 'options': [ + {'value': 'web', 'label': 'Web'}, + ], + }); + expect(field.recordsMissingValue, 23); + expect(field.isFilterable, isTrue); + expect(field.isRequired, isTrue); + expect(field.isActive, isFalse); + expect(field.fieldType, CustomFieldType.dropdown); + expect(field.options.single.value, 'web'); + }); + + test('defaults the count to zero when the server omitted it', () { + // The form path asks for `include_counts=false`, and the response then + // carries no `records_missing_value` at all. + final field = CustomFieldDefinition.fromJson({ + 'id': 'f1', + 'key': 'source', + 'label': 'Source', + }); + expect(field.recordsMissingValue, 0); + expect(field.isFilterable, isFalse); + }); + + test('an unknown field type falls back to text rather than throwing', () { + final field = CustomFieldDefinition.fromJson({ + 'id': 'f1', + 'field_type': 'colour_picker', + }); + expect(field.fieldType, CustomFieldType.text); + }); + }); + + group('grouping', () { + test('groups by record type, ordered by the name shown', () { + // "Deals" before "Tickets" is the label order, not the target_model + // order: `Opportunity` sorts after `Case` and would put Tickets first. + const state = CustomFieldsState( + fields: [ + CustomFieldDefinition( + id: 'a', + targetModel: 'Case', + key: 'a', + label: 'A', + fieldType: CustomFieldType.text, + ), + CustomFieldDefinition( + id: 'b', + targetModel: 'Opportunity', + key: 'b', + label: 'B', + fieldType: CustomFieldType.text, + ), + ], + ); + expect(state.grouped.map((e) => e.key), ['Opportunity', 'Case']); + }); + + test('orders fields by display_order, then label', () { + final state = CustomFieldsState( + fields: [ + _field(id: '1', label: 'Zulu', order: 0), + _field(id: '2', label: 'Alpha', order: 5), + _field(id: '3', label: 'Bravo', order: 0), + ], + ); + expect(state.grouped.single.value.map((f) => f.label), [ + 'Bravo', + 'Zulu', + 'Alpha', + ]); + }); + }); + + group('the provider', () { + late ProviderContainer container; + late _FakeClient client; + + setUp(() { + client = _FakeClient(); + ApiService().setClientForTesting(client); + container = ProviderContainer(); + }); + + tearDown(() => container.dispose()); + + // Riverpod 3 providers are auto-dispose, so a bare `read(p.future)` holds + // no subscription and can be torn down mid-request. Listen first. + Future readFields() { + container.listen(customFieldsProvider, (_, _) {}); + return container.read(customFieldsProvider.future); + } + + test('reads the definitions and the server totals', () async { + client.body = ''' + {"definitions": [ + {"id": "f1", "target_model": "Case", "key": "severity", + "label": "Severity", "field_type": "text", "is_required": true, + "is_active": true, "records_missing_value": 4} + ], + "totals": {"count": 9, "active": 7, "models_extended": 3, + "required_with_gaps": 2}} + '''; + final state = await readFields(); + expect(state.fields.single.label, 'Severity'); + // Taken from `totals`, not recounted from the rows. The server computes + // them over the org's whole definition set. + expect(state.count, 9); + expect(state.active, 7); + expect(state.modelsExtended, 3); + expect(state.requiredWithGaps, 2); + }); + + test('falls back to counting the rows when totals are absent', () async { + client.body = ''' + {"definitions": [ + {"id": "f1", "target_model": "Case", "key": "a", "label": "A", + "is_active": true, "is_required": true, "records_missing_value": 1}, + {"id": "f2", "target_model": "Lead", "key": "b", "label": "B", + "is_active": false} + ]} + '''; + final state = await readFields(); + expect(state.count, 2); + expect(state.active, 1); + expect(state.modelsExtended, 1, reason: 'only the active row counts'); + expect(state.requiredWithGaps, 1); + }); + + test('asks for the inactive rows too', () async { + // This screen is the one that turns a field back on, and it cannot offer + // that for a row it never fetched. + client.body = '{"definitions": []}'; + await readFields(); + expect(client.sent.single.url.queryParameters, isEmpty); + }); + + test('a create POSTs the payload it was given', () async { + client.body = '{"definitions": []}'; + await readFields(); + await container.read(customFieldsProvider.notifier).createField({ + 'key': 'severity', + 'label': 'Severity', + }); + final post = client.sent.firstWhere((r) => r.method == 'POST'); + expect(post.url.path, endsWith('/custom-fields/')); + expect(jsonDecode(client.bodies[client.sent.indexOf(post)]), { + 'key': 'severity', + 'label': 'Severity', + }); + }); + + test('turning a field off is a DELETE on the row', () async { + client.body = '{"definitions": []}'; + await readFields(); + await container.read(customFieldsProvider.notifier).deactivateField('f1'); + final sent = client.sent.firstWhere((r) => r.method == 'DELETE'); + expect(sent.url.path, endsWith('/custom-fields/f1/')); + }); + + test('turning a field on is a PUT carrying only is_active', () async { + client.body = '{"definitions": []}'; + await readFields(); + await container.read(customFieldsProvider.notifier).activateField('f1'); + final put = client.sent.firstWhere((r) => r.method == 'PUT'); + expect(put.url.path, endsWith('/custom-fields/f1/')); + expect(jsonDecode(client.bodies[client.sent.indexOf(put)]), { + 'is_active': true, + }); + }); + + test('a refusal is reported in the server own words', () async { + client.body = '{"definitions": []}'; + await readFields(); + client.status = 403; + client.body = '{"error": true, "errors": "Admin access required"}'; + final message = await container + .read(customFieldsProvider.notifier) + .createField({'key': 'x'}); + expect(message, 'Admin access required'); + }); + + test('a field-level 400 reports the field message', () async { + client.body = '{"definitions": []}'; + await readFields(); + client.status = 400; + client.body = + '{"error": true, "errors": {"key": ["key must be a lowercase slug ' + 'starting with a letter (a-z, 0-9, _)"]}}'; + final message = await container + .read(customFieldsProvider.notifier) + .createField({'key': 'Bad-Key'}); + expect(message, contains('lowercase slug')); + }); + }); + + group('the record-form fetch', () { + late ProviderContainer container; + late _FakeClient client; + + setUp(() { + client = _FakeClient(); + ApiService().setClientForTesting(client); + container = ProviderContainer(); + }); + + tearDown(() => container.dispose()); + + test('opts out of the counts the settings screen needs', () async { + // Without `include_counts=false` the endpoint runs one COUNT over the + // org's records per target model plus a `custom_fields ? key` scan per + // definition, on every lead, deal and task form open, to compute a + // number no form displays. The web opts out on this same path. + client.body = '{"definitions": []}'; + container.listen(customFieldDefinitionsProvider('Lead'), (_, _) {}); + await container.read(customFieldDefinitionsProvider('Lead').future); + + final params = client.sent.single.url.queryParameters; + expect(params['include_counts'], 'false'); + expect(params['target_model'], 'Lead'); + expect(params['active_only'], 'true'); + }); + }); +} diff --git a/mobile/test/unit/macros_test.dart b/mobile/test/unit/macros_test.dart new file mode 100644 index 00000000..24f8c796 --- /dev/null +++ b/mobile/test/unit/macros_test.dart @@ -0,0 +1,454 @@ +import 'dart:convert'; + +import 'package:bottle_crm/data/models/macro.dart'; +import 'package:bottle_crm/providers/settings_provider.dart'; +import 'package:bottle_crm/services/api_service.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; + +/// Saved replies, the one settings page every member can write to. +/// +/// The three things worth pinning: +/// +/// - Who may write. An org macro needs an admin, a personal one needs its +/// owner, and an admin gets no exception on somebody else's personal macro. +/// `MacroDetailView._get_writable` answers 404 rather than 403 there, so the +/// id space cannot be used to find out whose private replies exist. +/// - What removing one does. `DELETE` turns an org macro off and deletes a +/// personal one for good, from the same endpoint. A dialog that says the +/// wrong one is the defect. +/// - `owner` never goes on the wire. It is `read_only` on the serializer and +/// re-derived from `request.profile`; a client that could name one could +/// file a saved reply as somebody else. +class _FakeClient extends http.BaseClient { + int status = 200; + String body = '{}'; + final List sent = []; + final List bodies = []; + + @override + Future send(http.BaseRequest request) async { + sent.add(request); + final bytes = await request.finalize().toBytes(); + bodies.add(utf8.decode(bytes)); + return http.StreamedResponse( + Stream.value(utf8.encode(body)), + status, + request: request, + ); + } +} + +Macro _macro({ + String id = 'm1', + String title = 'Password reset', + String body = 'Hello %customer_name%', + String scope = Macro.scopeOrg, + String? ownerEmail, + bool isActive = true, +}) { + return Macro( + id: id, + title: title, + body: body, + scope: scope, + ownerEmail: ownerEmail, + isActive: isActive, + ); +} + +void main() { + group('who may write a macro', () { + // Both answers, for both scopes. A check that can only return one of them + // is a check that is not running. + test('an admin may write an org macro', () { + expect( + canWriteMacro( + isAdmin: true, + scope: Macro.scopeOrg, + ownerEmail: null, + myEmail: 'me@example.com', + ), + isTrue, + ); + }); + + test('a member may not write an org macro', () { + expect( + canWriteMacro( + isAdmin: false, + scope: Macro.scopeOrg, + ownerEmail: null, + myEmail: 'me@example.com', + ), + isFalse, + ); + }); + + test('anyone may write their own personal macro', () { + expect( + canWriteMacro( + isAdmin: false, + scope: Macro.scopePersonal, + ownerEmail: 'me@example.com', + myEmail: 'me@example.com', + ), + isTrue, + ); + }); + + test('an admin may NOT write somebody else personal macro', () { + // The one place admin is not an override. The server answers 404 there, + // so offering Edit would produce a refusal that reads like a bug and + // would confirm the row exists. + expect( + canWriteMacro( + isAdmin: true, + scope: Macro.scopePersonal, + ownerEmail: 'someone@example.com', + myEmail: 'me@example.com', + ), + isFalse, + ); + }); + + test('the email comparison ignores case and surrounding space', () { + expect( + canWriteMacro( + isAdmin: false, + scope: Macro.scopePersonal, + ownerEmail: ' Me@Example.com ', + myEmail: 'me@example.com', + ), + isTrue, + ); + }); + + test('an unknown email hides the action rather than offering it', () { + for (final pair in [ + (owner: null, me: 'me@example.com'), + (owner: 'me@example.com', me: null), + (owner: '', me: 'me@example.com'), + ]) { + expect( + canWriteMacro( + isAdmin: false, + scope: Macro.scopePersonal, + ownerEmail: pair.owner, + myEmail: pair.me, + ), + isFalse, + reason: '${pair.owner} / ${pair.me}', + ); + } + }); + }); + + group('what removing one does', () { + test('an org macro is turned off, and the dialog says so', () { + final removal = macroRemoval(_macro(scope: Macro.scopeOrg)); + expect(removal.actionLabel, 'Turn off'); + expect(removal.isPermanent, isFalse); + expect(removal.detail, contains('turn it back on')); + expect(removal.detail, isNot(contains('for good'))); + }); + + test('a personal macro is deleted, and the dialog says so', () { + // Same endpoint, opposite outcome. Calling this "Turn off" would promise + // a row that is about to stop existing. + final removal = macroRemoval( + _macro(scope: Macro.scopePersonal, ownerEmail: 'me@example.com'), + ); + expect(removal.actionLabel, 'Delete'); + expect(removal.isPermanent, isTrue); + expect(removal.detail, contains('for good')); + }); + + test('the dialog names the macro and the verb it is about to apply', () { + expect( + macroRemoval(_macro(title: 'Refund policy')).title, + 'Turn off Refund policy?', + ); + expect( + macroRemoval( + _macro( + title: 'Refund policy', + scope: Macro.scopePersonal, + ownerEmail: 'me@example.com', + ), + ).title, + 'Delete Refund policy?', + ); + }); + }); + + group('draft validation', () { + test('a complete draft passes', () { + expect( + validateMacroDraft(title: 'Hi', body: 'There', scope: Macro.scopeOrg), + isNull, + ); + }); + + test('both a title and a body are required', () { + // `MacroSerializer` requires both, so a blank one is a 400 rather than a + // saved-but-empty row. + expect( + validateMacroDraft(title: ' ', body: 'x', scope: Macro.scopeOrg), + contains('title'), + ); + expect( + validateMacroDraft(title: 'x', body: ' ', scope: Macro.scopeOrg), + contains('something to say'), + ); + }); + + test('a scope outside the two is refused', () { + expect( + validateMacroDraft(title: 'x', body: 'y', scope: 'everyone'), + contains('who this reply is for'), + ); + }); + }); + + group('the payload', () { + test('never carries owner, org or usage_count', () { + // `owner` is read_only and re-derived from request.profile. A client + // that could name one could file a saved reply as somebody else. + final body = macroPayload( + title: ' Hi ', + body: ' There ', + scope: Macro.scopePersonal, + ); + expect(body.keys.toSet(), {'title', 'body', 'scope'}); + }); + + test('trims the title and body', () { + final body = macroPayload( + title: ' Hi ', + body: ' There ', + scope: Macro.scopeOrg, + ); + expect(body['title'], 'Hi'); + expect(body['body'], 'There'); + }); + + test('the activate payload is is_active and nothing else', () { + // A reactivate control has no title or body to send, and the serializer + // requires both on a full write. + expect(macroActivatePayload(), {'is_active': true}); + }); + }); + + group('parsing', () { + test('reads the fields the row shows', () { + final macro = Macro.fromJson({ + 'id': 'm1', + 'title': 'Password reset', + 'body': 'Hi %customer_name%, try %reset_link%', + 'scope': 'personal', + 'owner': 'p1', + 'owner_name': 'me@example.com', + 'is_active': false, + 'usage_count': 12, + 'unknown_placeholders': ['%reset_link%'], + }); + expect(macro.isPersonal, isTrue); + expect(macro.scopeLabel, 'Just you'); + expect(macro.ownerEmail, 'me@example.com'); + expect(macro.isActive, isFalse); + expect(macro.usageCount, 12); + expect(macro.unknownPlaceholders, ['%reset_link%']); + }); + + test('an org macro has no owner and says Everyone', () { + final macro = Macro.fromJson({ + 'id': 'm1', + 'scope': 'org', + 'owner': null, + 'owner_name': null, + }); + expect(macro.ownerEmail, isNull); + expect(macro.scopeLabel, 'Everyone'); + }); + + test('an empty owner_name reads as unknown, not as an owner', () { + // An empty string compared against an empty email would make everybody + // the owner of it. `canWriteMacro` refuses an empty either way, and this + // keeps the two from having to agree separately. + expect( + Macro.fromJson({'id': 'm1', 'owner_name': ' '}).ownerEmail, + isNull, + ); + }); + + test('broken placeholders come from the server, not from a local regex', () { + // Nothing here recomputes them. A second implementation of the token set + // would drift from `macros/render.py` the first time either changed, and + // the client's guess is not what goes out to the customer. + final macro = Macro.fromJson({ + 'id': 'm1', + 'body': 'Hi %customer_name% and %nonsense%', + 'unknown_placeholders': [], + }); + expect(macro.unknownPlaceholders, isEmpty); + }); + }); + + group('the provider', () { + late ProviderContainer container; + late _FakeClient client; + + setUp(() { + client = _FakeClient(); + ApiService().setClientForTesting(client); + container = ProviderContainer(); + }); + + tearDown(() => container.dispose()); + + Future readMacros() { + container.listen(macrosProvider, (_, _) {}); + return container.read(macrosProvider.future); + } + + Future> readActive() { + container.listen(activeMacrosProvider, (_, _) {}); + return container.read(activeMacrosProvider.future); + } + + test('reads the rows, the totals and the placeholder reference', () async { + client.body = ''' + {"results": [ + {"id": "m1", "title": "Reset", "body": "Hi", "scope": "org", + "is_active": true, "usage_count": 3, "unknown_placeholders": []} + ], + "totals": {"count": 5, "org": 3, "personal": 2, "inactive": 1, + "with_unknown_placeholders": 1}, + "placeholders": [ + {"token": "%customer_name%", "resolves": "The case's first contact"} + ]} + '''; + final state = await readMacros(); + expect(state.macros.single.title, 'Reset'); + expect(state.orgCount, 3); + expect(state.personalCount, 2); + expect(state.inactiveCount, 1); + expect(state.brokenCount, 1); + expect(state.placeholders.single.token, '%customer_name%'); + }); + + test('the settings list asks for the turned-off rows too', () async { + // It is the screen that turns one back on, so it cannot filter them out. + client.body = '{"results": []}'; + await readMacros(); + expect(client.sent.single.url.queryParameters, isEmpty); + }); + + test('the reply-box list asks only for the active ones', () async { + // A turned-off reply must not be offered on a ticket at all: the render + // endpoint answers 400 for one, so it would be a dead row in a picker. + client.body = '{"results": []}'; + await readActive(); + expect(client.sent.single.url.queryParameters['active'], 'true'); + }); + + test('an edit is a PATCH, not a PUT', () async { + // `MacroDetailView.put` runs the serializer non-partial and + // `MacroSerializer` requires both title and body, so PUT is a 400 + // waiting to happen the moment a caller sends a partial body. + client.body = '{"results": []}'; + await readMacros(); + await container.read(macrosProvider.notifier).updateMacro('m1', { + 'title': 'x', + }); + expect(client.sent.any((r) => r.method == 'PUT'), isFalse); + final patch = client.sent.firstWhere((r) => r.method == 'PATCH'); + expect(patch.url.path, endsWith('/macros/m1/')); + }); + + test('turning one on sends only is_active', () async { + client.body = '{"results": []}'; + await readMacros(); + await container.read(macrosProvider.notifier).activateMacro('m1'); + final patch = client.sent.firstWhere((r) => r.method == 'PATCH'); + expect(jsonDecode(client.bodies[client.sent.indexOf(patch)]), { + 'is_active': true, + }); + }); + + test('removing one is a DELETE on the row', () async { + client.body = '{"results": []}'; + await readMacros(); + await container.read(macrosProvider.notifier).removeMacro('m1'); + final sent = client.sent.firstWhere((r) => r.method == 'DELETE'); + expect(sent.url.path, endsWith('/macros/m1/')); + }); + + test('the scope refusal is reported in the server own words', () async { + // These endpoints answer `{"error": ...}`, not the `{"errors": ...}` the + // rest of the app uses, and the wording is the difference between a bug + // and a rule the user can act on. + client.body = '{"results": []}'; + await readMacros(); + client.status = 403; + client.body = '{"error": "Only admins can manage org-scope macros."}'; + final message = await container.read(macrosProvider.notifier).createMacro( + {'title': 'x', 'body': 'y', 'scope': 'org'}, + ); + expect(message, 'Only admins can manage org-scope macros.'); + }); + + test('a 404 on somebody else macro is reported as given', () async { + client.body = '{"results": []}'; + await readMacros(); + client.status = 404; + client.body = '{"detail": "Not found."}'; + final message = await container + .read(macrosProvider.notifier) + .removeMacro('m2'); + expect(message, 'Not found.'); + }); + }); + + group('expanding one against a ticket', () { + late _FakeClient client; + + setUp(() { + client = _FakeClient(); + ApiService().setClientForTesting(client); + }); + + test('posts the ticket id and returns the rendered text', () async { + client.body = '{"rendered_body": "Hi Dana, about ticket 42"}'; + final result = await renderMacro(macroId: 'm1', ticketId: 't42'); + + expect(result.text, 'Hi Dana, about ticket 42'); + expect(result.error, isNull); + final sent = client.sent.single; + expect(sent.method, 'POST'); + expect(sent.url.path, endsWith('/macros/m1/render/')); + expect(jsonDecode(client.bodies.single), {'case_id': 't42'}); + }); + + test('reports the refusal rather than inserting nothing', () async { + client.status = 400; + client.body = '{"error": "Macro is inactive."}'; + final result = await renderMacro(macroId: 'm1', ticketId: 't42'); + + expect(result.text, isNull); + expect(result.error, 'Macro is inactive.'); + }); + + test('a response with no body is an error, not an empty insert', () async { + // Silently inserting nothing would look like the tap did not register, + // and the agent would send an empty reply. + client.body = '{}'; + final result = await renderMacro(macroId: 'm1', ticketId: 't42'); + + expect(result.text, isNull); + expect(result.error, isNotNull); + }); + }); +}