Problem/Motivation
When creating a view with "search score" sorting, the configuration generated has an error of "missing schema".
This causes modules that has such a view in there config to fail at tests. Contribe module 'Search Autocomplete' if blocked by its tests failing because of this. That is the reason why I set this issue (probably wrongly) to critical, relatively to Cause tests to fail in HEAD on the automated testing platform, since this blocks all other work.
considering also contrib module HEAD.
Attached is a view that reproduce this issue.
Proposed resolution
Add an entry which looks like:
views.sort_expose.*:
type: views_sort_expose
label: 'Fallback sort expose settings'
into core/modules/views/config/schema/views.sort.schema.yml
Add test coverage of a view using this sort.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
Not really. A default fallback for exposed sort schema will be added to Views so that this type of problem will not come up with other configuration.
Comment | File | Size | Author |
---|---|---|---|
#15 | sorts.score_.expose_missing_schema--2597137-15.patch | 1.31 KB | Dom. |
#12 | sorts.score_.expose_missing_schema--2597137-9--failing_tests_only.patch | 793 bytes | Dom. |
views missing shcema error.png | 5.55 KB | Dom. | |
views.view_.testing_search_view.yml | 4.86 KB | Dom. |
Comments
Comment #2
plachThis sounds more major than critical to me, but leaving the decision to a committer.
Comment #3
dawehnerWell yeah I don't care either whether this is critical or not to be honest. Adding some novice instructions .
On top of that we also needs tests.
Comment #4
catchSearch score views integration definitely isn't release blocking for me, but not sure how the missing config schema affects config deployments.
Moving to search module and tagging.
Comment #5
alexpottIt does not affect config deployments - we don;t yet use schema to validate them and anyway if we did we would probably only produce a warning. But missing schema is something that we should do during RC. Discussed with @catch and he agrees as there is no user facing error (simpletest does not count).
Comment #6
Dom. CreditAttribution: Dom. commentedIndeed, as a user I have no errors with this.
It only affects automated tests and is reported by the "Configuration Inspector" module too.
Comment #7
alexpottI think this issue also exposes the fact we have no test coverage of search score sorted views - we should add that here.
Comment #8
jhodgdonWe definitely *do* have coverage of views sorted by search score. It is in core/modules/views/src/Tests/SearchIntegrationTest, which uses the test view in core/modules/views/tests/modules/views_test_config/test_views/views.view.test_search.yml
So I think we need more information about how to reproduce this. The test currently passes. Views using search with a score sort definitely do work.
Comment #9
dawehnerOh yeah we need an exposed search for that, otherwise it won't appear in the needed config schema.
Comment #10
jhodgdonOK. Well then.
It sounds like we need to add another display to the test view and a few more lines to the test mentioned in #8, to hit an exposed sort. Although why it would even make sense to "expose" the sort by search score, I have no idea? Oh, I guess you can choose what to sort by, if there are multiple sorts, and then ascending/descending. Anyway.
And then... still not sure what we need to add to the config schema and where. The summary says to put something into Views schema as a fallback. Is that the right fix? If not then probably something needs to go into the search module's views schema.
Comment #11
dawehnerYeah exactly you choose which search criteria is used ...
Regarding the config schema, read the issue summary, its described what we should add.
Comment #12
Dom. CreditAttribution: Dom. commentedregarding #8 from jhodgdon:
Indeed, but this same view (views.view.test_search.yml), try this :
- import it manually to your D8
- edit / save the view
- export it manually
The export now contains the following newlines :
- search
in the module dependency section
and in the sort section:
This new export now is unvalid.
The patch attached is the update of this file according to what is actually exported by Drupal (not including cache context tags and stuffs).
Short answer to #8 is therefore: yes there is a test for this and it works. But the views that is depicted in the test is not the one that is exported by a D8 site, thus it is for me a test that does not describe reality.
Comment #13
Dom. CreditAttribution: Dom. commentedGo testbot, go !
Comment #15
Dom. CreditAttribution: Dom. commentedFollowing patch now includes the updated tests from #12 plus de correction suggested by @dawehner in #3 and #11. It should solve this issue.
Comment #16
jhodgdonNice! Thanks for reporting *and* fixing the problem! I know for a fact that that search config in the test was created from an export, so that must have been added to the views config schema after that test was created.
This exposes a problem in how we are testing with strict configuration checking: we are not ever verifying that config we read from a YML file matches what would now be saved if that same config object was saved, so we're missing this type of error.
In any case, this patch looks fine and I think it probably belongs in the Views project, because it may affect other projects that have Views config that (like search) didn't get updated whenever that stuff was added to the Views config schema.
Comment #18
xjm@alexpott and @catch signed off on this as an RC target in #5.
Comment #19
alexpottCommitted 5d450cb and pushed to 8.0.x. Thanks!