Hi all,
While debugging why a demo ProductStore (allocateInventory) unexpectedly
flipped from unset to Y after an unrelated Product Store information update
through the EditProductStore form, I traced it to what looks like a general
gap in how the form widget auto generates fields for indicator-type entity
columns, and wanted to get the community's take before filing a JIRA /
patch.
Here is the example:
ProductStore.allocateInventory is an indicator field with no default-value
and is never set by demo ProductStore data, so the column is genuinely NULL
out of the box. EditProductStore form doesn't declare an explicit field for
it, it only picks it up via <auto-fields-service
service-name="updateProductStore"/>.
ModelFormFieldBuilder.induceFieldInfoFromEntityField() builds the
auto-generated control for an indicator field differently depending on
context:
// "find" form (line ~762) includes a blank option:
List<OptionSource> optionSources = UtilMisc.toList(
new ModelFormField.SingleOption("", null, null),
new ModelFormField.SingleOption("Y", null, null),
new ModelFormField.SingleOption("N", null, null));
// create/update form (line ~795) no blank option:
List<OptionSource> optionSources = UtilMisc.toList(
new ModelFormField.SingleOption("Y", null, null),
new ModelFormField.SingleOption("N", null, null));
For a null current value, the browser's <select> has nothing to match, so
it defaults to highlighting the first <option> - Y. Saving the form for any
reason (unrelated field edit) silently writes allocateInventory="Y" to the
DB. In this store's case, that quietly disables inventory reservation for
every sales order item unless it carries an autoReserve=true attribute
(OrderServices.reserveInventory()), Not fun to debug back to a form save
from days earlier.
Two ways to fix it:
1. Per-form fix: give the specific field an explicit <field> definition
with allow-empty="true" (and no no-current-selected-key), same pattern
already used by neighboring fields like requireInventory/reserveInventory
when a form author wants a forced default. This fixes allocateInventory on
this one form, but any other entity's auto-generated indicator field with
no explicit override anywhere in OFBiz has the same latent bug.
2. Framework fix: add the blank SingleOption to the create/update form too,
mirroring what the find form already does. This would auto-correct
allocateInventory (and any other field in the same situation) with no form
changes needed at all, for every entity, present and future. I'd propose
gating it on ModelField.getIsNotNull() so a genuinely NOT NULL column
without an explicit override still forces a real default rather than
letting a blank submission hit a DB constraint at save time.
I am looking for feedback on this from the community around this:
Is there a known reason the of form rendering in different ways find vs
create/update forms?
Any forms/screens people are aware of that might (even accidentally) rely
on the current "defaults to first option" behavior for a null indicator
value? I haven't been able to audit the whole codebase for that.
Preference between fixing this narrowly per-field (safe, but doesn't
prevent recurrence elsewhere) vs. at the framework level (fixes the whole
class, but touches shared widget-rendering code used everywhere).
Happy to file a JIRA and put together a patch for whichever direction the
community leans toward - the per-form fix for
allocateInventory/StoreForms.xml is already trivial and I can submit that
regardless, but wanted to raise the framework question first to get some
feedback.
Best regards,
Pranay Pandey
