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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dom. created an issue. See original summary.

plach’s picture

This sounds more major than critical to me, but leaving the decision to a committer.

dawehner’s picture

Issue summary: View changes
Issue tags: +Novice, +Needs tests

Well 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.

catch’s picture

Component: configuration system » search.module
Issue tags: +Configuration system

Search 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.

alexpott’s picture

Priority: Critical » Major
Issue tags: +rc target, +Configuration schema

It 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).

Dom.’s picture

Indeed, as a user I have no errors with this.
It only affects automated tests and is reported by the "Configuration Inspector" module too.

alexpott’s picture

Issue summary: View changes

I think this issue also exposes the fact we have no test coverage of search score sorted views - we should add that here.

jhodgdon’s picture

Status: Active » Postponed (maintainer needs more info)

We 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.

dawehner’s picture

Status: Postponed (maintainer needs more info) » Active

Oh yeah we need an exposed search for that, otherwise it won't appear in the needed config schema.

jhodgdon’s picture

OK. 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.

dawehner’s picture

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.

Yeah exactly you choose which search criteria is used ...

Regarding the config schema, read the issue summary, its described what we should add.

Dom.’s picture

regarding #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:

exposed: false
expose:
   label: ''

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.

Dom.’s picture

Status: Active » Needs review

Go testbot, go !

Status: Needs review » Needs work
Dom.’s picture

Status: Needs work » Needs review
FileSize
1.31 KB

Following patch now includes the updated tests from #12 plus de correction suggested by @dawehner in #3 and #11. It should solve this issue.

jhodgdon’s picture

Component: search.module » views.module
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests, -Configuration system, -rc target, -Configuration schema +rc target triage

Nice! 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.

xjm’s picture

Issue tags: -rc target triage +rc target

@alexpott and @catch signed off on this as an RC target in #5.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 5d450cb and pushed to 8.0.x. Thanks!

  • alexpott committed 5d450cb on 8.0.x
    Issue #2597137 by Dom., jhodgdon, dawehner: Missing schema error on '...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.