Skip to content
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
c69cbc0
get unique html element id from record view helper
RLangeUni Jul 21, 2023
4859ace
add prefix for unique html id - adapt test
RLangeUni Jul 21, 2023
3d33b82
refs #2999 unique element id
Apr 3, 2024
3dd8446
refs #2999 unique element id
Apr 3, 2024
22e7630
refs #2999 keep prefix in context and set checkboxElementId in checkb…
RLangeUni Apr 18, 2024
cbdedf7
refs #2999 adjust test fir getCheckbox: use separate property for new…
RLangeUni Apr 18, 2024
3e308f2
refs #2999 use escapeHtmlAttr to escape checkboxElementId attribute
RLangeUni Apr 19, 2024
e294a75
refs #2999 fix testGetCheckbox with expectConsecutiveCalls
RLangeUni Apr 19, 2024
c05492d
refs #2999 fix styles
RLangeUni Apr 19, 2024
0c34fbc
refs #2999 fix styles
RLangeUni Apr 19, 2024
07af7fa
refs #2999 fix styles
RLangeUni Apr 19, 2024
60de01a
refs #2999 fix styles
RLangeUni Apr 19, 2024
d8dfc5e
refs #2999 remove unnecessary tryMethod
RLangeUni Apr 22, 2024
d4a6492
refs #2999 restore comment for duplicate call of getCheckbox
RLangeUni Apr 22, 2024
6c57f53
refs #2999 add simple test for getUniqueHtmlElementId
RLangeUni Apr 22, 2024
3a9c45f
refs #2999 add simple test for getUniqueHtmlElementId - remove traili…
RLangeUni Apr 22, 2024
1848009
Merge remote-tracking branch 'vufind-org/dev' into pull-request-uniqu…
RLangeUni Oct 15, 2024
bd13052
refs vufind-org#2999 Add support for result set identifier in Record …
RLangeUni Oct 16, 2024
6ee8f92
refs vufind-org#2999 sync method signature
RLangeUni Oct 16, 2024
010756c
refs vufind-org#2999 fix styles with vufind rules
RLangeUni Oct 16, 2024
634dacb
refs vufind-org#2999 fix styles with vufind rules
RLangeUni Oct 16, 2024
898df17
refs vufind-org#2999 import sprintf - todo: generateUuid() should be …
RLangeUni Oct 16, 2024
48e5a10
refs vufind-org#2999 add recordCollection as property in AbstractBack…
RLangeUni Oct 16, 2024
939411f
refs vufind-org#2999 sync checkbox.phtml 3 and 5
RLangeUni Oct 16, 2024
87480db
refs vufind-org#2999 remove function setResultSetIdentifier in Abstra…
RLangeUni Oct 18, 2024
d31c992
refs vufind-org#2999 cleanup AbstractRecordCollection - order setResu…
RLangeUni Oct 18, 2024
d614377
refs vufind-org#2999 do not use inheritdoc
RLangeUni Oct 18, 2024
3a9b9d5
refs vufind-org#2999 sync checkbox examples with dev - should be chan…
RLangeUni Oct 18, 2024
8cba230
refs vufind-org#2999 handle nullable result set id: add test
RLangeUni Oct 18, 2024
46b2f01
refs vufind-org#2999 handle nullable result set id - initializing to …
RLangeUni Oct 18, 2024
83f543d
refs vufind-org#2999 add testGetCheckboxWithoutIdAndWithEmptyPrefix
RLangeUni Oct 18, 2024
0ee81bc
refs vufind-org#2999 handle nullable result set id - initializing to …
RLangeUni Oct 18, 2024
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
41 changes: 37 additions & 4 deletions module/VuFind/src/VuFind/View/Helper/Root/Record.php
Original file line number Diff line number Diff line change
Expand Up @@ -560,10 +560,11 @@ public function getSearchResult($view)
*/
public function getCheckbox($idPrefix = '', $formAttr = false, $number = null)
{
$id = $this->driver->getSourceIdentifier() . '|'
. $this->driver->getUniqueId();
$context
= ['id' => $id, 'number' => $number, 'prefix' => $idPrefix];
$context = compact('number') + [
Comment thread
demiankatz marked this conversation as resolved.
'id' => $this->getUniqueIdWithSourcePrefix(),
'checkboxElementId' => $this->getUniqueHtmlElementId($idPrefix),
'prefix' => $idPrefix,
];
if ($formAttr) {
$context['formAttr'] = $formAttr;
}
Expand Down Expand Up @@ -857,4 +858,36 @@ protected function deduplicateLinks($links)
{
return array_values(array_unique($links, SORT_REGULAR));
}

/**
* Get the source identifier + unique id of the record without spaces
*
* @param string $idPrefix Prefix for HTML ids
*
* @return string
*/
public function getUniqueHtmlElementId($idPrefix = '')
{
return preg_replace(
"/\s+/",
'_',
($idPrefix ? $idPrefix . '-' : '')
. $this->driver->getResultSetIdentifier() . '-'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add handling here to ensure this value is non-null, since concatenating null into a string may cause a notice. It would probably be good to add a case to the unit test to cover the "null record set identifier" case so we can ensure it behaves as expected.

. $this->driver->getUniqueId()
);
}

/**
* Get the source identifier + unique id of the record
*
* @return string
*/
public function getUniqueIdWithSourcePrefix()
{
if ($this->driver) {
Comment thread
demiankatz marked this conversation as resolved.
return "{$this->driver->getSourceIdentifier()}"
. "|{$this->driver->getUniqueId()}";
}
throw new \Exception('No record driver found.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -389,32 +389,74 @@ public function testGetLink(
*/
public function testGetCheckbox(): void
{
$driver = $this->loadRecordFixture('testbug1.json');
$tpl = 'record/checkbox.phtml';
$context = $this->getMockContext();
$this->expectConsecutiveCalls(
$context,
'renderInContext',
$randomIdentifier = 'baz';
$driver->setResultSetIdentifier($randomIdentifier);

$expectedCalls = [
[
$tpl,
[
'record/checkbox.phtml',
['id' => 'Solr|000105196', 'number' => 1, 'prefix' => 'bar', 'formAttr' => 'foo'],
'number' => 1,
'id' => 'Solr|000105196',
'checkboxElementId' => "bar-{$randomIdentifier}-000105196",
'prefix' => 'bar',
'formAttr' => 'foo',
],
],
[
$tpl,
[
'record/checkbox.phtml',
['id' => 'Solr|000105196', 'number' => 2, 'prefix' => 'bar', 'formAttr' => 'foo'],
'number' => 2,
'id' => 'Solr|000105196',
'checkboxElementId' => "bar-{$randomIdentifier}-000105196",
'prefix' => 'bar',
'formAttr' => 'foo',
],
],
'success'
);
$record = $this->getRecord(
$this->loadRecordFixture('testbug1.json'),
[],
$context
];

$this->expectConsecutiveCalls(
$context,
'renderInContext',
$expectedCalls,
['success', 'success']
);

$record = $this->getRecord($driver, [], $context);

// We run the test twice to ensure that checkbox incrementing works properly:
Comment thread
demiankatz marked this conversation as resolved.
$this->assertEquals('success', $record->getCheckbox('bar', 'foo', 1));
$this->assertEquals('success', $record->getCheckbox('bar', 'foo', 2));
}

/**
* Test getUniqueHtmlElementId.
*
* @return void
*/
public function testGetUniqueHtmlElementId()
{
$driver = $this->loadRecordFixture('testbug1.json');
$record = $this->getRecord($driver);
$randomIdentifier = 'bar';
$driver->setResultSetIdentifier($randomIdentifier);

// with prefix
$this->assertEquals(
"testPrefix-{$randomIdentifier}-000105196",
$record->getUniqueHtmlElementId('testPrefix')
);

// without prefix
$this->assertEquals(
"{$randomIdentifier}-000105196",
$record->getUniqueHtmlElementId()
);
}

/**
* Test getTab.
*
Expand Down
43 changes: 43 additions & 0 deletions module/VuFindSearch/src/VuFindSearch/Backend/AbstractBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@
namespace VuFindSearch\Backend;

use Laminas\Log\LoggerAwareInterface;
use Laminas\Math\Rand;
use VuFindSearch\Response\RecordCollectionFactoryInterface;
use VuFindSearch\Response\RecordCollectionInterface;

use function sprintf;

/**
* Abstract backend.
*
Expand All @@ -53,6 +56,13 @@ abstract class AbstractBackend implements BackendInterface, LoggerAwareInterface
*/
protected $collectionFactory = null;

/**
* Record collection.
*
* @var RecordCollectionInterface|null
*/
protected $recordCollection = null;

/**
* Backend identifier.
*
Expand Down Expand Up @@ -116,6 +126,39 @@ abstract public function getRecordCollectionFactory();
protected function injectSourceIdentifier(RecordCollectionInterface $response)
{
$response->setSourceIdentifiers($this->identifier);
$response->setResultSetIdentifier($this->generateUuid());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach is fine for now, but maybe it's worth putting a TODO comment here to switch to using the Doctrine UUID generator once it becomes available to us in the future. Then we could potentially address that as part of (or sometime after the merge of) #2233.


return $response;
}

/**
* Sets the result set identifier for the record collection.
*
* @param string $uuid A valid UUID associated with the data set.
*
* @return void
*/
public function setResultSetIdentifier(string $uuid)
{
$this->recordCollection->setResultSetIdentifier($uuid);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method actually used? I think the property you were forced to add was related to the call in this method, but I suspect this method is unnecessary.

}

/**
* Generates a shorter UUID-like identifier.
*
* This method uses Laminas\Math\Rand to generate cryptographically secure random bytes
* and formats them into a shorter identifier.
*
* @return string A randomly generated shorter UUID-like identifier.
*/
public function generateUuid(): string
{
$data = bin2hex(Rand::getBytes(8));
return sprintf(
'%08s-%04s-%04s',
substr($data, 0, 8),
substr($data, 8, 4),
substr($data, 12, 4)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,21 @@ public function count(): int
{
return count($this->records);
}

/**
* Sets the result set identifier for all records in the collection.
*
* This method assigns a given UUID to each record in the collection by calling
* the `setResultSetIdentifier` method on each record.
*
* @param string $uuid A valid UUID to be assigned to each record in the collection.
*
* @return void
*/
public function setResultSetIdentifier(string $uuid)
{
foreach ($this->records as $record) {
$record->setResultSetIdentifier($uuid);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ public function setSourceIdentifiers($identifier, $searchBackendId = '');
*/
public function getSourceIdentifier();

/**
* Sets the result set identifier for the record.
*
* This method assigns a UUID or a unique string identifier to the result set.
*
* @param string $uuid A valid UUID or unique identifier to be assigned to the result set.
*
* @return void
*/
public function setResultSetIdentifier(string $uuid);

/**
* Add a record to the collection.
*
Expand Down
19 changes: 19 additions & 0 deletions module/VuFindSearch/src/VuFindSearch/Response/RecordInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ public function getSourceIdentifier();
*/
public function getSearchBackendIdentifier();

/**
* Sets the result set identifier for the record collection.
*
* @param string $uuid A valid UUID associated with the data set.
*
* @return void
*/
public function setResultSetIdentifier(string $uuid);

/**
* Retrieves the unique result set identifier.
*
* This method returns the UUID or similar identifier associated with the result set.
* If no identifier has been set, it will return null.
*
* @return string|null The UUID of the result set, or null if not set.
*/
public function getResultSetIdentifier();

/**
* Add a label for the record
*
Expand Down
37 changes: 37 additions & 0 deletions module/VuFindSearch/src/VuFindSearch/Response/RecordTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ trait RecordTrait
*/
protected $sourceIdentifier = '';

/**
* The unique identifier for the result set.
*
* This property stores a UUID or similar identifier that uniquely identifies
* the result set. It is typically set by calling the `setResultSetIdentifier` method.
*
* @var string|null
*/
protected $resultSetIdentifier;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest explicitly initializing this to null.


/**
* Used for identifying the search backend used to find the record
*
Expand Down Expand Up @@ -110,6 +120,33 @@ public function getSearchBackendIdentifier()
return $this->searchBackendIdentifier;
}

/**
* Sets the unique result set identifier.
*
* This method assigns a UUID or similar identifier to the result set.
*
* @param string $uuid A valid UUID or identifier to assign to the result set.
*
* @return void
*/
public function setResultSetIdentifier(string $uuid)
{
$this->resultSetIdentifier = $uuid;
}

/**
* Retrieves the unique result set identifier.
*
* This method returns the UUID or similar identifier associated with the result set.
* If no identifier has been set, it will return null.
*
* @return string|null The UUID of the result set, or null if not set.
*/
public function getResultSetIdentifier(): ?string
{
return $this->resultSetIdentifier;
}

/**
* Add a label for the record
*
Expand Down
2 changes: 1 addition & 1 deletion themes/bootstrap3/templates/record/checkbox.phtml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<label class="record-checkbox hidden-print">
<input class="checkbox-select-item" type="checkbox" name="ids[]" value="<?=$this->escapeHtmlAttr($this->id) ?>"<?php if (isset($this->formAttr)): ?> form="<?=$this->formAttr ?>"<?php endif; ?> aria-label="<?=$this->transEscAttr('select_item')?>">
<input class="checkbox-select-item" type="checkbox" name="ids[]" value="<?=$this->escapeHtmlAttr($this->id) ?>"<?php if (isset($this->formAttr)): ?> form="<?=$this->formAttr ?>"<?php endif; ?><?php if (isset($this->checkboxElementId)): ?> aria-describedby="<?=$this->escapeHtmlAttr($this->checkboxElementId) ?>"<?php endif; ?> aria-label="<?=$this->transEscAttr('select_item')?>">
<span class="checkbox-icon"></span>
<?php if (strlen($this->number ?? '') > 0): ?><span class="sr-only"><?=$this->transEsc('result_checkbox_label', ['%%number%%' => $this->number]) ?></span><?php endif; ?>
</label>
Expand Down
2 changes: 1 addition & 1 deletion themes/bootstrap5/templates/record/checkbox.phtml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<label class="record-checkbox hidden-print">
<input class="checkbox-select-item" type="checkbox" name="ids[]" value="<?=$this->escapeHtmlAttr($this->id) ?>"<?php if (isset($this->formAttr)): ?> form="<?=$this->formAttr ?>"<?php endif; ?> aria-label="<?=$this->transEscAttr('select_item')?>">
<input class="checkbox-select-item" type="checkbox" name="ids[]" value="<?=$this->escapeHtmlAttr($this->id) ?>"<?php if (isset($this->formAttr)): ?> form="<?=$this->formAttr ?>"<?php endif; ?><?php if (isset($this->checkboxElementId)): ?> aria-describedby="<?=$this->escapeHtmlAttr($this->checkboxElementId) ?>"<?php endif; ?> aria-label="<?=$this->transEscAttr('select_item')?>">
<span class="checkbox-icon"></span>
<?php if (strlen($this->number ?? '') > 0): ?><span class="sr-only"><?=$this->transEsc('result_checkbox_label', ['%%number%%' => $this->number]) ?></span><?php endif; ?>
</label>
Expand Down