If large numbers of new results come in, for broader (or, possibly, completeley unfiltered) searches, users could get mails with hundreds or thousands of new results. This, of course, usually is not very practical. Therefore, we should implement a way for admins to limit the number of results sent per mail, and a probably a custom message to be included if that number is surpassed.

CommentFileSizeAuthor
#3 1881704--limit_mail_results-3.patch12.06 KBdrunken monkey
#1 1881704--limit_num_results-1.patch8.89 KBdrunken monkey

Comments

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 1881704--limit_num_results-1.patch8.89 KB

Attached is a patch which (hopefully) accomplishes this.
Please help testing!

mh86’s picture

mh86 commented
Status: Needs review » Needs work

Thanks for the patch, Thomas. Limiting the results shown in the mail basically works!

Still there a few things:

  • You are already using $settings->getTranslatedOption() to get the translated text (needs #1447004: Add i18n support), but the string is not registered in SearchApiSavedSearchesSettingsI18nController::translatableProperties(), and thus missing in the translation interface.
  • The view URL in t('View all results') . ': [search-api-saved-search:view-url]') is broken in my case. It just prints the relative part of the URL in the email. Maybe is due to the sanitization or it's a problem of my HTML mails. Does it work for you?
  • You use "0" for the unlimited option. Maybe "-1" would be more consistent with the rest of Drupal (at least it is used internally, e.g. for field cardinality). What would you say?
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new 1881704--limit_mail_results-3.patch12.06 KB

Thanks for the feedback. I hope the attached patch addresses all of it!

mh86’s picture

mh86 commented

Thanks for the update, Thomas.
There is one more issue that came up while testing: The results-capped token is shown in the email even if all results are listed (num results <= max results). I don't think this makes sense.

Rest of the patch is RTBC in my opinion.

mh86’s picture

mh86 commented
Status: Needs review » Needs work
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Fixed

Fixed (was just a missing break) and committed.
Thanks for testing!

Status: Fixed » Closed (fixed)

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