Forumjump enhancement

[rica-carv]

Motivation and Context

Allow the forum jump list to display parent forum names + forum names.
Single displaying forum names is erroneous...

Description

CHanged the SQL to return also the parent name, to allow render parent forum + forum

How Has This Been Tested?

Current e107 github files

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist

• My code adheres to the e107 code style standard.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests pass.

[qlty-cloud-legacy]

Code Climate has analyzed commit df70f91 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 16.6% (80% is the threshold).

This pull request will bring the total coverage in the repository to 48.9% (0.0% change).

View more on Code Climate.

[e107help]

@rica-carv The complaint behind this is legitimate. A jump list of bare forum names is genuinely ambiguous once two parents own a forum with a similar name, and adding the parent fixes it.

I have not pushed anything here, because master moved underneath it and the fix is not a tidy-up. forumGetAllowed() was migrated to the query builder in v2.4, so the method you are editing no longer looks like this:

function forumGetAllowed($type='view')
{
	if(empty($this->permList[$type]))
	{
		return array();
	}

	$sql = e107::getDb();
	$rows = $sql->createQueryBuilder()
		->select('forum_id', 'forum_name', 'forum_sef')->from('forum')
		->whereIn('forum_id', $this->permList[$type])->where('forum_parent', '!=', 0)
		->fetchAll();

(forum_class.php#L2475-L2486)

GitHub reports the branch as conflicting for that reason. Beyond the mechanical clash, core no longer builds SQL by concatenation, so a new hand-written $qry would not get through review even after a rebase.

The same query on the builder is about this:

$qb = $sql->createQueryBuilder();
$rows = $qb->select('f.forum_id', 'f.forum_name', 'f.forum_sef')
	->selectAs('pf.forum_name', 'parent_name')
	->from('forum', 'f')
	->leftJoin('forum', 'pf', $qb->expr()->compareColumns('f.forum_parent', 'pf.forum_id'))
	->whereIn('f.forum_id', $this->permList[$type])->where('f.forum_parent', '!=', 0)
	->orderBy('pf.forum_id')->addOrderBy('f.forum_name')
	->fetchAll();

Three other notes while you are in there. forumGetAllowed('view', 'true') passes the string 'true', which works but reads as a mistake; a named $withParent = false boolean would be clearer. $val['parent_name'] will emit an undefined-key warning on PHP 8 for a top-level forum, so varset() or !empty() around it. And the new ORDER BY changes the jump list order for every site, not just those wanting parents, so it may be worth keeping that inside the same flag.

If you would rather not fight the rebase, say so on the thread and we will carry it for you; the feature is worth having either way. @Deltik decides which.

[rica-carv]

Ok, i'll give it a look as soon i feel "comfortable" with the new SQL query handler....