From b2e8fd96ec20b4b1709b89d920ad99d3093f8093 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 21 Apr 2016 14:38:02 +1200 Subject: [PATCH] BUG Fix form schema to use correct ID values API Implement creation of new changesets endpoint --- .../src/components/FormBuilder/FormBuilder.js | 43 +++++++++++++++---- .../src/components/TextField/TextField.js | 10 ++++- .../containers/CampaignAdmin/CampaignAdmin.js | 11 ++--- admin/code/CampaignAdmin.php | 37 ++-------------- admin/code/LeftAndMain.php | 35 ++++++++++++--- forms/FormSchema.php | 10 ++--- forms/ReadonlyField.php | 27 +++++++++++- model/versioning/ChangeSet.php | 10 +++-- 8 files changed, 115 insertions(+), 68 deletions(-) diff --git a/admin/client/src/components/FormBuilder/FormBuilder.js b/admin/client/src/components/FormBuilder/FormBuilder.js index b13b89c37..f418ac255 100644 --- a/admin/client/src/components/FormBuilder/FormBuilder.js +++ b/admin/client/src/components/FormBuilder/FormBuilder.js @@ -75,6 +75,30 @@ export class FormBuilderComponent extends SilverStripeComponent { this.handleFieldUpdate = this.handleFieldUpdate.bind(this); this.handleSubmit = this.handleSubmit.bind(this); this.removeForm = this.removeForm.bind(this); + this.getFormId = this.getFormId.bind(this); + this.getFormSchema = this.getFormSchema.bind(this); + } + + /** + * Get the schema for this form + * + * @returns {array} + */ + getFormSchema() { + return this.props.schemas[this.props.schemaUrl]; + } + + /** + * Gets the ID for this form + * + * @returns {string} + */ + getFormId() { + const schema = this.getFormSchema(); + if (schema) { + return schema.id; + } + return null; } componentDidMount() { @@ -184,9 +208,9 @@ export class FormBuilderComponent extends SilverStripeComponent { */ handleFieldUpdate(event, updates, fn) { if (typeof fn !== 'undefined') { - fn(this.props.formId, this.props.formsActions.updateField); + fn(this.getFormId(), this.props.formsActions.updateField); } else { - this.props.formsActions.updateField(this.props.formId, updates); + this.props.formsActions.updateField(this.getFormId(), updates); } } @@ -223,7 +247,7 @@ export class FormBuilderComponent extends SilverStripeComponent { */ handleSubmit(event) { const schemaFields = this.props.schemas[this.props.schemaUrl].schema.fields; - const fieldValues = this.props.forms[this.props.formId].fields + const fieldValues = this.props.forms[this.getFormId()].fields .reduce((prev, curr) => Object.assign({}, prev, { [schemaFields.find(schemaField => schemaField.id === curr.id).name]: curr.value, }), {}); @@ -231,7 +255,7 @@ export class FormBuilderComponent extends SilverStripeComponent { const submitFn = () => { this.props.formsActions.submitForm( this.submitApi, - this.props.formId, + this.getFormId(), fieldValues ); }; @@ -352,8 +376,12 @@ export class FormBuilderComponent extends SilverStripeComponent { } render() { - const formSchema = this.props.schemas[this.props.schemaUrl]; - const formState = this.props.forms[this.props.formId]; + const formId = this.getFormId(); + if (!formId) { + return null; + } + const formSchema = this.getFormSchema(); + const formState = this.props.forms[formId]; // If the response from fetching the initial data // hasn't come back yet, don't render anything. @@ -382,7 +410,7 @@ export class FormBuilderComponent extends SilverStripeComponent { componentWillUnmount: this.removeForm, data: formSchema.schema.data, fields: fieldData, - formId: formSchema.id, + formId, handleSubmit: this.handleSubmit, mapActionsToComponents: this.mapActionsToComponents, mapFieldsToComponents: this.mapFieldsToComponents, @@ -397,7 +425,6 @@ FormBuilderComponent.propTypes = { createFn: React.PropTypes.func, forms: React.PropTypes.object.isRequired, formsActions: React.PropTypes.object.isRequired, - formId: React.PropTypes.string.isRequired, handleSubmit: React.PropTypes.func, schemas: React.PropTypes.object.isRequired, schemaActions: React.PropTypes.object.isRequired, diff --git a/admin/client/src/components/TextField/TextField.js b/admin/client/src/components/TextField/TextField.js index 73e387e43..82a950724 100644 --- a/admin/client/src/components/TextField/TextField.js +++ b/admin/client/src/components/TextField/TextField.js @@ -14,6 +14,13 @@ class TextField extends SilverStripeComponent { ? this.props.leftTitle : this.props.title; + let field = null; + if (this.props.readOnly) { + field =
{this.props.value}
; + } else { + field = ; + } + return (
{labelText && @@ -22,7 +29,7 @@ class TextField extends SilverStripeComponent { }
- + {field}
); @@ -59,6 +66,7 @@ TextField.propTypes = { name: React.PropTypes.string.isRequired, handleFieldUpdate: React.PropTypes.func, value: React.PropTypes.string, + readOnly: React.PropTypes.bool, }; export default TextField; diff --git a/admin/client/src/containers/CampaignAdmin/CampaignAdmin.js b/admin/client/src/containers/CampaignAdmin/CampaignAdmin.js index 3d9a5e649..3fb01dc7c 100644 --- a/admin/client/src/containers/CampaignAdmin/CampaignAdmin.js +++ b/admin/client/src/containers/CampaignAdmin/CampaignAdmin.js @@ -92,7 +92,6 @@ class CampaignAdmin extends SilverStripeComponent { }; const formBuilderProps = { createFn: this.campaignListCreateFn, - formId: 'EditForm', schemaUrl, }; @@ -140,10 +139,7 @@ class CampaignAdmin extends SilverStripeComponent { */ renderDetailEditView() { const baseSchemaUrl = this.props.sectionConfig.forms.DetailEditForm.schemaUrl; - const formBuilderProps = { - formId: 'DetailEditForm', - schemaUrl: `${baseSchemaUrl}/ChangeSet/${this.props.campaignId}`, - }; + const schemaUrl = `${baseSchemaUrl}/ChangeSet/${this.props.campaignId}`; return (
@@ -153,7 +149,7 @@ class CampaignAdmin extends SilverStripeComponent {

Campaigns

- + ); @@ -163,10 +159,9 @@ class CampaignAdmin extends SilverStripeComponent { * Render the view for creating a new Campaign. */ renderCreateView() { - const baseSchemaUrl = this.props.sectionConfig.forms.CreateEditForm.schemaUrl; + const baseSchemaUrl = this.props.sectionConfig.forms.DetailEditForm.schemaUrl; const formBuilderProps = { createFn: this.campaignCreationView, - formId: 'CreateEditForm', schemaUrl: `${baseSchemaUrl}/ChangeSet`, }; diff --git a/admin/code/CampaignAdmin.php b/admin/code/CampaignAdmin.php index faadc8cfe..2ccfed3e7 100644 --- a/admin/code/CampaignAdmin.php +++ b/admin/code/CampaignAdmin.php @@ -12,12 +12,9 @@ class CampaignAdmin extends LeftAndMain implements PermissionProvider { 'set', 'sets', 'schema', - 'CreateEditForm', 'DetailEditForm', 'readCampaigns', - 'createCampaign', 'readCampaign', - 'updateCampaign', 'deleteCampaign', 'publishCampaign', ]; @@ -63,9 +60,6 @@ class CampaignAdmin extends LeftAndMain implements PermissionProvider { 'DetailEditForm' => [ 'schemaUrl' => $this->Link('schema/DetailEditForm') ], - 'CreateEditForm' => [ - 'schemaUrl' => $this->Link('schema/CreateEditForm') - ], ], 'campaignViewRoute' => $this->Link() . ':type?/:id?/:view?', 'itemListViewEndpoint' => $this->Link() . 'set/:id/show', @@ -80,10 +74,10 @@ class CampaignAdmin extends LeftAndMain implements PermissionProvider { // TODO Hardcoding schema until we can get GridField to generate a schema dynamically $json = <<getCMSFields(), - FieldList::create( - FormAction::create('save', 'Save'), - FormAction::create('cancel', 'Cancel') - ) - ); - } - /** * Gets user-visible url to edit a specific {@see ChangeSet} * @@ -518,11 +496,4 @@ JSON; ); } - /** - * - */ - public function FindReferencedChanges() { - - } - } diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 757938b9c..fa45339a4 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -233,7 +233,7 @@ class LeftAndMain extends Controller implements PermissionProvider { $formName = $request->param('FormName'); $recordType = $request->param('RecordType'); $itemID = $request->param('ItemID'); - + if (!$formName || !$recordType) { throw new SS_HTTPResponse_Exception( 'Missing request params', @@ -294,7 +294,6 @@ class LeftAndMain extends Controller implements PermissionProvider { */ protected function getSchemaForForm(Form $form) { $request = $this->getRequest(); - $schemaParts = []; $return = null; // Valid values for the "X-Formschema-Request" header are "schema" and "state". @@ -309,7 +308,7 @@ class LeftAndMain extends Controller implements PermissionProvider { $schemaParts = ['schema']; } - $return = ['id' => $form->getName()]; + $return = ['id' => $form->FormName()]; if (in_array('schema', $schemaParts)) { $return['schema'] = $this->schema->getSchema($form); @@ -1166,12 +1165,18 @@ class LeftAndMain extends Controller implements PermissionProvider { // Existing or new record? $id = $data['ID']; - if(substr($id,0,3) != 'new') { + if(is_numeric($id) && $id > 0) { $record = DataObject::get_by_id($className, $id); - if($record && !$record->canEdit()) return Security::permissionFailure($this); - if(!$record || !$record->ID) $this->httpError(404, "Bad record ID #" . (int)$id); + if($record && !$record->canEdit()) { + return Security::permissionFailure($this); + } + if(!$record || !$record->ID) { + $this->httpError(404, "Bad record ID #" . (int)$id); + } } else { - if(!singleton($this->stat('tree_class'))->canCreate()) return Security::permissionFailure($this); + if(!singleton($this->stat('tree_class'))->canCreate()) { + return Security::permissionFailure($this); + } $record = $this->getNewItem($id, false); } @@ -1195,6 +1200,22 @@ class LeftAndMain extends Controller implements PermissionProvider { return $response; } + /** + * Create new item. + * + * @param string|int $id + * @param bool $setID + * @return DataObject + */ + public function getNewItem($id, $setID = true) { + $class = $this->stat('tree_class'); + $object = Injector::inst()->create($class); + if($setID) { + $object->ID = $id; + } + return $object; + } + public function delete($data, $form) { $className = $this->stat('tree_class'); diff --git a/forms/FormSchema.php b/forms/FormSchema.php index 06c91f207..8181ac1ef 100644 --- a/forms/FormSchema.php +++ b/forms/FormSchema.php @@ -23,13 +23,13 @@ class FormSchema { */ public function getSchema(Form $form) { $request = $form->controller()->getRequest(); - $params = $request->AllParams(); $schema = [ 'name' => $form->getName(), - 'id' => isset($params['ID']) ? $params['ID'] : null, - 'action' => isset($params['Action']) ? $params['Action'] : null, - 'method' => $form->controller()->getRequest()->HttpMethod(), + 'id' => $form->FormName(), + 'action' => $form->FormAction(), + 'method' => $form->FormMethod(), + // @todo Not really reliable. Refactor into action on $this->Link('schema') 'schema_url' => $request->getUrl(), 'attributes' => $form->getAttributes(), 'data' => [], @@ -57,7 +57,7 @@ class FormSchema { */ public function getState(Form $form) { $state = [ - 'id' => $form->getName(), + 'id' => $form->FormName(), 'fields' => [], 'messages' => [] ]; diff --git a/forms/ReadonlyField.php b/forms/ReadonlyField.php index 856d1cc11..c5aab88dc 100644 --- a/forms/ReadonlyField.php +++ b/forms/ReadonlyField.php @@ -17,6 +17,8 @@ class ReadonlyField extends FormField { */ protected $includeHiddenField = false; + protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_TEXT; + /** * If true, a hidden field will be included in the HTML for the readonly field. * @@ -52,9 +54,30 @@ class ReadonlyField extends FormField { } } + /** + * If $dontEscape is true the returned value will be plain text + * and should be escaped in templates via .XML + * + * If $dontEscape is false the returned value will be safely encoded, + * but should not be escaped by the frontend. + * + * @return mixed|string + */ public function Value() { - if($this->value) return $this->dontEscape ? $this->value : Convert::raw2xml($this->value); - else return '(' . _t('FormField.NONE', 'none') . ')'; + if($this->value) { + if($this->dontEscape) { + return $this->value; + } else { + Convert::raw2xml($this->value); + } + } else { + $value = '(' . _t('FormField.NONE', 'none') . ')'; + if($this->dontEscape) { + return $value; + } else { + return ''.Convert::raw2xml($value).''; + } + } } public function getAttributes() { diff --git a/model/versioning/ChangeSet.php b/model/versioning/ChangeSet.php index a62e01203..2478a2efe 100644 --- a/model/versioning/ChangeSet.php +++ b/model/versioning/ChangeSet.php @@ -346,10 +346,12 @@ class ChangeSet extends DataObject { public function getCMSFields() { $fields = new FieldList(); - $fields->merge([ - TextField::create('Name'), - ReadonlyField::create('State') - ]); + $fields->push(TextField::create('Name')); + if($this->isInDB()) { + $state = ReadonlyField::create('State') + ->setDontEscape(true); + $fields->push($state); // Escape is done in react + } $this->extend('updateCMSFields', $fields); return $fields; }