BUG Fix sorting on main ReportAdmin grid #1017

Merged
merged 1 commit into from Aug 12, 2014

Conversation

Projects
None yet
5 participants
@tractorcow
Contributor

tractorcow commented May 15, 2014

Currently trying to sort the reportadmin grid results in a JS error. This is due to the controller routing not properly supporting multiple routes. This fix mocks an 'index' sub-action to expose the main form via ajax.

ref: CWPBUG-133

Depends on silverstripe/silverstripe-framework#3139 and silverstripe/silverstripe-framework#3138

@tractorcow tractorcow added the 3.1 label May 15, 2014

- $this->reportClass = (isset($this->urlParams['ReportClass'])) ? $this->urlParams['ReportClass'] : null;
+ $this->reportClass = (isset($this->urlParams['ReportClass']) && $this->urlParams['ReportClass'] !== 'index')
+ ? $this->urlParams['ReportClass']
+ : null;

This comment has been minimized.

Show comment Hide comment
@stojg

stojg Jul 28, 2014

Contributor

I'm not really sure if I approve the use of multi line ternaries, however I can't quote http://doc.silverstripe.org/framework/en/trunk/misc/coding-conventions since there is nothing about ternaries in there.

@stojg

stojg Jul 28, 2014

Contributor

I'm not really sure if I approve the use of multi line ternaries, however I can't quote http://doc.silverstripe.org/framework/en/trunk/misc/coding-conventions since there is nothing about ternaries in there.

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Jul 28, 2014

Contributor

I love multi line ternaries. It makes reading much easier to see it broken down line by line: condition, true result, false result. :)

@tractorcow

tractorcow Jul 28, 2014

Contributor

I love multi line ternaries. It makes reading much easier to see it broken down line by line: condition, true result, false result. :)

This comment has been minimized.

Show comment Hide comment
@halkyon

halkyon Jul 29, 2014

Owner

I'd prefer multi-line ternaries in this case, making it an if/else wouldn't make it anymore readable IMO.

@halkyon

halkyon Jul 29, 2014

Owner

I'd prefer multi-line ternaries in this case, making it an if/else wouldn't make it anymore readable IMO.

@stojg stojg referenced this pull request in silverstripe/silverstripe-framework Jul 28, 2014

Closed

Remove SplitView preview option if the device cannot use it. #3338

halkyon added a commit that referenced this pull request Aug 12, 2014

@halkyon halkyon merged commit 796874d into silverstripe:3.1 Aug 12, 2014

@stojg

This comment has been minimized.

Show comment Hide comment
@stojg

stojg Aug 12, 2014

Contributor

Ref: CWPBUG-133 and CWPBUG-126

Contributor

stojg commented Aug 12, 2014

Ref: CWPBUG-133 and CWPBUG-126

@tractorcow tractorcow deleted the tractorcow:pulls/3.1/fix-report-sorting branch Aug 12, 2014

@wilr

This comment has been minimized.

Show comment Hide comment
@wilr

wilr Aug 13, 2014

Owner

Probably should also be cherry-picked to the reports module for 3.2.

Owner

wilr commented Aug 13, 2014

Probably should also be cherry-picked to the reports module for 3.2.

@tractorcow

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Aug 13, 2014

Contributor

I didn't realise you could cherry pick across different repositories. neat. :)

Contributor

tractorcow commented Aug 13, 2014

I didn't realise you could cherry pick across different repositories. neat. :)

@Matthew-Bonner

This comment has been minimized.

Show comment Hide comment
@Matthew-Bonner

Matthew-Bonner Oct 8, 2014

git does some amazing things

git does some amazing things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment