API Make ElementalAreaController a subclass of FormSchemaController #1273
Conversation
| class ElementalAreaController extends CMSMain | ||
| class ElementalAreaController extends FormSchemaController |
There was a problem hiding this comment.
This was originally changed from subclassing LeftAndMain to CMSMain in #449
The reasoning was
It makes more sense for ElementalAreaController to inherit from CMSMain rather than LeftAndMain, as this means that permissions are checked in relation to the Page Level rather than for the entire CMS.
Maybe that made sense at the time, but you can have an elemental area on any DataObject including inside a ModelAdmin.... so requiring CMS_ACCESS_CMSMain permissions is just wrong. Instead, require CMS access more generally, and let the model permissions dictate what can and can't be done within the controller.
GuySartorelli
force-pushed
the
pulls/6/formschema
branch
from
fb79ecb to
7be2612
Compare
GuySartorelli
force-pushed
the
pulls/6/formschema
branch
from
7be2612 to
c7d8f88
Compare
| // Will end up at an HTML page with "Silverstripe - Bad Request" | ||
| $this->assertSame('text/html; charset=utf-8', $response->getHeader('Content-type')); | ||
| $this->assertStringContainsString('Silverstripe - Bad Request', $response->getBody()); | ||
| // Gives suitable error message for XHR request | ||
| $this->assertStringStartsWith('text/plain', $response->getHeader('Content-type')); | ||
| $this->assertStringContainsString('There seems to have been a technical problem', $response->getBody()); |
There was a problem hiding this comment.
The modal links used to be routed through LeftAndMain which meant the error handling in that class was used. For some reason they weren't picked up as XHR requests so the full HTML error page was being rendered which wasn't really the correct behaviour, but it's what happened.
Now, a simple 400 response (the default one produced when a form's CSRF token is missing) is produced and isn't altered. This is the correct behaviour.
The end result for the user is no change.
Needs silverstripe/silverstripe-admin#1850 for CI to go green. Please merge that first.
Issue
LeftAndMaininto its own abstract class silverstripe-admin#1762