Comments

capysara created an issue. See original summary.

capysara’s picture

drunken monkey’s picture

Issue tags: +needs port to Drupal 8

Thanks for creating this issue and posting your patch!
Looks pretty good already, except for one mistake:

+++ b/search_api.admin.inc
@@ -1761,13 +1762,14 @@ function search_api_admin_index_workflow_submit(array $form, array &$form_state)
-    drupal_set_message(t("The indexing workflow was successfully edited. All content was scheduled for re-indexing so the new settings can take effect."));
+    $vars = array('@url' => $index_path);
+    drupal_set_message(t('The indexing workflow was successfully edited. All content was scheduled for <a href="@url">re-indexing</a> so the new settings can take effect.', $vars));

The path needs to be wrapped in an url() call, I think.
But other than that, it looks good, thanks!

Also, it seems not only would this need a port to D8, but we also seem to have neglected to do that for the other issue.

drunken monkey’s picture

Status: Active » Needs work
capysara’s picture

Like this?

drunken monkey’s picture

Status: Needs work » Needs review

Yes, looks great now, thanks a lot!

However, two little tips: When posting patches, please

  • set the status to "Needs review"
  • and include an interdiff (if the patch builds on a previous one).

The former is, apart from being "proper procedure" also needed for the test bot to spring into action.

drunken monkey’s picture

Issue summary: View changes
Status: Needs review » Patch (to be ported)
Issue tags: -needs port to Drupal 8
Related issues: +#2638740: add a link to the 'The index was cleared and will have to be re-indexed' message

Tests are green, so: committed.
Thanks again!

capysara’s picture

Thanks for tips!

I'll work on a patch for D8 next.

capysara’s picture

D8 version

capysara’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Patch (to be ported) » Needs review
drunken monkey’s picture

drunken monkey’s picture

Oops, wrong button …
Anyways, thanks a lot for the patch, looks great already!
Just had to re-roll because of #2931730: Adapt to changes in drupal_set_message(). Also, we'll want a test (assertion) for this, and I think using $index->toUrl() gets us the same URL in a simpler and cleaner manner.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

This looks very solid, great work!

  • drunken monkey committed 80505c4 on 8.x-1.x
    Issue #2948820 by capysara, drunken monkey, borisson_: Added a link to...
drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Great to hear, thanks for reviewing!
Committed.
Thanks again, capysara!

Status: Fixed » Closed (fixed)

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