Skip to content

Commit

Permalink
API Move dependency on model class from form schema API
Browse files Browse the repository at this point in the history
API Refactor hard-coded dataobject class references from CampaignAdmin
Fixes #5730
  • Loading branch information
Damian Mooyman committed Jun 22, 2016
1 parent 27ce713 commit 80e5b91
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 43 deletions.
6 changes: 3 additions & 3 deletions admin/client/dist/js/bundle-framework.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion admin/client/dist/js/bundle-lib.js

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion admin/client/src/components/GridField/GridField.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ class GridField extends SilverStripeComponent {

return (
<GridFieldRow {...rowProps}>
{cells.concat(rowActions)}
{cells}
{rowActions}
</GridFieldRow>
);
}
Expand Down
5 changes: 2 additions & 3 deletions admin/client/src/containers/CampaignAdmin/CampaignAdmin.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class CampaignAdmin extends SilverStripeComponent {
const baseSchemaUrl = this.props.sectionConfig.form.DetailEditForm.schemaUrl;
const formBuilderProps = {
createFn: this.campaignEditCreateFn,
schemaUrl: `${baseSchemaUrl}/ChangeSet/${this.props.campaignId}`,
schemaUrl: `${baseSchemaUrl}/${this.props.campaignId}`,
};

return (
Expand All @@ -191,10 +191,9 @@ class CampaignAdmin extends SilverStripeComponent {
* Render the view for creating a new Campaign.
*/
renderCreateView() {
const baseSchemaUrl = this.props.sectionConfig.form.DetailEditForm.schemaUrl;
const formBuilderProps = {
createFn: this.campaignAddCreateFn,
schemaUrl: `${baseSchemaUrl}/ChangeSet`,
schemaUrl: this.props.sectionConfig.form.DetailEditForm.schemaUrl,
};

return (
Expand Down
18 changes: 13 additions & 5 deletions admin/client/src/containers/CampaignAdmin/CampaignAdminList.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ class CampaignAdminList extends SilverStripeComponent {

// Only load record if not already present
if (!Object.keys(this.props.record).length) {
this.props.recordActions.fetchRecord('ChangeSet', 'get', fetchURL).then(this.setBreadcrumbs);
this.props.recordActions
.fetchRecord(this.props.treeClass, 'get', fetchURL)
.then(this.setBreadcrumbs);
}
}

Expand Down Expand Up @@ -152,14 +154,17 @@ class CampaignAdminList extends SilverStripeComponent {
the <a href={pagesLink}>edit page screen</a>.
</div>
);
const bodyClass = [
'container-fluid', 'campaign-items', 'panel-scrollable', 'panel-scrollable--double-toolbar',
];

return (
<div className="cms-content__split cms-content__split--left-sm">
<div className="cms-content__left cms-campaigns collapse in" aria-expanded="true">
<Toolbar showBackButton handleBackButtonClick={this.props.handleBackButtonClick}>
<BreadcrumbComponent multiline crumbs={this.props.breadcrumbs} />
</Toolbar>
<div className="container-fluid campaign-items panel-scrollable panel-scrollable--double-toolbar">
<div className={bodyClass.join(' ')}>
{body}
</div>
<div className="toolbar--south">
Expand Down Expand Up @@ -236,7 +241,7 @@ class CampaignAdminList extends SilverStripeComponent {
*/
getItems() {
if (this.props.record && this.props.record._embedded) {
return this.props.record._embedded.ChangeSetItems;
return this.props.record._embedded.items;
}

return null;
Expand Down Expand Up @@ -278,6 +283,7 @@ class CampaignAdminList extends SilverStripeComponent {
e.preventDefault();
this.props.campaignActions.publishCampaign(
this.props.publishApi,
this.props.treeClass,
this.props.campaignId
);
}
Expand All @@ -301,13 +307,15 @@ CampaignAdminList.propTypes = {
function mapStateToProps(state, ownProps) {
// Find record specific to this item
let record = null;
if (state.records && state.records.ChangeSet && ownProps.campaignId) {
record = state.records.ChangeSet[parseInt(ownProps.campaignId, 10)];
const treeClass = ownProps.sectionConfig.treeClass;
if (state.records && state.records[treeClass] && ownProps.campaignId) {
record = state.records[treeClass][parseInt(ownProps.campaignId, 10)];
}
return {
config: state.config,
record: record || {},
campaign: state.campaign,
treeClass,
breadcrumbs: state.breadcrumbs,
};
}
Expand Down
5 changes: 3 additions & 2 deletions admin/client/src/state/campaign/CampaignActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ export function showCampaignView(campaignId, view) {
* Publish a campaign and all its items
*
* @param {Function} publishApi See lib/Backend
* @param {string} recordType
* @param {number} campaignId
* @return {Object}
*/
export function publishCampaign(publishApi, campaignId) {
export function publishCampaign(publishApi, recordType, campaignId) {
return (dispatch) => {
dispatch({
type: ACTION_TYPES.PUBLISH_CAMPAIGN_REQUEST,
Expand All @@ -53,7 +54,7 @@ export function publishCampaign(publishApi, campaignId) {
});
dispatch({
type: RECORD_ACTION_TYPES.FETCH_RECORD_SUCCESS,
payload: { recordType: 'ChangeSet', data },
payload: { recordType, data },
});
})
.catch((error) => {
Expand Down
9 changes: 7 additions & 2 deletions admin/client/src/state/records/RecordsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ function recordsReducer(state = initialState, action) {

case ACTION_TYPES.FETCH_RECORDS_SUCCESS:
recordType = action.payload.recordType;
// TODO Automatic pluralisation from recordType
records = action.payload.data._embedded[`${recordType}s`] || {};
if (!recordType) {
throw new Error('Undefined record type');
}
records = action.payload.data._embedded[recordType] || {};
records = records.reduce((prev, val) => Object.assign({}, prev, { [val.ID]: val }), {});
return deepFreeze(Object.assign({}, state, {
[recordType]: records,
Expand All @@ -44,6 +46,9 @@ function recordsReducer(state = initialState, action) {
recordType = action.payload.recordType;
record = action.payload.data;

if (!recordType) {
throw new Error('Undefined record type');
}
return deepFreeze(Object.assign({}, state, {
[recordType]: Object.assign({}, state[recordType], { [record.ID]: record }),
}));
Expand Down
36 changes: 24 additions & 12 deletions admin/code/CampaignAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ public function getClientConfig() {
'publishEndpoint' => [
'url' => $this->Link() . 'set/:id/publish',
'method' => 'post'
]
],
'treeClass' => $this->config()->tree_class
]);
}

public function schema($request) {
// TODO Hardcoding schema until we can get GridField to generate a schema dynamically
$treeClassJS = Convert::raw2js($this->config()->tree_class);
$json = <<<JSON
{
"id": "Form_EditForm",
Expand Down Expand Up @@ -124,7 +126,7 @@ public function schema($request) {
"customValidationMessage": "",
"attributes": [],
"data": {
"recordType": "ChangeSet",
"recordType": "{$treeClassJS}",
"collectionReadEndpoint": {
"url": "admin\/campaigns\/sets",
"method": "GET"
Expand All @@ -148,7 +150,7 @@ public function schema($request) {
"editFormSchemaEndpoint": "admin\/campaigns\/schema\/DetailEditForm",
"columns": [
{"name": "Title", "field": "Name"},
{"name": "Changes", "field": "_embedded.ChangeSetItems.length"},
{"name": "Changes", "field": "ChangesCount"},
{"name": "Description", "field": "Description"}
]
}
Expand Down Expand Up @@ -189,11 +191,9 @@ public function schema($request) {
/**
* REST endpoint to get a list of campaigns.
*
* @param SS_HTTPRequest $request
*
* @return SS_HTTPResponse
*/
public function readCampaigns(SS_HTTPRequest $request) {
public function readCampaigns() {
$response = new SS_HTTPResponse();
$response->addHeader('Content-Type', 'application/json');
$hal = $this->getListResource();
Expand All @@ -209,6 +209,8 @@ public function readCampaigns(SS_HTTPRequest $request) {
protected function getListResource() {
$items = $this->getListItems();
$count = $items->count();
/** @var string $treeClass */
$treeClass = $this->config()->tree_class;
$hal = [
'count' => $count,
'total' => $count,
Expand All @@ -217,12 +219,12 @@ protected function getListResource() {
'href' => $this->Link('items')
]
],
'_embedded' => ['ChangeSets' => []]
'_embedded' => [$treeClass => []]
];
foreach($items as $item) {
/** @var ChangeSet $item */
$resource = $this->getChangeSetResource($item);
$hal['_embedded']['ChangeSets'][] = $resource;
$hal['_embedded'][$treeClass][] = $resource;
}
return $hal;
}
Expand Down Expand Up @@ -251,7 +253,7 @@ protected function getChangeSetResource(ChangeSet $changeSet) {
'State' => $changeSet->State,
'canEdit' => $changeSet->canEdit(),
'canPublish' => $changeSet->canPublish(),
'_embedded' => ['ChangeSetItems' => []]
'_embedded' => ['items' => []]
];
foreach($changeSet->Changes() as $changeSetItem) {
if(!$changeSetItem) {
Expand All @@ -260,8 +262,9 @@ protected function getChangeSetResource(ChangeSet $changeSet) {

/** @var ChangesetItem $changeSetItem */
$resource = $this->getChangeSetItemResource($changeSetItem);
$hal['_embedded']['ChangeSetItems'][] = $resource;
$hal['_embedded']['items'][] = $resource;
}
$hal['ChangesCount'] = count($hal['_embedded']['items']);
return $hal;
}

Expand Down Expand Up @@ -336,6 +339,7 @@ protected function getListItems() {
return ChangeSet::get()
->filter('State', ChangeSet::STATE_OPEN)
->filterByCallback(function($item) {
/** @var ChangeSet $item */
return ($item->canView());
});
}
Expand All @@ -357,7 +361,8 @@ public function readCampaign(SS_HTTPRequest $request) {
return (new SS_HTTPResponse(null, 400));
}

$changeSet = ChangeSet::get()->byId($request->param('ID'));
/** @var ChangeSet $changeSet */
$changeSet = ChangeSet::get()->byID($request->param('ID'));
if(!$changeSet) {
return (new SS_HTTPResponse(null, 404));
}
Expand Down Expand Up @@ -424,6 +429,7 @@ public function publishCampaign(SS_HTTPRequest $request) {
return (new SS_HTTPResponse(null, 400));
}

/** @var ChangeSet $record */
$record = ChangeSet::get()->byID($id);
if(!$record) {
return (new SS_HTTPResponse(null, 404));
Expand Down Expand Up @@ -468,7 +474,7 @@ public function getDetailEditForm($id) {
// Get record-specific fields
$record = null;
if($id) {
$record = ChangeSet::get()->byId($id);
$record = ChangeSet::get()->byID($id);
if(!$record || !$record->canView()) {
return null;
}
Expand All @@ -491,6 +497,12 @@ public function getDetailEditForm($id) {
FormAction::create('cancel', _t('LeftAndMain.CANCEL', 'Cancel'))
)
);

// Load into form
if($id && $record) {
$form->loadDataFrom($record);
}

// Configure form to respond to validation errors with form schema
// if requested via react.
$form->setValidationResponseCallback(function() use ($form) {
Expand Down
17 changes: 3 additions & 14 deletions admin/code/LeftAndMain.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class LeftAndMain extends Controller implements PermissionProvider {
];

private static $url_handlers = [
'GET schema/$FormName/$RecordType/$ItemID' => 'schema'
'GET schema/$FormName/$ItemID' => 'schema'
];

private static $dependencies = [
Expand Down Expand Up @@ -226,15 +226,15 @@ public function getClientConfig() {
*
* WARNING: Experimental API.
*
* @param SS_HTTPRequest $request
* @return SS_HTTPResponse
*/
public function schema($request) {
$response = $this->getResponse();
$formName = $request->param('FormName');
$recordType = $request->param('RecordType');
$itemID = $request->param('ItemID');

if (!$formName || !$recordType) {
if (!$formName) {
return (new SS_HTTPResponse('Missing request params', 400));
}

Expand All @@ -248,17 +248,6 @@ public function schema($request) {

$form = $this->{"get{$formName}"}($itemID);

if($itemID) {
$record = $recordType::get()->byId($itemID);
if(!$record) {
return (new SS_HTTPResponse('Record not found', 404));
}
if(!$record->canView()) {
return (new SS_HTTPResponse('Record not accessible', 403));
}
$form->loadDataFrom($record);
}

$response->addHeader('Content-Type', 'application/json');
$response->setBody(Convert::raw2json($this->getSchemaForForm($form)));

Expand Down

0 comments on commit 80e5b91

Please sign in to comment.