Simplify common surnames

5 files changed, 87 insertions, 117 deletions

diff --git a/app/Functions/FunctionsDb.php b/app/Functions/FunctionsDb.php
index 26b50fc72d..7e2f011194 100644
--- a/app/Functions/FunctionsDb.php
+++ b/app/Functions/FunctionsDb.php
@@ -642,36 +642,15 @@ class FunctionsDb {
 	 * This function returns a simple array of the most common surnames
 	 * found in the individuals list.
 	 *
+	 * @deprecated
+	 *
 	 * @param int $min The number of times a surname must occur before it is added to the array
 	 * @param Tree $tree
 	 *
 	 * @return mixed[][]
 	 */
 	public static function getCommonSurnames($min, Tree $tree) {
-		$COMMON_NAMES_ADD    = $tree->getPreference('COMMON_NAMES_ADD');
-		$COMMON_NAMES_REMOVE = $tree->getPreference('COMMON_NAMES_REMOVE');
-
-		$topsurns = self::getTopSurnames($tree->getTreeId(), $min, 0);
-		foreach (explode(',', $COMMON_NAMES_ADD) as $surname) {
-			if ($surname && !array_key_exists($surname, $topsurns)) {
-				$topsurns[$surname] = $min;
-			}
-		}
-		foreach (explode(',', $COMMON_NAMES_REMOVE) as $surname) {
-			unset($topsurns[I18N::strtoupper($surname)]);
-		}
-
-		//-- check if we found some, else recurse
-		if (empty($topsurns) && $min > 2) {
-			return self::getCommonSurnames($min / 2, $tree);
-		} else {
-			uksort($topsurns, '\Fisharebest\Webtrees\I18N::strcasecmp');
-			foreach ($topsurns as $key => $value) {
-				$topsurns[$key] = array('name' => $key, 'match' => $value);
-			}
-
-			return $topsurns;
-		}
+		return self::getTopSurnames($tree->getTreeId(), $min, 0);
 	}
 
 	/**
@@ -691,7 +670,7 @@ class FunctionsDb {
 			return
 				Database::prepare(
 					"SELECT SQL_CACHE n_surn, COUNT(n_surn) FROM `##name`" .
-					" WHERE n_file = :tree_id AND n_type != '_MARNM' AND n_surn NOT IN ('@N.N.', '', '?', 'UNKNOWN')" .
+					" WHERE n_file = :tree_id AND n_type != '_MARNM' AND n_surn NOT IN ('@N.N.', '')" .
 					" GROUP BY n_surn HAVING COUNT(n_surn) >= :min" .
 					" ORDER BY 2 DESC"
 				)->execute(array(
@@ -702,7 +681,7 @@ class FunctionsDb {
 			return
 				Database::prepare(
 					"SELECT SQL_CACHE n_surn, COUNT(n_surn) FROM `##name`" .
-					" WHERE n_file = :tree_id AND n_type != '_MARNM' AND n_surn NOT IN ('@N.N.', '', '?', 'UNKNOWN')" .
+					" WHERE n_file = :tree_id AND n_type != '_MARNM' AND n_surn NOT IN ('@N.N.', '')" .
 					" GROUP BY n_surn HAVING COUNT(n_surn) >= :min" .
 					" ORDER BY 2 DESC" .
 					" LIMIT :limit"
diff --git a/app/Module/FamilyTreeStatisticsModule.php b/app/Module/FamilyTreeStatisticsModule.php
index e01b731476..006a2bb082 100644
--- a/app/Module/FamilyTreeStatisticsModule.php
+++ b/app/Module/FamilyTreeStatisticsModule.php
@@ -16,18 +16,25 @@
 namespace Fisharebest\Webtrees\Module;
 
 use Fisharebest\Webtrees\Auth;
+use Fisharebest\Webtrees\Database;
 use Fisharebest\Webtrees\Filter;
 use Fisharebest\Webtrees\Functions\FunctionsDb;
 use Fisharebest\Webtrees\Functions\FunctionsEdit;
+use Fisharebest\Webtrees\Functions\FunctionsPrintLists;
 use Fisharebest\Webtrees\I18N;
 use Fisharebest\Webtrees\Module;
+use Fisharebest\Webtrees\Query\QueryName;
 use Fisharebest\Webtrees\Stats;
 use Fisharebest\Webtrees\Theme;
+use Fisharebest\Webtrees\Tree;
 
 /**
  * Class FamilyTreeStatisticsModule
  */
 class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockInterface {
+	/** Show this number of surnames by default */
+	const DEFAULT_NUMBER_OF_SURNAMES = 10;
+	
 	/** {@inheritdoc} */
 	public function getTitle() {
 		return /* I18N: Name of a module */ I18N::translate('Statistics');
@@ -52,6 +59,7 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 
 		$show_last_update     = $this->getBlockSetting($block_id, 'show_last_update', '1');
 		$show_common_surnames = $this->getBlockSetting($block_id, 'show_common_surnames', '1');
+		$number_of_surnames   = $this->getBlockSetting($block_id, 'number_of_surnames', self::DEFAULT_NUMBER_OF_SURNAMES);
 		$stat_indi            = $this->getBlockSetting($block_id, 'stat_indi', '1');
 		$stat_fam             = $this->getBlockSetting($block_id, 'stat_fam', '1');
 		$stat_sour            = $this->getBlockSetting($block_id, 'stat_sour', '1');
@@ -71,9 +79,8 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 
 		// This can be overriden when embedding in an HTML block
 		$block     = '0';
-		$stat_link = '1';
 
-		foreach (array('show_common_surnames', 'stat_indi', 'stat_fam', 'stat_sour', 'stat_media', 'stat_surname', 'stat_events', 'stat_users', 'stat_first_birth', 'stat_last_birth', 'stat_first_death', 'stat_last_death', 'stat_long_life', 'stat_avg_life', 'stat_most_chil', 'stat_avg_chil', 'stat_link', 'block') as $name) {
+		foreach (array('show_common_surnames', 'number_common_surnames', 'stat_indi', 'stat_fam', 'stat_sour', 'stat_media', 'stat_surname', 'stat_events', 'stat_users', 'stat_first_birth', 'stat_last_birth', 'stat_first_death', 'stat_last_death', 'stat_long_life', 'stat_avg_life', 'stat_most_chil', 'stat_avg_chil', 'block') as $name) {
 			if (array_key_exists($name, $cfg)) {
 				$$name = $cfg[$name];
 			}
@@ -193,27 +200,22 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 			$content .= '</div>';
 		}
 		$content .= '</div>';
-		if ($stat_link && Module::isActiveChart($WT_TREE, 'statistics_chart')) {
-			$content .= '<div class="clearfloat">';
-			$content .= '<p>';
-			$content .= '<a href="statistics.php?ged=' . $WT_TREE->getNameUrl() . '" rel="nofollow"><b>' . I18N::translate('View the statistics as graphs') . '</b></a>';
-			$content .= '</p>';
-			$content .= '</div>';
-		}
 
 		if ($show_common_surnames) {
-			$surnames = FunctionsDb::getCommonSurnames($WT_TREE->getPreference('COMMON_NAMES_THRESHOLD'), $WT_TREE);
-			$surnames = array_map(function ($x) use ($WT_TREE) {
-				return '<a href="indilist.php?ged=' . $WT_TREE->getNameUrl() . '&amp;surname=' . Filter::escapeUrl($x['name']) . '">' . Filter::escapeHtml($x['name']) . '</a>';
-			}, $surnames);
-			$surnames = implode(I18N::$list_separator, $surnames);
+			$surnames = FunctionsDb::getTopSurnames($WT_TREE->getTreeId(), 0, (int) $number_of_surnames);
 
-			if ($surnames) {
+			$all_surnames = array();
+			foreach (array_keys($surnames) as $surname) {
+				$all_surnames = array_merge($all_surnames, QueryName::surnames($WT_TREE, $surname, '', false, false));
+			}
+
+			if (!empty($surnames)) {
+				ksort($all_surnames);
 				$content .= '<div class="clearfloat">';
 				$content .= '<p>';
 				$content .= '<strong>' . I18N::translate('Most common surnames') . '</strong>';
 				$content .= '<br>';
-				$content .= '<span class="common_surnames">' . $surnames . '</span>';
+				$content .= '<span class="common_surnames">' . FunctionsPrintLists::surnameList($all_surnames, 2, false, 'indilist.php', $WT_TREE) . '</span>';
 				$content .= '</p>';
 				$content .= '</div>';
 			}
@@ -250,6 +252,7 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 		if (Filter::postBool('save') && Filter::checkCsrf()) {
 			$this->setBlockSetting($block_id, 'show_last_update', Filter::postBool('show_last_update'));
 			$this->setBlockSetting($block_id, 'show_common_surnames', Filter::postBool('show_common_surnames'));
+			$this->setBlockSetting($block_id, 'number_of_surnames', Filter::postInteger('number_of_surnames'));
 			$this->setBlockSetting($block_id, 'stat_indi', Filter::postBool('stat_indi'));
 			$this->setBlockSetting($block_id, 'stat_fam', Filter::postBool('stat_fam'));
 			$this->setBlockSetting($block_id, 'stat_sour', Filter::postBool('stat_sour'));
@@ -271,6 +274,7 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 
 		$show_last_update     = $this->getBlockSetting($block_id, 'show_last_update', '1');
 		$show_common_surnames = $this->getBlockSetting($block_id, 'show_common_surnames', '1');
+		$number_of_surnames   = $this->getBlockSetting($block_id, 'number_of_surnames', self::DEFAULT_NUMBER_OF_SURNAMES);
 		$stat_indi            = $this->getBlockSetting($block_id, 'stat_indi', '1');
 		$stat_fam             = $this->getBlockSetting($block_id, 'stat_fam', '1');
 		$stat_sour            = $this->getBlockSetting($block_id, 'stat_sour', '1');
@@ -291,23 +295,18 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 		?>
 		<tr>
 			<td class="descriptionbox wrap width33">
-				<?php echo /* I18N: label for yes/no option */
-				I18N::translate('Show date of last update') ?>
+				<label for="show-last-update">
+					<?php echo /* I18N: label for yes/no option */ I18N::translate('Show date of last update') ?>
+				</label>
 			</td>
 			<td class="optionbox">
-				<?php echo FunctionsEdit::editFieldYesNo('show_last_update', $show_last_update) ?>
+				<input type="checkbox" value="yes" id="show-last-update" name="show_last_update" <?php echo $show_last_update ? 'checked' : ''; ?>>
 			</td>
 		</tr>
 		<tr>
 			<td class="descriptionbox wrap width33">
-				<?php echo I18N::translate('Show common surnames') ?>
-			</td>
-			<td class="optionbox">
-				<?php echo FunctionsEdit::editFieldYesNo('show_common_surnames', $show_common_surnames) ?>
+				<?php echo I18N::translate('Statistics'); ?>
 			</td>
-		</tr>
-		<tr>
-			<td class="descriptionbox wrap width33"><?php echo I18N::translate('Select the stats to show in this block'); ?></td>
 			<td class="optionbox">
 				<table>
 					<tbody>
@@ -427,6 +426,34 @@ class FamilyTreeStatisticsModule extends AbstractModule implements ModuleBlockIn
 				</table>
 			</td>
 		</tr>
+		<tr>
+			<td class="descriptionbox wrap width33">
+				<label for="show-common-surnames">
+					<?php echo I18N::translate('Most common surnames') ?>
+				</label>
+			</td>
+			<td class="optionbox">
+				<input type="checkbox" value="yes" id="show-common-surnames" name="show_common_surnames" <?php echo $show_common_surnames ? 'checked' : ''; ?>>
+			</td>
+		</tr>
+		<tr>
+			<td class="descriptionbox wrap width33">
+				<label for="number-of-surnames">
+					<?php echo I18N::translate('Number of surnames') ?>
+				</label>
+			</td>
+			<td class="optionbox">
+				<input
+					id="number-of-surnames"
+					maxlength="5"
+					name="number_of_surnames"
+					pattern="[1-9][0-9]*"
+					required
+					type="text"
+					value="<?php echo Filter::escapeHtml($number_of_surnames); ?>"
+				>
+			</td>
+		</tr>
 		<?php
 	}
 }
diff --git a/app/Module/TopSurnamesModule.php b/app/Module/TopSurnamesModule.php
index ee41cdfc89..c59d9ad23e 100644
--- a/app/Module/TopSurnamesModule.php
+++ b/app/Module/TopSurnamesModule.php
@@ -104,11 +104,11 @@ class TopSurnamesModule extends AbstractModule implements ModuleBlockInterface {
 			break;
 		case 'list':
 			uasort($all_surnames, '\Fisharebest\Webtrees\Module\TopSurnamesModule::surnameCountSort');
-			$content = FunctionsPrintLists::surnameList($all_surnames, '1', true, 'indilist.php', $WT_TREE);
+			$content = FunctionsPrintLists::surnameList($all_surnames, 1, true, 'indilist.php', $WT_TREE);
 			break;
 		case 'array':
 			uasort($all_surnames, '\Fisharebest\Webtrees\Module\TopSurnamesModule::surnameCountSort');
-			$content = FunctionsPrintLists::surnameList($all_surnames, '2', true, 'indilist.php', $WT_TREE);
+			$content = FunctionsPrintLists::surnameList($all_surnames, 2, true, 'indilist.php', $WT_TREE);
 			break;
 		case 'table':
 		default:
@@ -154,7 +154,7 @@ class TopSurnamesModule extends AbstractModule implements ModuleBlockInterface {
 		$infoStyle = $this->getBlockSetting($block_id, 'infoStyle', 'table');
 
 		echo '<tr><td class="descriptionbox wrap width33">';
-		echo I18N::translate('Number of items to show');
+		echo I18N::translate('Number of surnames');
 		echo '</td><td class="optionbox">';
 		echo '<input type="text" name="num" size="2" value="', $num, '">';
 		echo '</td></tr>';
diff --git a/app/Schema/Migration34.php b/app/Schema/Migration34.php
index c013b0a6b4..4e5a292cde 100644
--- a/app/Schema/Migration34.php
+++ b/app/Schema/Migration34.php
@@ -15,17 +15,23 @@
  */
 namespace Fisharebest\Webtrees\Schema;
 
+use Fisharebest\Webtrees\Database;
 use Fisharebest\Webtrees\Module;
 
 /**
  * Upgrade the database schema from version 33 to version 34.
  */
 class Migration34 implements MigrationInterface {
-
 	/**
-	 * New modules (charts) have been added.
+	 * Upgrade to to the next version
 	 */
 	public function upgrade() {
+		// New modules (charts) have been added.
 		Module::getInstalledModules('enabled');
+
+		// Delete old/unused settings
+		Database::exec(
+			"DELETE FROM `##gedcom_setting` WHERE setting_name IN ('COMMON_NAMES_ADD', 'COMMON_NAMES_REMOVE', 'COMMON_NAMES_THRESHOLD')"
+		);
 	}
 }
diff --git a/app/Stats.php b/app/Stats.php
index 9f2f891842..bf8ca6618b 100644
--- a/app/Stats.php
+++ b/app/Stats.php
@@ -5310,29 +5310,14 @@ class Stats {
 	 * @return string
 	 */
 	private function commonSurnamesQuery($type = 'list', $show_tot = false, $params = array()) {
-		if (isset($params[0]) && $params[0] > 0) {
-			$threshold = (int) $params[0];
-		} else {
-			$threshold = $this->tree->getPreference('COMMON_NAMES_THRESHOLD');
-		}
-		if (isset($params[1])) {
-			$maxtoshow = (int) $params[1];
-		} else {
-			$maxtoshow = 0;
-		}
-		if (isset($params[2])) {
-			$sorting = $params[2];
-		} else {
-			$sorting = 'alpha';
-		}
-		$surname_list = FunctionsDb::getCommonSurnames($threshold, $this->tree);
-		if (count($surname_list) == 0) {
+		$threshold          = empty($params[0]) ? 10 : (int) $params[0];
+		$number_of_surnames = empty($params[1]) ? 10 : (int) $params[1];
+		$sorting            = empty($params[2]) ? 'alpha' : $params[2];
+
+		$surname_list = FunctionsDb::getTopSurnames($this->tree->getTreeId(), $threshold, $number_of_surnames);
+		if (empty($surname_list)) {
 			return '';
 		}
-		uasort($surname_list, '\Fisharebest\Webtrees\Stats::nameTotalReverseSort');
-		if ($maxtoshow > 0) {
-			$surname_list = array_slice($surname_list, 0, $maxtoshow);
-		}
 
 		switch ($sorting) {
 		default:
@@ -5340,10 +5325,10 @@ class Stats {
 			uksort($surname_list, '\Fisharebest\Webtrees\I18N::strcasecmp');
 			break;
 		case 'count':
-			uasort($surname_list, '\Fisharebest\Webtrees\Stats::nameTotalSort');
+			asort($surname_list);
 			break;
 		case 'rcount':
-			uasort($surname_list, '\Fisharebest\Webtrees\Stats::nameTotalReverseSort');
+			arsort($surname_list);
 			break;
 		}
 
@@ -5424,50 +5409,23 @@ class Stats {
 		$WT_STATS_S_CHART_X    = Theme::theme()->parameter('stats-small-chart-x');
 		$WT_STATS_S_CHART_Y    = Theme::theme()->parameter('stats-small-chart-y');
 
-		if (isset($params[0]) && $params[0] != '') {
-			$size = strtolower($params[0]);
-		} else {
-			$size = $WT_STATS_S_CHART_X . "x" . $WT_STATS_S_CHART_Y;
-		}
-		if (isset($params[1]) && $params[1] != '') {
-			$color_from = strtolower($params[1]);
-		} else {
-			$color_from = $WT_STATS_CHART_COLOR1;
-		}
-		if (isset($params[2]) && $params[2] != '') {
-			$color_to = strtolower($params[2]);
-		} else {
-			$color_to = $WT_STATS_CHART_COLOR2;
-		}
-		if (isset($params[3]) && $params[3] != '') {
-			$threshold = strtolower($params[3]);
-		} else {
-			$threshold = $this->tree->getPreference('COMMON_NAMES_THRESHOLD');
-		}
-		if (isset($params[4]) && $params[4] != '') {
-			$maxtoshow = strtolower($params[4]);
-		} else {
-			$maxtoshow = 7;
-		}
+		$size               = empty($params[0]) ? $WT_STATS_S_CHART_X . "x" . $WT_STATS_S_CHART_Y : strtolower($params[0]);
+		$color_from         = empty($params[1]) ? $WT_STATS_CHART_COLOR1 : strtolower($params[1]);
+		$color_to           = empty($params[2]) ? $WT_STATS_CHART_COLOR2 : strtolower($params[2]);
+		$number_of_surnames = empty($params[3]) ? 10 : (int) $params[3];
+
 		$sizes    = explode('x', $size);
 		$tot_indi = $this->totalIndividualsQuery();
-		$surnames = FunctionsDb::getCommonSurnames($threshold, $this->tree);
-		if (count($surnames) <= 0) {
+		$surnames = FunctionsDb::getTopSurnames($this->tree->getTreeId(), 0, $number_of_surnames);
+		if (empty($surnames)) {
 			return '';
 		}
 		$SURNAME_TRADITION = $this->tree->getPreference('SURNAME_TRADITION');
-		uasort($surnames, '\Fisharebest\Webtrees\Stats::nameTotalReverseSort');
-		$surnames     = array_slice($surnames, 0, $maxtoshow);
 		$all_surnames = array();
-		foreach (array_keys($surnames) as $n => $surname) {
-			if ($n >= $maxtoshow) {
-				break;
-			}
+		$tot          = 0;
+		foreach ($surnames as $surname => $num) {
 			$all_surnames = array_merge($all_surnames, QueryName::surnames($this->tree, I18N::strtoupper($surname), '', false, false));
-		}
-		$tot = 0;
-		foreach ($surnames as $surname) {
-			$tot += $surname['match'];
+			$tot += $num;
 		}
 		$chd = '';
 		$chl = array();