enferno/admin/views.py (1)

6682-6697: Avoid leaking exception messages to clients

Mirror the pattern above: log exception with stack; return a generic 500 to callers.

-    except Exception as e:
-        # Log the error and return a generic server error response
-        return jsonify({"errors": [{"message": str(e)}]}), 500
+    except Exception:
+        logger.exception("api_dynamic_field failed")
+        return jsonify({"errors": [{"message": "Internal server error"}]}), 500

enferno/admin/templates/admin/partials/edit_related_bulletins.html (1)

1-1: Potential spacing regression: removing my-5 may crowd the dialog

Dropping the vertical margin can compress adjacent sections in incident_dialog. Verify spacing across small/medium breakpoints and consider applying margin at the container level if this was intentional.

enferno/admin/templates/admin/partials/edit_related_incidents.html (1)

1-1: Double-check vertical spacing consistency

Same as related_bulletins: confirm the dialog still has adequate visual separation without my-5, or move spacing to the parent container for consistency.

enferno/static/js/components/FieldRenderer.js (1)

56-63: Dropdowns should default items to [] to avoid runtime warnings

field.options may be undefined; v-select expects an array.

Apply this diff:

-                    items: field.options,
+                    items: Array.isArray(field.options) ? field.options : [],
@@
-                    items: field.options,
+                    items: Array.isArray(field.options) ? field.options : [],

Also applies to: 64-71

enferno/static/js/components/ConfirmDialog.js (3)

16-17: Remove unused reject plumbing; confirm() resolves boolean only

rejectPromise is stored but never used; simplify API and state.

Apply this diff:

       resolvePromise: null,
-      rejectPromise: null,

-      return new Promise((resolve, rejectPromise) => {
-        this.resolvePromise = resolve;
-        this.rejectPromise = rejectPromise;
-      });
+      return new Promise((resolve) => {
+        this.resolvePromise = resolve;
+      });

-        this.resolvePromise = null;
-        this.rejectPromise = null;
+        this.resolvePromise = null;

Also applies to: 42-45, 75-76

---

63-66: Log acceptance errors as errors

Use console.error for failures; keep UI behavior unchanged.

Apply this diff:

-      } catch (error) {
-        console.log('Something failed when confirming dialog', error)
+      } catch (error) {
+        console.error('Something failed when confirming dialog', error)

---

7-9: Avoid cleanup race if a new dialog opens within 300ms

The delayed cleanup can clobber props of a newly opened dialog. Track and clear the timeout.

Apply this diff:

       loading: false,
+      cleanupTimer: null,
       dialogProps: {},

-    cleanup() {
-      setTimeout(() => {
+    cleanup() {
+      clearTimeout(this.cleanupTimer);
+      this.cleanupTimer = setTimeout(() => {
         this.loading = false;
         this.onAccept = null;
         this.onReject = null;
         this.resolvePromise = null;
-        this.rejectPromise = null;
         this.cancelProps = { text: 'Cancel', variant: 'outlined' };
         this.acceptProps = { text: 'Confirm', color: 'primary', variant: 'flat' };
         this.dialogProps = {};
-      }, 300)
+      }, 300)
     },

Also applies to: 70-81

enferno/admin/templates/admin/system-administration.html (1)

1018-1027: Vuetify color tokens: use v3-friendly names

Prefer hyphenated tokens for consistency with Vuetify 3 theming (matches existing style if already used elsewhere).

Apply this diff:

-                <div class="d-flex align-center body-2">
-                    <v-avatar class="mr-2" size="18" color="red lighten-3"></v-avatar>
+                <div class="d-flex align-center body-2">
+                    <v-avatar class="mr-2" size="18" color="red-lighten-3"></v-avatar>
                     {{ _('Old Value') }}
                 </div>
 
                 <div class="d-flex align-center ml-4 body-2">
-                    <v-avatar class="mr-2" size="18" color="green lighten-2"></v-avatar>
+                    <v-avatar class="mr-2" size="18" color="green-lighten-2"></v-avatar>
                     {{ _('New Value') }}
                 </div>

enferno/admin/templates/admin/partials/actor_dialog.html (1)

904-915: LGTM on wrapping partials; standardize grid prop.

The new cols="12" wrappers look good and keep each partial isolated. One nit: earlier in this file Line 896 uses col="12" (singular). In Vuetify, the prop is cols. Please align for consistency.

Apply:

-<v-col col="12">
+<v-col cols="12">

enferno/admin/templates/admin/partials/incident_dialog.html (1)

133-140: Switching to partials is good; align grid prop usage.

These includes are clean and consistent. Similar to actor dialog, Line 123 above still uses col="12". Use cols="12" for consistency.

-<v-col col="12">
+<v-col cols="12">

enferno/static/js/mixins/global-mixin.js (1)

3-6: Prefer global component registration over mixin‐level
No calls to app.component('confirm-dialog', …) or app.mixin(globalMixin) were found. Instead of injecting ConfirmDialog and Toast via globalMixin in every createApp call, register them once at bootstrap (e.g. in main.js):

import { createApp } from 'vue'
import App from './App.vue'
import ConfirmDialog from '@/components/ConfirmDialog.vue'
import Toast from '@/components/Toast.vue'

const app = createApp(App)
app.component('confirm-dialog', ConfirmDialog)
app.component('toast', Toast)
app.mount('#app')

Remove them from globalMixin to ensure they’re always available without needing to apply the mixin.

enferno/static/js/mixins/form-builder-mixin.js (2)

10-12: Safer width class handling

Coerce width to a non-empty string; fall back to 'w-100' if absent.

-    fieldClass(field) {
-      return [field?.ui_config?.width || 'w-100', 'px-3 py-2']
-    },
+    fieldClass(field) {
+      const width = (typeof field?.ui_config?.width === 'string' && field.ui_config.width.trim())
+        ? field.ui_config.width.trim()
+        : 'w-100';
+      return [width, 'px-3 py-2'];
+    },

---

18-22: Sort should tolerate missing sort_order and avoid in-place mutation

Provide a numeric fallback and sort a shallow copy to avoid unexpected reactivity glitches.

-    sortDynamicFields() {
-      this.formBuilder.dynamicFields = this.formBuilder.dynamicFields.sort((a, b) => {
-        return a.sort_order - b.sort_order; // normal sort
-      });
-    },
+    sortDynamicFields() {
+      const hi = Number.MAX_SAFE_INTEGER;
+      this.formBuilder.dynamicFields = [...this.formBuilder.dynamicFields].sort(
+        (a, b) => (a?.sort_order ?? hi) - (b?.sort_order ?? hi)
+      );
+    },

enferno/admin/models/core_fields.py (2)

6-172: Align field_type and options with the DynamicField contract

Types like long_text, multi_select, single_select, html_block must be recognized by DynamicField. Also, “status” options here may diverge from statuses configured elsewhere (e.g., DB/ACL driven lists).

• Confirm DynamicField’s allowed types include the above.
• Avoid dual sources of truth for Status; consider sourcing options from the canonical statuses table/API or marking this core field as “options_managed=true” to prevent drift.

---

175-211: Seed updates and logging

The seeder only creates missing rows and prints to stdout. Prefer idempotent upserts and structured logging.

- from enferno.admin.models.DynamicField import DynamicField
+ from enferno.admin.models.DynamicField import DynamicField
+ from enferno.utils.logging_utils import get_logger
+ logger = get_logger()
@@
-            field.save()
-            print(f"✅ Created core field: {name}")
+            field.save()
+            logger.info("Created core field: %s", name)
         else:
-            print(f"✓ Core field already exists: {name}")
+            # Optionally upsert changed attributes to keep core fields in sync
+            updated = False
+            for attr, key in (
+                ("title", "title"),
+                ("field_type", "field_type"),
+                ("ui_component", "ui_component"),
+                ("sort_order", "sort_order"),
+                ("active", "visible"),
+            ):
+                new_val = config.get(key, getattr(existing, attr))
+                if getattr(existing, attr) != new_val:
+                    setattr(existing, attr, new_val)
+                    updated = True
+            # ui_config/options reconciliation
+            desired_ui = {"html_template": config.get("html_template")} if config.get("html_template") else {}
+            if (existing.ui_config or {}) != desired_ui:
+                existing.ui_config = desired_ui
+                updated = True
+            if config.get("options") is not None and existing.options != config["options"]:
+                existing.options = config["options"]
+                updated = True
+            if updated:
+                existing.save()
+                logger.info("Updated core field: %s", name)
+            else:
+                logger.debug("Core field already up-to-date: %s", name)

I can wire this into your CLI seed flow if you want a single “seed-core-fields” command.

enferno/admin/models/Bulletin.py (1)

23-23: Avoid potential circular import by localizing DynamicField import

If DynamicField imports Bulletin (directly/indirectly), the top-level import can create a cycle. Import where needed.

-from enferno.admin.models.DynamicField import DynamicField
+# Move this import into to_dict to avoid import cycles:
+# from enferno.admin.models.DynamicField import DynamicField

Add inside to_dict before use:

from enferno.admin.models.DynamicField import DynamicField  # localized import

Please confirm DynamicField doesn’t import Bulletin to avoid cycles; if it does, localize the import as suggested.

enferno/static/js/components/SelectFieldTypeDialog.js (3)

71-74: Normalize options default to an array (avoid nulls for non-selects)

Downstream code often expects options to be iterable. Prefer [] over null for non-select field types.

-          options: (field_type === 'single_select' || field_type === 'multi_select') ? [{ id: 1, label: this.translations.optionN_(1), value: 'option_1', hidden: false }] : null,
+          options: (field_type === 'single_select' || field_type === 'multi_select')
+            ? [{ label: this.translations.optionN_(1), value: 'option_1' }]
+            : [],

---

96-98: Add :key to v-for for stable rendering

Vue warns without a key; also improves diffing and performance.

-            <v-col v-for="({ label, field_type, ui_component, icon }) in componentTypes" cols="12" md="4">
+            <v-col
+              v-for="({ label, field_type, ui_component, icon }, idx) in componentTypes"
+              :key="`${field_type}-${ui_component}-${idx}`"
+              cols="12"
+              md="4">

---

84-84: Tweak log message to reflect action

You’re not saving here; you’re constructing and emitting.

-        console.error('Error saving field:', err);
+        console.error('Error creating field:', err);

enferno/static/js/diff-tool.js (2)

133-138: Wrap header cells in a row

<thead> should contain a <tr>. Minor HTML validity/semantics fix.

-        return `<table class="text-left w-100" style="table-layout: fixed; border-collapse: collapse;">
-                    <thead>
-                        <th class="border-b pa-1">${window?.translations?.field_ ?? 'Field'}</th>
-                        <th class="border-b pa-1">${window?.translations?.previous_ ?? 'Previous'}</th>
-                        <th class="border-b pa-1">${window?.translations?.updated_ ?? 'Updated'}</th>
-                    </thead>
+        return `<table class="text-left w-100" style="table-layout: fixed; border-collapse: collapse;">
+                    <thead>
+                      <tr>
+                        <th class="border-b pa-1">${window?.translations?.field_ ?? 'Field'}</th>
+                        <th class="border-b pa-1">${window?.translations?.previous_ ?? 'Previous'}</th>
+                        <th class="border-b pa-1">${window?.translations?.updated_ ?? 'Updated'}</th>
+                      </tr>
+                    </thead>

---

145-148: Expose idKey/options through getAndRenderDiff

Forward options (e.g., custom idKey) to getDiff and keep labels positioning intact.

-    getAndRenderDiff(obj1 = {}, obj2 = {}, labels = {}) {
-        const diff = DiffTool.getDiff(obj1, obj2);
+    getAndRenderDiff(obj1 = {}, obj2 = {}, labels = {}, options = {}) {
+        const diff = DiffTool.getDiff(obj1, obj2, options);
         return DiffTool.renderDiff(diff, labels);
     }

enferno/commands.py (2)

369-377: Exception handling: avoid blind catches and log with tracebacks

Catching bare Exception obscures actionable errors, and logger.error drops tracebacks. Prefer narrower exceptions where possible and use logger.exception(...).

-    except Exception as e:
-        logger.error(f"Error during initial cleanup: {str(e)}")
+    except Exception:
+        logger.exception("Error during initial cleanup")
...
-            except Exception as e:
-                logger.error(f"Error creating field {field_data['name']}: {str(e)}")
+            except Exception:
+                logger.exception("Error creating field %s", field_data["name"])
...
-        except Exception as e:
-            logger.error(f"Error testing bulletin: {str(e)}")
+        except Exception:
+            logger.exception("Error testing bulletin")
...
-    except Exception as e:
-        logger.error(f"Error in test_dynamic_fields: {str(e)}")
+    except Exception:
+        logger.exception("Error in test_dynamic_fields")
...
-            except Exception as e:
-                logger.error(f"Error cleaning up field {field.name}: {str(e)}")
+            except Exception:
+                logger.exception("Error cleaning up field %s", field.name)
...
-    except Exception as e:
-        logger.error(f"Error in cleanup: {str(e)}")
+    except Exception:
+        logger.exception("Error in cleanup")

Also applies to: 503-509, 534-540, 552-557, 577-580, 586-589

---

565-569: Delete all test bulletins, not just the first

Cleanup should remove every “Test Bulletin”, not only the first match.

-        test_bulletin = Bulletin.query.filter_by(title="Test Bulletin").first()
-        if test_bulletin:
-            test_bulletin.delete()
-            db.session.commit()
+        test_bulletins = Bulletin.query.filter_by(title="Test Bulletin").all()
+        for b in test_bulletins:
+            b.delete()
+        db.session.commit()

enferno/static/js/common/config.js (2)

95-117: Make focus more resilient after scrolling to the first invalid field

If the invalid element isn’t focusable (wrapper divs), focus will silently fail. Prefer focussing a child input if present.

   element.scrollIntoView({
       behavior: 'smooth',
       block: 'center',
   })
-
-  if (element.focus) {
-      setTimeout(() => element.focus(), 300) // give time for scroll
-  }
+  // Focus the element or a focusable child after scrolling
+  const focusTarget =
+    (typeof element.focus === 'function' && element) ||
+    element.querySelector?.('input, textarea, select, [tabindex]');
+  if (focusTarget && typeof focusTarget.focus === 'function') {
+    setTimeout(() => focusTarget.focus(), 300);
+  }

---

713-722: slugify: non‑Latin labels can yield empty values; add a safe fallback

For Arabic or other non‑Latin titles, the current regex may strip everything and leave an empty value, which later becomes an invalid key/value. Add a fallback.

 function slugify(text) {
-    return text
+    const out = text
         .toString()
         .toLowerCase()
         .trim()
         .replace(/\s+/g, '_')       // Replace spaces with underscores
         .replace(/[^\w\-]+/g, '')   // Remove non-word characters
-        .replace(/\_\_+/g, '_');    // Replace multiple underscores
+        .replace(/\_\_+/g, '_');    // Replace multiple underscores
+    // Fallback if result is blank (e.g., only non-Latin chars)
+    return out || 'value_' + Math.random().toString(36).slice(2, 8);
 }

If you prefer deterministic output, we can pass an incrementing counter or use transliteration.

enferno/static/js/components/FieldListItem.js (2)

57-58: Defensive access to theme; avoid runtime errors if $vuetify.theme is undefined

Use optional chaining to prevent crashes in non-themed contexts.

- readonly: true,
- bgColor: !this.$vuetify.theme.global.current.dark && field.core ? 'core-field-accent' : undefined
+ readonly: true,
+ bgColor: !(this.$vuetify?.theme?.global?.current?.dark) && field.core ? 'core-field-accent' : undefined

---

105-108: Dropdown option slug may become empty for non‑Latin labels; add fallback

Same concern as slugify(). Ensure option.value is never blank.

 if (!originalOption?.value) {
-  option.value = slugify(nextValue);
+  const s = slugify(nextValue);
+  option.value = s || `option_${option.id ?? index + 1}`;
 }

enferno/admin/templates/admin/bulletin-fields.html (3)

131-140: Avoid recomputing diffs on every render

hasChanges calls computeChanges() on each re-render, which scans the whole list. Cache the diff in data and recompute only when dynamicFields or edited inputs change.

 computed: {
-  hasChanges() {
-    const changes = this.computeChanges();
-    return Boolean(changes.create.length || changes.update.length || changes.delete.length);
-  },
+  hasChanges() {
+    const c = this.changes?.diff || { create: [], update: [], delete: [] };
+    return Boolean(c.create.length || c.update.length || c.delete.length);
+  },
 }

---

294-296: Update detection via JSON.stringify is brittle

JSON order and transient props can cause false positives. Use a stable, field-level deep-compare on a whitelist of properties (title, active, sort_order, ui_config, schema_config, validation_config, options).

- if (JSON.stringify(field) !== JSON.stringify(orig)) {
+ const pick = (o) => ({
+   title: o.title, active: o.active, sort_order: o.sort_order,
+   ui_config: o.ui_config, schema_config: o.schema_config,
+   validation_config: o.validation_config, options: o.options
+ });
+ if (JSON.stringify(pick(field)) !== JSON.stringify(pick(orig))) {

---

448-472: Soft delete vs. “delete” diff consistency

deleteField only flips active=false and keeps the item in dynamicFields, so computeChanges classifies it as an update, not a delete. If you want the red “Deleted” row in the review table, also remove the item from dynamicFields; otherwise, adjust labels to reflect “Disable”.

 onAccept: () => {
   const dynamicField = this.formBuilder.dynamicFields.find(d => d.id === field.id)
-  dynamicField.active = false;
+  dynamicField.active = false;
+  // Optional: to show as "delete" in the diff table
+  // this.formBuilder.dynamicFields = this.formBuilder.dynamicFields.filter(d => d.id !== field.id)

enferno/admin/templates/admin/jsapi.jinja2 (2)

537-542: Fix typo in translations

“Seperated” should be “Separated”.

-{"en": "Seperated", "tr": "{{ _('Seperated') }}"},
+{"en": "Separated", "tr": "{{ _('Separated') }}"},

---

11-486: Guard against duplicate keys in the translations object

There are repeated keys (e.g., number_, title_, tags_). While JS keeps the last one, this can hide mistakes. Consider deduping to avoid accidental overrides.

enferno/admin/views.py (4)

19-19: Remove unsafe text() import (not used after safer sort fix)

Drop text from the import list to avoid accidental use in user-influenced SQL.

-from sqlalchemy import desc, or_, asc, text, select, func
+from sqlalchemy import desc, or_, asc, select, func

---

3989-3990: Comment mismatch with value

S3_URL_EXPIRY = 3600 is 1 hour, not 2. Update the comment.

-S3_URL_EXPIRY = 3600  # 2 hours
+S3_URL_EXPIRY = 3600  # 1 hour

---

6908-6988: Prefer logger.exception over bare exception catches and generic logs

Several broad except Exception blocks log with logger.error; switch to logger.exception to keep stack traces in logs without exposing them to clients.

---

6670-6670: Remove unused variable

entity_type_filter is assigned but not used until the fix above; lint warns (F841).

enferno/static/css/app.css (2)

842-851: Add fallbacks for CSS variables to avoid no-op styles when tokens are undefined.

If any of the --v-* tokens aren’t provided by the theme at runtime, these helpers resolve to invalid and get dropped. Add sensible fallbacks.

-.overflow-unset { overflow: var(--v-overflow-unset); }
-.z-1 { z-index: var(--v-z-1); }
-.z-100 { z-index: var(--v-z-100); }
-.left-auto { left: var(--v-left-auto) !important; }
-.h-fit { height: var(--v-h-fit); }
-.pointer-events-none { pointer-events: var(--v-pointer-events-none) !important; }
-.pointer-events-auto { pointer-events: var(--v-pointer-events-auto) !important; }
-.rounded-16 { border-radius: var(--v-rounded-16) !important; }
-.rounded-12 { border-radius: var(--v-rounded-12) !important; }
-.rounded-10 { border-radius: var(--v-rounded-10) !important; }
+.overflow-unset { overflow: var(--v-overflow-unset, visible); }
+.z-1 { z-index: var(--v-z-1, 1); }
+.z-100 { z-index: var(--v-z-100, 100); }
+.left-auto { left: var(--v-left-auto, auto) !important; }
+.h-fit { height: var(--v-h-fit, fit-content); }
+.pointer-events-none { pointer-events: var(--v-pointer-events-none, none) !important; }
+.pointer-events-auto { pointer-events: var(--v-pointer-events-auto, auto) !important; }
+.rounded-16 { border-radius: var(--v-rounded-16, 16px) !important; }
+.rounded-12 { border-radius: var(--v-rounded-12, 12px) !important; }
+.rounded-10 { border-radius: var(--v-rounded-10, 10px) !important; }

---

876-915: Use theme colors for DnD indicators; hard-coded “dodgerblue” breaks theming.

Replace with rgba(var(--v-theme-primary), …) for light/dark parity. Consider 3–4px thickness for better visibility.

-    background: dodgerblue;
+    background: rgba(var(--v-theme-primary), 0.85);

enferno/admin/models/DynamicField.py (8)

341-350: Normalize entity model imports and avoid mixed import styles.

Import Actor/Bulletin/Incident in one place to reduce cycle surprises.

-    def get_entity_model(self):
-        """Get the model class for this field's entity type"""
-        from enferno.admin.models import Bulletin, Actor
-
-        models = {"bulletin": Bulletin, "actor": Actor, "incident": Incident}
+    def get_entity_model(self):
+        """Get the model class for this field's entity type"""
+        from enferno.admin.models import Bulletin, Actor, Incident
+        models = {"bulletin": Bulletin, "actor": Actor, "incident": Incident}

---

70-78: PR objective says 8 types (boolean, float, json) but model defines 6 (+ html_block).

Add missing types and mappings or adjust docs/CLI accordingly.

-    NUMBER = "number"  # Numeric input
+    NUMBER = "number"  # Numeric input
+    FLOAT = "float"
+    BOOLEAN = "boolean"
+    JSON = "json"

-        DATETIME: "timestamp with time zone",  # Date and time
+        DATETIME: "timestamp with time zone",  # Date and time
+        FLOAT: "double precision",
+        BOOLEAN: "boolean",
+        JSON: "jsonb",

-        DATETIME: DateTime(timezone=True),  # Date and time
+        DATETIME: DateTime(timezone=True),  # Date and time
+        FLOAT: Float,
+        BOOLEAN: Boolean,
+        JSON: JSONB,

Also extend COMPONENT_MAP/UI to cover BOOLEAN (e.g., "checkbox") as needed.

---

331-341: TEXT max_length validation: accept int-like strings but surface precise errors; extend to NUMBER/FLOAT ranges when provided.

Currently only TEXT max_length is validated; consider min/max for numeric types.

---

279-279: Log with stack traces on failures.

Use logger.exception to capture trace; already reflected in earlier diff.

---

352-360: get_dynamic_columns returns only active fields; confirm core=True exclusion is desired.

If core dynamic fields should also be exposed, drop the core filter here or document the behavior.

---

363-379: Return None for missing values to preserve type semantics.

Empty string for NUMBER/FLOAT/BOOLEAN/JSON may confuse API consumers; prefer None except for MULTI_SELECT (empty list).

-        if value is None:
-            if field_type == DynamicField.MULTI_SELECT:
-                return []
-            return ""
+        if value is None:
+            return [] if field_type == DynamicField.MULTI_SELECT else None

---

1-11: Remove unused imports (ForeignKey) and annotate class-level maps with ClassVar.

Keeps linters quiet and avoids accidental instrumentation.

-from sqlalchemy import (
+from sqlalchemy import (
     Column,
     String,
     Text,
     Integer,
     DateTime,
     Boolean,
     Float,
-    ARRAY as SQLArray,
-    ForeignKey,
+    ARRAY as SQLArray,
 )
+from typing import ClassVar

-    COMPONENT_MAP = {
+    COMPONENT_MAP: ClassVar[dict] = {
@@
-    TYPE_MAP = {
+    TYPE_MAP: ClassVar[dict] = {
@@
-    _column_types = {
+    _column_types: ClassVar[dict] = {

---

282-304: Rollback on create_column failure mirrors drop_column; ensure symmetrical transaction handling.

You added rollback in drop_column; create_column should rollback on failure too (covered in earlier diff). Also consider explicit commit on success.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge Base: Disabled due to data retention organization setting

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.