12.0p validator.dependentFields not taken into account on update-DSRequest

[Blama]

Hi Isomorphic,

continuing this thread here (v12.0p_2019-01-05). Please see this BuiltInDS-based testcase:

BuiltInDS.java:

package com.smartgwt.sample.client;

import com.google.gwt.core.client.EntryPoint;
import com.smartgwt.client.Version;
import com.smartgwt.client.core.KeyIdentifier;
import com.smartgwt.client.data.AdvancedCriteria;
import com.smartgwt.client.data.Criterion;
import com.smartgwt.client.data.DataSource;
import com.smartgwt.client.data.SortSpecifier;
import com.smartgwt.client.types.OperatorId;
import com.smartgwt.client.types.SortDirection;
import com.smartgwt.client.util.Page;
import com.smartgwt.client.util.PageKeyHandler;
import com.smartgwt.client.util.SC;
import com.smartgwt.client.widgets.IButton;
import com.smartgwt.client.widgets.Window;
import com.smartgwt.client.widgets.events.ClickEvent;
import com.smartgwt.client.widgets.events.ClickHandler;
import com.smartgwt.client.widgets.grid.ListGrid;
import com.smartgwt.client.widgets.grid.ListGridField;
import com.smartgwt.client.widgets.layout.VLayout;

public class BuiltInDS implements EntryPoint {
    private VLayout mainLayout;
    private IButton recreateBtn;

    public void onModuleLoad() {
        KeyIdentifier debugKey = new KeyIdentifier();
        debugKey.setCtrlKey(true);
        debugKey.setKeyName("D");

        Page.registerKey(debugKey, new PageKeyHandler() {
            public void execute(String keyName) {
                SC.showConsole();
            }
        });

        mainLayout = new VLayout(20);
        mainLayout.setWidth100();
        mainLayout.setHeight100();

        recreateBtn = new IButton("Recreate");
        recreateBtn.addClickHandler(new ClickHandler() {
            @Override
            public void onClick(ClickEvent event) {
                recreate();
            }
        });
        mainLayout.addMember(recreateBtn);
        recreate();
        mainLayout.draw();
    }

    private void recreate() {
        Window w = new Window();
        w.setWidth("95%");
        w.setHeight("95%");
        w.setMembersMargin(0);
        w.setModalMaskOpacity(70);
        w.setTitle(" (" + Version.getVersion() + "/" + Version.getSCVersionNumber() + ")");
        w.setTitle("TITLE" + w.getTitle());
        w.setShowMinimizeButton(false);
        w.setIsModal(true);
        w.setShowModalMask(true);
        w.centerInPage();

        final ListGrid supplyItemGrid = new ListGrid(DataSource.get("supplyItem"));
        supplyItemGrid.setHeight100();
        supplyItemGrid.setWidth100();
        supplyItemGrid.setCanEdit(true);
        supplyItemGrid.setAutoFetchData(false);

        ListGridField itemID = new ListGridField("itemID");
        ListGridField itemName = new ListGridField("itemName");
        ListGridField sku = new ListGridField("SKU");
        ListGridField category = new ListGridField("category");
        ListGridField units = new ListGridField("units");
        ListGridField unitCost = new ListGridField("unitCost");
        ListGridField inStock = new ListGridField("inStock");
        ListGridField nextShipment = new ListGridField("nextShipment");

        supplyItemGrid.setFields(itemID, itemName, sku, category, units, unitCost, inStock, nextShipment);
        supplyItemGrid.setSort(new SortSpecifier[] { new SortSpecifier(itemName.getName(), SortDirection.ASCENDING) });
        supplyItemGrid.fetchData(new AdvancedCriteria(new Criterion(itemName.getName(), OperatorId.STARTS_WITH, "A")));
        w.addItem(supplyItemGrid);
        w.show();
    }
}

TestValidator.java:

package com.smartgwt.sample.server.listener;

import java.util.Map;

import javax.servlet.http.HttpServletRequest;

import com.isomorphic.datasource.DSResponse;
import com.isomorphic.datasource.DataSource;
import com.isomorphic.datasource.Validator;
import com.isomorphic.log.Logger;

public class TestValidator {
    Logger log = new Logger(DSResponse.class.getName());

    public boolean condition(Object value, Validator validator, String fieldName, @SuppressWarnings("rawtypes") Map record, DataSource ds,
            HttpServletRequest httpServletRequest) throws Exception {
        log.info("TestValidator called");
        String valueName = (String) record.get("itemName");
        if (!valueName.equals("Account Book 168 Page Hardcover A4 4MC")) {
            validator.addErrorMessageVariable("errorMessage", "Error");
            return false;
        } else
            return true;
    }
}

supplyItem.ds.xml:

<DataSource ID="supplyItem" serverType="sql" tableName="supplyItem" titleField="itemName"
    testFileName="/examples/shared/ds/test_data/supplyItem.data.xml" dbImportFileName="/examples/shared/ds/test_data/supplyItemLarge.data.xml">
    <fields>
        <field name="itemID" type="sequence" hidden="true" primaryKey="true" />
        <field name="itemName" type="text" title="Item" length="128" required="true">
            <validators>
                <validator type="serverCustom" validateOnChange="true">
                    <serverObject lookupStyle="new" className="com.smartgwt.sample.server.listener.TestValidator" />
                    <errorMessage>$errorMessage</errorMessage>
                    <dependentFields>
                        <dependentField>inStock</dependentField>
                        <dependentField>SKU</dependentField>
                    </dependentFields>
                </validator>
            </validators>
        </field>
        <field name="SKU" type="text" title="SKU" length="10" required="true" />
        <field name="description" type="text" title="Description" length="2000" />
        <field name="category" type="text" title="Category" length="128" required="true" foreignKey="supplyCategory.categoryName" />
        <field name="units" type="enum" title="Units" length="5">
            <valueMap>
                <value>Roll</value>
                <value>Ea</value>
                <value>Pkt</value>
                <value>Set</value>
                <value>Tube</value>
                <value>Pad</value>
                <value>Ream</value>
                <value>Tin</value>
                <value>Bag</value>
                <value>Ctn</value>
                <value>Box</value>
            </valueMap>
        </field>
        <field name="unitCost" type="float" title="Unit Cost" required="true">
            <validators>
                <validator type="floatRange" min="0" errorMessage="Please enter a valid (positive) cost" />
                <validator type="floatPrecision" precision="2" errorMessage="The maximum allowed precision is 2" />
            </validators>
        </field>
        <field name="inStock" type="boolean" title="In Stock" />
        <field name="nextShipment" type="date" title="Next Shipment" />
    </fields>
</DataSource>

Flow to reproduce:

1. Double click a row to enter edit mode
2. Change "In Stock" or "SKU" to see validation requests issued as expected
3. See that the requests return an error (expected)
4. This error is not shown in the GUI (issue one, might be on purpose (?))
5. Exit row to issue update (possible, as the error is not shown in the GUI, see above)
6. Update issued successfully and validator server code not hit (issue two, validator should be hit)

Easier flow to trigger issue two:

1. "In Stock" toggle without entering ListGrid edit mode also does not trigger the server side validator

Best regards
Blama

[Isomorphic]

We've addressed your "issue 2" back to SGWT 6.1p/SC 11.1p so that the expected errors are reported back from the server and shown on the client. In this case the server-side validator was not run on the update since the field wasn't modified. We agree that the dependentField docs make it sound as though server-side validators would be run if a dependent field was changed, even if the field where the validator is installed wasn't changed, so we've modified the behavior to do just that.

The fix should be in the nightly builds dated 2019-01-10 and beyond.

[Blama]

Hi Isomorphic,

thank you.
Actually you are making it sound like this was not the intended effect of the attribute before.
I wrote in #1 here that it is unclear to me what the attribute actually does. So if this wasn't the intended effect before (I got that it is now as well), what was/is it?

W.r.t. to "issue 1": Do you treat this as an issue? Is it an issue?
I think with "issue 2" fixed I don't really need validateOnChange="true", so this might be fine for me.

Best regards
Blama

[Isomorphic]

Intended behavior was essentially as doc'd for client-side behavior, but there was a regression that caused the errors to be calculated but not displayed in certain cases with validateOnChange settings, which has been fixed.

As far as server-side behavior, at the time the feature was implemented, there was essentially no server-side behavior needed because all fields used to be passed to the validation system, so the docs didn't mention anything about a server-side behavior. However, validators are now run more efficiently - just for changed fields - and reading the docs, we can see that it would be reasonable to expect that validators are triggered if there is a change in a dependent field even if there is no change in the field where the validator is declared, and we don't want anyone writing a validator with this assumption and ending up with a security issue, so we made it behave that way.

[Blama]

Hi Isomorphic,

thank you for the explanation, I think I understand now.

Best regards
Blama