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.

Sign up for GitHub

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

Jump to bottom

Display status of item locations by status priority. #3758

Open
wants to merge 22 commits into
base: dev-11.0
Choose a base branch
from
Open

Display status of item locations by status priority. #3758

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2e917b0
Display status of item locations by status priority.
GrothTobias Jun 6, 2024
9c07778
Removed unneeded code
GrothTobias Jun 6, 2024
4d74ee2
Refactored display of item locations in the results
GrothTobias Jun 21, 2024
8cf1ee9
Reverted return values of AvailabilityStatus::availabilityAsString. A…
GrothTobias Jul 8, 2024
cc13f9d
Code styling
GrothTobias Jul 8, 2024
f257af4
Code styling
GrothTobias Jul 8, 2024
5fc5bac
Fixed results-scripts.phtml for theme bootstrap3
GrothTobias Jul 8, 2024
92b5022
Fixed availability display
GrothTobias Aug 12, 2024
0a56e68
Merge branch 'dev' into item-status-location-by-priority
demiankatz Sep 5, 2024
04ee7cf
Added 'status-uncertain' icons to js-icons.phtml
GrothTobias Sep 6, 2024
239c360
Fix broken tests.
demiankatz Sep 6, 2024
cfaa055
Return location list's icons and classes in ajax response.
GrothTobias Sep 9, 2024
2c46f8e
Removed unused 'availabilityClasses' declaration from check_item_stat…
GrothTobias Sep 9, 2024
34cc7be
Server side rendering of location list and callnumbers.
GrothTobias Sep 13, 2024
3f6a1fb
Fixed code style
GrothTobias Sep 13, 2024
c3d3ef6
Fixed code styling
GrothTobias Sep 13, 2024
cc00199
code style fix
GrothTobias Sep 13, 2024
264487b
Revert unwanted changes
GrothTobias Sep 23, 2024
47d57da
Refactored pickValue() to return an array. Use associative array for …
GrothTobias Oct 4, 2024
4d5522b
Added changes to bootstrap3 theme. Fixed code styling.
GrothTobias Oct 4, 2024
72ad7e4
Fixed code styling.
GrothTobias Oct 4, 2024
d1711b7
Fixed code styling.
GrothTobias Oct 4, 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
21 changes: 8 additions & 13 deletions module/VuFind/src/VuFind/AjaxHandler/GetItemStatuses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,22 +330,12 @@ protected function getItemStatusGroup($record, $callnumberSetting)
// Summarize call number, location and availability info across all items:
$locations = [];
foreach ($record as $info) {
$availabilityStatus = $info['availability'];
// Find an available copy
if ($availabilityStatus->isAvailable()) {
if ('true' !== ($locations[$info['location']]['available'] ?? null)) {
$locations[$info['location']]['available'] = $availabilityStatus->getStatusDescription();
}
}
// Check for a use_unknown_message flag
if ($availabilityStatus->is(AvailabilityStatusInterface::STATUS_UNKNOWN)) {
$locations[$info['location']]['status_unknown'] = true;
}
// Store call number/location info:
$locations[$info['location']]['callnumbers'][] = $this->formatCallNo(
$info['callnumber_prefix'] ?? '',
$info['callnumber']
);
$locations[$info['location']]['items'][] = $info;
}

// Build list split out by location:
Expand All @@ -362,16 +352,21 @@ protected function getItemStatusGroup($record, $callnumberSetting)
$callnumberSetting,
'Multiple Call Numbers'
);

// Get combined availability for location
$locationStatus = $this->availabilityStatusManager->combine($details['items']);
$locationAvailability = $locationStatus['availability'];

$locationInfo = [
'availability' => $details['available'] ?? false,
'availability' =>
GrothTobias marked this conversation as resolved.
Show resolved Hide resolved
$locationAvailability->availabilityAsString(),
'location' => htmlentities(
$this->translateWithPrefix('location_', $location),
ENT_COMPAT,
'UTF-8'
),
'callnumbers' =>
htmlentities($locationCallnumbers, ENT_COMPAT, 'UTF-8'),
'status_unknown' => $details['status_unknown'] ?? false,
'callnumber_handler' => $callnumberHandler,
];
$locationList[] = $locationInfo;
Expand Down
9 changes: 6 additions & 3 deletions module/VuFind/tests/integration-tests/src/VuFindTest/Mink/HoldingsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,10 @@ public function testItemStatus(
);
$this->assertEquals(
'Main Library',
$this->findCssAndGetText($page, '.result-body .callnumAndLocation .groupLocation .text-success')
$this->findCssAndGetText(
$page,
'.result-body .callnumAndLocation .groupLocation .text-' . $expectedType
)
);
}
} else {
Expand All @@ -196,8 +199,8 @@ public function testItemStatus(
} else {
// No extra items to care for:
if ('group' === $multipleLocations) {
// Unknown status displays as warning:
$type = null === $availability ? 'warning' : 'danger';
// Unknown status displays as muted:
$type = null === $availability ? 'muted' : 'danger';
$selector = ".result-body .callnumAndLocation .groupLocation .text-$type";
} else {
$selector = '.result-body .callnumAndLocation .location';
Expand Down
32 changes: 11 additions & 21 deletions themes/bootstrap3/js/check_item_statuses.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*global AjaxRequestQueue, VuFind */
/*global AjaxRequestQueue, VuFind, availabilityClasses */

VuFind.register('itemStatuses', function ItemStatuses() {
var _checkItemHandlers = {};
Expand Down Expand Up @@ -61,27 +61,17 @@ VuFind.register('itemStatuses', function ItemStatuses() {
el.querySelectorAll('.callnumber,.hideIfDetailed,.location').forEach((e) => e.classList.add('hidden'));
var locationListHTML = "";
for (var x = 0; x < result.locationList.length; x++) {
var availability = result.locationList[x].availability;
var statusIcon =
availability === 'true' ? 'available'
: availability === 'false' ? 'unavailable'
: availability;

locationListHTML += '<div class="groupLocation">';
if (result.locationList[x].availability) {
locationListHTML += '<span class="text-success">'
+ VuFind.icon("status-available")
+ result.locationList[x].location
+ '</span> ';
} else if (typeof(result.locationList[x].status_unknown) !== 'undefined'
&& result.locationList[x].status_unknown
) {
if (result.locationList[x].location) {
locationListHTML += '<span class="text-warning">'
+ VuFind.icon("status-unknown")
+ result.locationList[x].location
+ '</span> ';
}
} else {
locationListHTML += '<span class="text-danger">'
+ VuFind.icon("status-unavailable")
+ result.locationList[x].location
+ '</span> ';
}
locationListHTML += '<span class="' + availabilityClasses[availability] + '">'
+ VuFind.icon('status-' + statusIcon)
Copy link
Member

Choose a reason for hiding this comment

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

Should we have fallback behavior if availabilityClasses[availability] is undefined for some reason (either the AJAX handler returns an unexpected value, or the scripts setting the underlying data get broken)? It might be helpful to log something to the console, or at least avoid a Javascript error caused by an illegal array access.

Copy link
Member

Choose a reason for hiding this comment

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

Or alternatively, would it make sense to just pass the class as part of the AJAX response, and move some of the logic currently added to results-scripts.phtml into GetItemStatuses.php instead?

Copy link
Contributor Author

@GrothTobias GrothTobias Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think returning the class and icon in GetItemStatuses.php would be cleaner and is easier to customize. Also the whole availabilityClasses would be unnecessary (I wasn't really happy with how I added it).
We could also directly return the rendered HTML. We have customized the HTML a bit, so returning rendered HTML would be appreciated. :-)
@ThoWagen what do you think?

Regarding the priority: For now I would leave it as is. I think this is a somewhat individual desicion for the libraries, so maybe a little survey would be good to determine what should be used as default.

Also, if we move this to 11.0, I would change the true and false in Helper\Root\AvailabilityStatus:availabilityAsString() to more descriptive terms. Previously I reverted that change in case you wanted to include this PR in 10.1

Copy link
Contributor

Choose a reason for hiding this comment

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

@GrothTobias I just remembered that we had this discussion already here #3585 (review) . So yes, I think moving the logic from JS to GetItemStatuses.php would make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't clear if server-side rendering of the location list is wanted, so the classes and icons for each location are now returned in GetItemStatuses.php. I added the icon names (not sure what to call it) to Helper\Root\AvailabilityStatus.php, similar to the classes.
With this the field availability is not needed for locations but I'm unsure if I should remove it - it could be useful if someone wants to customize this in the JS.

Copy link
Member

Choose a reason for hiding this comment

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

I think at this point I've gone ahead and moved this to the 11.0 milestone, so you can feel free to make larger changes if you like. I don't mind doing more server-side rendering in the AJAX handler as long as it properly accounts for the theme system (e.g. use a template to render where appropriate). Less Javascript code constructing HTML is probably a good thing, for readability and customization. :-)

I'm going to hold off on further review/testing until you're done making the changes you wish to make, so please let me know when you are ready for me to take a closer look again!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The GetItemStatuses handler now returns rendered HTML. As mentioned in the @TODO there is still something to do but I don't have time for it next week.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the progress; since this is now scheduled for release 11, there's no huge rush to finish -- just please let me know when you're ready for another review.

+ result.locationList[x].location
+ '</span> ';
locationListHTML += '</div>';
locationListHTML += '<div class="groupCallnumber">';
locationListHTML += (result.locationList[x].callnumbers)
Expand Down
1 change: 1 addition & 0 deletions themes/bootstrap3/templates/layout/js-icons.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ $list = [
'status-pending',
'status-ready',
'status-unavailable',
'status-uncertain',
'status-unknown',
'ui-failure',
'ui-success',
Expand Down
19 changes: 19 additions & 0 deletions themes/bootstrap3/templates/search/results-scripts.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,22 @@ if (($this->listViewOption ?? 'full') !== 'full') {
if ($this->jsResults ?? false) {
$this->headScript()->appendFile('search.js');
}

$statuses = [
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_AVAILABLE,
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_UNAVAILABLE,
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_UNKNOWN,
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_UNCERTAIN,
];

$availabilityStatuses = [];
foreach ($statuses as $status) {
$status = new \VuFind\ILS\Logic\AvailabilityStatus($status);
$availabilityClasses[$status->availabilityAsString()] =
$this->availabilityStatus()->getClass($status);
}

$availabilityClassesJson = json_encode($availabilityClasses);
$this->headScript()->appendScript(<<<JS
var availabilityClasses = $availabilityClassesJson;
JS);
1 change: 1 addition & 0 deletions themes/bootstrap3/theme.config.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@
'status-pending' => 'FontAwesome:clock-o',
'status-ready' => 'FontAwesome:bell',
'status-unavailable' => 'FontAwesome:times',
'status-uncertain' => 'FontAwesome:circle',
'status-unknown' => 'FontAwesome:circle',
'tag-add' => 'Alias:ui-add',
'tag-remove' => 'Alias:ui-remove',
Expand Down
32 changes: 11 additions & 21 deletions themes/bootstrap5/js/check_item_statuses.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*global AjaxRequestQueue, VuFind */
/*global AjaxRequestQueue, VuFind, availabilityClasses */

VuFind.register('itemStatuses', function ItemStatuses() {
var _checkItemHandlers = {};
Expand Down Expand Up @@ -61,27 +61,17 @@ VuFind.register('itemStatuses', function ItemStatuses() {
el.querySelectorAll('.callnumber,.hideIfDetailed,.location').forEach((e) => e.classList.add('hidden'));
var locationListHTML = "";
for (var x = 0; x < result.locationList.length; x++) {
var availability = result.locationList[x].availability;
var statusIcon =
availability === 'true' ? 'available'
: availability === 'false' ? 'unavailable'
: availability;

locationListHTML += '<div class="groupLocation">';
if (result.locationList[x].availability) {
locationListHTML += '<span class="text-success">'
+ VuFind.icon("status-available")
+ result.locationList[x].location
+ '</span> ';
} else if (typeof(result.locationList[x].status_unknown) !== 'undefined'
&& result.locationList[x].status_unknown
) {
if (result.locationList[x].location) {
locationListHTML += '<span class="text-warning">'
+ VuFind.icon("status-unknown")
+ result.locationList[x].location
+ '</span> ';
}
} else {
locationListHTML += '<span class="text-danger">'
+ VuFind.icon("status-unavailable")
+ result.locationList[x].location
+ '</span> ';
}
locationListHTML += '<span class="' + availabilityClasses[availability] + '">'
+ VuFind.icon('status-' + statusIcon)
+ result.locationList[x].location
+ '</span> ';
locationListHTML += '</div>';
locationListHTML += '<div class="groupCallnumber">';
locationListHTML += (result.locationList[x].callnumbers)
Expand Down
1 change: 1 addition & 0 deletions themes/bootstrap5/templates/layout/js-icons.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ $list = [
'status-pending',
'status-ready',
'status-unavailable',
'status-uncertain',
'status-unknown',
'ui-failure',
'ui-success',
Expand Down
19 changes: 19 additions & 0 deletions themes/bootstrap5/templates/search/results-scripts.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,22 @@ if (($this->listViewOption ?? 'full') !== 'full') {
if ($this->jsResults ?? false) {
$this->headScript()->appendFile('search.js');
}

$statuses = [
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_AVAILABLE,
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_UNAVAILABLE,
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_UNKNOWN,
\VuFind\ILS\Logic\AvailabilityStatus::STATUS_UNCERTAIN,
];

$availabilityStatuses = [];
foreach ($statuses as $status) {
$status = new \VuFind\ILS\Logic\AvailabilityStatus($status);
$availabilityClasses[$status->availabilityAsString()] =
$this->availabilityStatus()->getClass($status);
}

$availabilityClassesJson = json_encode($availabilityClasses);
$this->headScript()->appendScript(<<<JS
var availabilityClasses = $availabilityClassesJson;
JS);
1 change: 1 addition & 0 deletions themes/bootstrap5/theme.config.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@
'status-pending' => 'FontAwesome:clock-o',
'status-ready' => 'FontAwesome:bell',
'status-unavailable' => 'FontAwesome:times',
'status-uncertain' => 'FontAwesome:circle',
'status-unknown' => 'FontAwesome:circle',
'tag-add' => 'Alias:ui-add',
'tag-remove' => 'Alias:ui-remove',
Expand Down
Loading