DynamicForm: Bug with save after clearValues()

[Blama]

Hi Isomorphic,

Please the the following issue I stumbled into when creating a testcase for another issue.

Case 1:

1. Hit Reload
2. Enter Data
3. Hit Save

Case 2:

1. Hit Reload
2. Hit Load Button
3. Hit Clear Button
4. Enter Data
5. Hit Save

BuiltInDS.java

package com.smartgwt.sample.client;

import com.google.gwt.core.client.EntryPoint;
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.types.OperatorId;
import com.smartgwt.client.util.PageKeyHandler;
import com.smartgwt.client.util.Page;
import com.smartgwt.client.util.SC;
import com.smartgwt.client.widgets.IButton;
import com.smartgwt.client.widgets.events.ClickEvent;
import com.smartgwt.client.widgets.events.ClickHandler;
import com.smartgwt.client.widgets.form.DynamicForm;
import com.smartgwt.client.widgets.layout.HLayout;
import com.smartgwt.client.widgets.layout.VLayout;

public class BuiltInDS implements EntryPoint {
	private VLayout vL;
	private TestForm tF;
	private HLayout hL;

	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();
			}
		});

		vL = new VLayout(5);
		vL.setTop(20);
		vL.setLeft(20);
		vL.setWidth100();
		vL.setHeight100();
		tF = new TestForm();

		hL = new HLayout(5);

		IButton load1 = new IButton("Load Charles Madigen");
		load1.setWidth(200);
		load1.addClickHandler(new ClickHandler() {
			@Override
			public void onClick(ClickEvent event) {
				tF.fetchData(new AdvancedCriteria(new Criterion("EmployeeId", OperatorId.EQUALS, 4)));
			}
		});

		IButton load2 = new IButton("Load Ralph Brogan");
		load2.setWidth(200);
		load2.addClickHandler(new ClickHandler() {
			@Override
			public void onClick(ClickEvent event) {
				tF.fetchData(new AdvancedCriteria(new Criterion("EmployeeId", OperatorId.EQUALS, 192)));
			}
		});

		IButton clear = new IButton("Clear");
		clear.setWidth(200);
		clear.addClickHandler(new ClickHandler() {
			@Override
			public void onClick(ClickEvent event) {
				tF.clearValues();
			}
		});

		IButton save = new IButton("Save");
		save.setWidth(200);
		save.addClickHandler(new ClickHandler() {
			@Override
			public void onClick(ClickEvent event) {
				tF.saveData();
			}
		});

		IButton reload = new IButton("Reload");
		reload.setWidth(200);
		reload.addClickHandler(new ClickHandler() {
			@Override
			public void onClick(ClickEvent event) {
				vL.removeChild(tF);
				vL.markForRedraw();
				tF.markForDestroy();
				tF = new TestForm();
				vL.addMember(tF, 0);
			}
		});

		hL.addMembers(load1, load2, clear, save, reload);
		vL.addMembers(tF, hL);
		vL.draw();
	}

	private class TestForm extends DynamicForm {
		public TestForm() {
			super();
			setDataSource(DataSource.get("employees"));
			setAddOperation("myAdd1");
			setUpdateOperation("myUpdate1");
			setAutoFetchData(false);
		}
	}
}

employees.ds.xml (Main change: pkField hidden="true")

<DataSource ID="employees" serverType="sql" tableName="employeeTable" recordName="employee"
	testFileName="/examples/shared/ds/test_data/employees.data.xml" titleField="Name">
	<fields>
		<field name="userOrder" title="userOrder" type="integer" canEdit="false" hidden="true" />
		<field name="Name" title="Name" type="text" length="128" />
		<field name="EmployeeId" title="Employee ID" type="integer" primaryKey="true" required="true" hidden="true" />
		<field name="ReportsTo" title="Manager" type="integer" required="true" foreignKey="employees.EmployeeId" rootValue="1" detail="true" />
		<field name="Job" title="Title" type="text" length="128" />
		<field name="Email" title="Email" type="text" length="128" />
		<field name="EmployeeType" title="Employee Type" type="text" length="40" />
		<field name="EmployeeStatus" title="Status" type="text" length="40" />
		<field name="Salary" title="Salary" type="float" />
		<field name="OrgUnit" title="Org Unit" type="text" length="128" />
		<field name="Gender" title="Gender" type="text" length="7">
			<valueMap>
				<value>male</value>
				<value>female</value>
			</valueMap>
		</field>
		<field name="MaritalStatus" title="Marital Status" type="text" length="10">
			<valueMap>
				<value>married</value>
				<value>single</value>
			</valueMap>
		</field>
	</fields>

	<operationBindings>
		<operationBinding operationType="add" operationId="myAdd1" cacheSyncOperation="myFetch1" />
		<operationBinding operationType="fetch" operationId="myFetch1" outputs="EmployeeId, Name, EmployeeType">
			<criteria fieldName="EmployeeId" operator="notNull" />
		</operationBinding>
		<operationBinding operationType="update" operationId="myUpdate1" cacheSyncOperation="myFetch1" />
	</operationBindings>
</DataSource>

As you will notice the error messages are different and I think the STATUS=VALIDATION_ERROR message is correct, while the STATUS=-4 is not.
After clearValues() I'd expect the DynamicForm to behave like if it were freshly loaded.

I'm using v10.0p_2015-08-24/PowerEdition.

Best regards
Blama

[Isomorphic]

Test cases are great, but we do need at least a short prose description of what you think the bug is, otherwise, we might look at your test case and not see anything wrong (if we disagree about correct behavior).

Also, status=-4 is the same as validation error.

[Blama]

Hi Isomorphic,

see the screenshots "Case 1", "Case 2" for the respective tests (now with SNAPSHOT_v10.1d_2015-10-13/PowerEdition).

I'm thinking now that the error in case 2 comes from the missing PK for update. I'd also think that the DynamicForm would/should be in setSaveOperationType(DSOperationType.ADD) after clearValues(), but perhaps I'm wrong here.
W.r.t. to the status "-4": I meant "-9" here.

Sorry for taking so much time here to answer. I did not keep track of all the reports I sent. I'll give feedback now on the open treads in the next days one after another.

Best regards
Blama

[Isomorphic]

Yes - it looks like the problem here is the operation type - add vs update. Editing an existing record will set the form's saveOperationType to "update" (appropriately). clearValues() doesn't set it to "add".
Perhaps this should happen automatically - we'll consider whether there's any good reason not to have this happen - but regardless you can easily fix this by setting the saveOperationType explicitly after you clear values.

Regards
Isomorphic Software

[Blama]

Hi Isomorphic,

that is exactly what I meant. It is possible to do it manual and I do that currently, but an automatic reset would be nice if there isn't a good reason against this.

Best regards
Blama

[Isomorphic]

On reflection we don't plan to do this. We already have an explicit 'editNewRecord()' API which gives you this behavior explicitly and our other values manipulation code doesn't attempt to guess whether your working with a new or existing record (and this would be tough to achieve - for example 'setValues()' would be ambiguous - we could check for a 'primaryKey' field value or something and use that to guess whether this was an 'add' rather than an 'update' but it's not a route which makes much sense to go down as a framework feature, given that we have explicit APIs in place already)

Regards
Isomorphic Software

[Blama]

OK, thanks for considering and the explanation.

Best regards
Blama