Posted by msonnabaum on October 23, 2012 at 1:11am
7 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | views_ui.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Needs tests, VDC |
Issue Summary
Attached patch fixes a couple bugs.
The views UI uses Database, but doesn't "use" it. This bug isn't ever found because we're checking for an instance of views_plugin_query_default, which is no longer the right class name. Both of these result in the query args not being replaced in the preview.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| views_ui_db_class_bug.patch | 963 bytes | Idle | PASSED: [[SimpleTest]]: [MySQL] 46,042 pass(es). | View details |
Comments
#1
#2
#3
I have a feeling this needs tests...
#4
We need tests for so many things.
#5
The last submitted patch, core-1820332-4-test.patch, failed testing.
#6
Proven that there is a bug.
#7
Looks pretty good.
I will create a follow up issue for testing all views settings in the UI match the set variables.
+++ b/core/modules/views/lib/Drupal/views/Tests/UI/SettingsTest.phpundefined@@ -91,6 +91,29 @@ function testEditUI() {
+ $this->assertFalse(strpos($xpath[0], 'db_condition_placeholder_0') !== FALSE, 'The placeholders in the views sql are not shown directly..');
We should just test for all placeholders here, not just the first?
Also, should this live in it's own method, like testUIPreview or something?
#8
After looking the the testEditUI method, this seems to fit in there, as it tests most other parts of the edit form too.
#9
Later we should enable the settings via config() and test the output, and just test that the settings form is saving the right config.
#10
This is looking good.
#11
We use instanceof most everywhere else, let's do that here as well.
#12
#13
Makes sense, and comes with tests. Yay!
Committed and pushed to 8.x. Thanks!
#14
Automatically closed -- issue fixed for 2 weeks with no activity.