Make Content Ranking settings for Search not say "weights" to avoid confusion

Oh gracious. I think this is a UI bug, not a feature request. It should be a fairly easy patch.

I looked into this a bit.

The problem is actually that the value shown on the screen is not a "weight" as used elsewhere in the Drupal UI (i.e. an ordering of items in a list). It's actually a multiplying factor for calculating rankings of search results. So having values from 0 to 10 is actually the right thing, and a drag-drop interface is not the right thing.

There is a bug though: the UI should not be calling this "weight", probably, but "multiplier" or something...

Also, I noticed that this part of the settings form in D7 had some help information that was not being rendered, due to a typo (it was #value and not #markup in the form definition).

Here's a fix, and before/after screen shots.

Tagging.

Wrong patch, sorry!!! I was working on something else and forgot to revert it. Try this one.

Fixing title to reflect actual issue

#5: 535616.patch queued for re-testing.

See also
#834792: semantics in interface translation

#5: 535616.patch queued for re-testing.

#5: 535616.patch queued for re-testing.

#5: 535616.patch queued for re-testing.

tagging

There is zero chance this is making it into D7 at this point, since no one cared to review the patch for more than 6 months. Bumping to D8 for consideration, although hopefully we'll start over with a new search framework for D8 and this will be irrelevant.

Proper D8 patch.

Why are we adding all this help text, I do not see how its much more clear?

+1 for this change, was confused about this table myself and was almost suggesting tabledrag component. Please do it.

PS. Influence would be an awesome case for #1174646: Add new HTML5 FAPI element: range

#14: string-improvements-535616-14.patch queued for re-testing.

Retesting against latest HEAD since it has been over a year.

I hope this can get into 8. I just ran into this confusion installing a new D7 site.

I suspect the reason there isn't more attention paid to this is because "power" users replace Drupal core search with other things like Solr. But the built-in search settings page shouldn't say "weight" when it doesn't mean the same thing it means everywhere else.

The patch will need to be rerolled because this code is now I think in the NodeSearch plugin. But I think it's a good patch... can someone reroll it and we'll get it into 8 hopefully?

re-roll for the plugin change.

21: 535616-21.patch queued for re-testing.

pwolanin and I both think this is a sensible fix, and it is just UI text, so I am tentatively marking it RTBC but also tagging Usability in case any of the usability folks want to weigh in. I also just requested a re-test, just in case.

And I'm redoing the issue summary.

I got a bit confused scanning the text by 'higher influence numbers', makes me think of 7 or 13 - maybe one more look over for that?

This will conflict with #2042807: Convert search plugins to use a ConfigEntity and a PluginBag and could maybe just be changed in the patch for that issue.

RE #25 - Actually, no, we don't want to combine issues together and fix a lot of unrelated stuff on that issue. We'll just need to live with a few rerolls, depending on which issue lands first.

RE #24... good point.

So we've changed the UI so that the table headers are "Factor" and "Influence" instead of "Factor and "Weight" (see screen shots in #3). How about if the text above the table says:

Influence is a numeric multiplier used in ordering search results. A higher number means the corresponding factor has more influence on search results; zero means the factor is ignored. Changing these numbers does not require the search index to be rebuilt. Changes take effect immediately.

Here's a new patch, which will hopefully apply since I just (gasp!) edited the previous patch directly. Interdiff seems kind of pointless on a two-line patch. :)

Looks like a better explanation - let's get this little fix in.

I was going to say that tim.plunkett would be sad if this gets committed, but I do not believe it will conflict with #2042807: Convert search plugins to use a ConfigEntity and a PluginBag so it should not be a problem. It touches a few lines of the same file (NodeSearch), but they do not overlap so I think it will be OK.

Confirmed the two do not conflict.

Committed and pushed to 8.x. Thanks!

Do we want to backport this to D7 as well, since it's just a string change?

Sure, and it should be an easy patch to port.

Here is a straight port of this UI-text-only patch to Drupal 7.x.

It looks like the fix to show the help text got lost in a reroll, here's a patch to add it back again. I've also added a <p> tag for consistency and to give the text a bit of space.

Doh! Thanks, this is good.

(in other words, I applied the patch and tested manually by going to an edit page for the Node Search page config and the text now shows up)

32: search-535616-32.patch queued for re-testing.

Committed that follow-up, back to 7.x.

The 7.x patch in #31 needs a similar update to #32. The help text is not shown there either.

How's this?

I know I shouldn't review my own patch, but I figured I'd post screenshots at least.

Without patch:

With patch:

Screen shots are always welcome!

So, this patch is a straightforward port of the D8 patch and it works, so I think it is ready to be RTBC.

32: search-535616-32.patch queued for re-testing.

There was a test failure here in ComentPreviewTest. The log says:

exception 'DatabaseSchemaObjectExistsException' with message 'Table variable already exists.' in /var/lib/drupaltestbot/sites/default/files/checkout/includes/database/schema.inc:657
Stack trace:
#0 /var/lib/drupaltestbot/sites/default/files/checkout/includes/database/database.inc(2720): DatabaseSchema->createTable('variable', Array)
#1 /var/lib/drupaltestbot/sites/default/files/checkout/includes/common.inc(6951): db_create_table('variable', Array)
#2 /var/lib/drupaltestbot/sites/default/files/checkout/modules/system/system.install(525): drupal_install_schema('system')
#3 [internal function]: system_install()
#4 /var/lib/drupaltestbot/sites/default/files/checkout/includes/module.inc(866): call_user_func_array('system_install', Array)
#5 /var/lib/drupaltestbot/sites/default/files/checkout/includes/install.inc(724): module_invoke('system', 'install')
#6 /var/lib/drupaltestbot/sites/default/files/checkout/modules/simpletest/drupal_web_test_case.php(1458): drupal_install_system()
#7 /var/lib/drupaltestbot/sites/default/files/checkout/modules/comment/comment.test(14): DrupalWebTestCase->setUp('comment', 'search')
#8 /var/lib/drupaltestbot/sites/default/files/checkout/modules/simpletest/drupal_web_test_case.php(501): CommentHelperCase->setUp()
#9 /var/lib/drupaltestbot/sites/default/files/checkout/scripts/run-tests.sh(368): DrupalTestCase->run()
#10 /var/lib/drupaltestbot/sites/default/files/checkout/scripts/run-tests.sh(22): simpletest_script_run_one_test('1', 'CommentPreviewT...')
#11 {main}FATAL ContactPersonalTestCase: test runner returned a non-zero error code (1).

Unrelated to this patch. I don't think we need to click "Retest" here. The bot will test all RTBC patches every 24 hours anyway.

Committed to 7.x - thanks!

Drupal 7.27 was a security release, so this is now scheduled to go out in Drupal 7.28 instead.