Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround for operator minification bug. #1081

Merged
merged 5 commits into from
Dec 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions module/VuFind/src/VuFind/Search/QueryAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,13 @@ public static function minify(AbstractQuery $query, $topLevel = true)
'b' => $operator
];
if (null !== ($op = $current->getOperator())) {
// Some search forms omit the operator for the first element;
// if we have an operator in a subsequent element, we should
// backfill a blank here for consistency; otherwise, VuFind
// may not construct correct search URLs.
if (isset($retVal[0]['f']) && !isset($retVal[0]['o'])) {
$retVal[0]['o'] = '';
}
$currentArr['o'] = $op;
}
$retVal[] = $currentArr;
Expand Down
Binary file added module/VuFind/tests/fixtures/searches/operators
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,36 @@ public function testConversions()
}
}

/**
* Test that when one part of the query contains an operator, ALL parts of the
* query contain an operator. (We want to be sure that in cases where the first
* part of the query has no operator associated with it, a blank value is filled
* in as a placeholder.
*
* @return void
*/
public function testOperatorDefinedEverywhere()
{
$fixturePath = realpath(__DIR__ . '/../../../../fixtures/searches') . '/';
$q = unserialize(file_get_contents($fixturePath . '/operators'));
$minified = QueryAdapter::minify($q);

// First, check that count of 'o' values matches count of queries in group:
$callback = function ($carry, $item) {
return $carry + (isset($item['o']) ? 1 : 0);
};
$this->assertEquals(
count($minified[0]['g']),
array_reduce($minified[0]['g'], $callback, 0)
);

// Next, confirm that first operator is set to empty (filler) value:
$this->assertEquals('', $minified[0]['g'][0]['o']);

// Finally, make sure that we can round-trip back to the input.
$this->assertEquals($q, QueryAdapter::deminify($minified));
}

/**
* Test building an advanced query from a request.
*
Expand Down