Reviewer search is broken

[asmecher]

Valid Title

• I have updated the title to accurately reflect the bug description

Description

The reviewer search in OJS/OMP 3.5.0-2 is broken. See https://forum.pkp.sfu.ca/t/3-5-0-2-not-loading-search-in-add-reviewer/97520 for details.

This appears to be a regression caused by #11791.

Steps to Reproduce

1. Login with an Editor role
2. Navigate to a submission in review
3. Click the "Add reviewer" button
4. In the search box, type some text that will result in at least one patch
5. Check the error console and see a JS error.

Expected Result

The search should present no errors and the expected reviewer matches.

Actual Result

The search does not work and a JS error appears.

Environment Details

No response

Application Version

OJS 3.5.0-2

Logs

TypeError: this.items.filter is not a function or its return value is not iterable
at Proxy.currentReviewers (build.js?v=3.5.0.2:3:130)
at vme (build.js?v=3.5.0.2:1:9181)
at get value (build.js?v=3.5.0.2:1:21188)
at Object.get [as currentReviewers] (build.js?v=3.5.0.2:1:52005)
at build.js?v=3.5.0.2:13:2493
at Nn (build.js?v=3.5.0.2:1:48284)
at Proxy.tct (build.js?v=3.5.0.2:13:1258)
at yB (build.js?v=3.5.0.2:1:74312)
at fw.Bt [as fn] (build.js?v=3.5.0.2:1:64722)
at fw.run (build.js?v=3.5.0.2:1:7744)
M8e @ build.js?v=3.5.0.2:1
n5 @ build.js?v=3.5.0.2:1
yB @ build.js?v=3.5.0.2:1
Bt @ build.js?v=3.5.0.2:1
run @ build.js?v=3.5.0.2:1
Ue @ build.js?v=3.5.0.2:1
Be @ build.js?v=3.5.0.2:1
M @ build.js?v=3.5.0.2:1
me @ build.js?v=3.5.0.2:1
ge @ build.js?v=3.5.0.2:1
ee @ build.js?v=3.5.0.2:1
M @ build.js?v=3.5.0.2:1
Bt @ build.js?v=3.5.0.2:1
run @ build.js?v=3.5.0.2:1
runIfDirty @ build.js?v=3.5.0.2:1
t5 @ build.js?v=3.5.0.2:1
Wme @ build.js?v=3.5.0.2:1
Promise.then
Ume @ build.js?v=3.5.0.2:1
aJ @ build.js?v=3.5.0.2:1
(anonymous) @ build.js?v=3.5.0.2:1
trigger @ build.js?v=3.5.0.2:1
KX @ build.js?v=3.5.0.2:1
_b @ build.js?v=3.5.0.2:1
set @ build.js?v=3.5.0.2:1
set @ build.js?v=3.5.0.2:13
t5 @ build.js?v=3.5.0.2:1
Rh @ build.js?v=3.5.0.2:1
dwe @ build.js?v=3.5.0.2:1
setItems @ build.js?v=3.5.0.2:13
success @ build.js?v=3.5.0.2:1
c @ jquery.min.js?v=3.5.0.2:2
fireWith @ jquery.min.js?v=3.5.0.2:2
l @ jquery.min.js?v=3.5.0.2:2
(anonymous) @ jquery.min.js?v=3.5.0.2:2
XMLHttpRequest.send
send @ jquery.min.js?v=3.5.0.2:2
ajax @ jquery.min.js?v=3.5.0.2:2
get @ build.js?v=3.5.0.2:1
(anonymous) @ build.js?v=3.5.0.2:1
Promise.then
Ta @ build.js?v=3.5.0.2:1
searchPhrase @ build.js?v=3.5.0.2:1
t5 @ build.js?v=3.5.0.2:1
Rh @ build.js?v=3.5.0.2:1
(anonymous) @ build.js?v=3.5.0.2:1
U @ build.js?v=3.5.0.2:1
t5 @ build.js?v=3.5.0.2:1
Wme @ build.js?v=3.5.0.2:1
Promise.then
Ume @ build.js?v=3.5.0.2:1
aJ @ build.js?v=3.5.0.2:1
(anonymous) @ build.js?v=3.5.0.2:1
(anonymous) @ build.js?v=3.5.0.2:1
trigger @ build.js?v=3.5.0.2:1
KX @ build.js?v=3.5.0.2:1
_b @ build.js?v=3.5.0.2:1
set @ build.js?v=3.5.0.2:1
set @ build.js?v=3.5.0.2:1
setSearchPhrase @ build.js?v=3.5.0.2:1
t5 @ build.js?v=3.5.0.2:1
Rh @ build.js?v=3.5.0.2:1
dwe @ build.js?v=3.5.0.2:1
(anonymous) @ build.js?v=3.5.0.2:1
b @ build.js?v=3.5.0.2:1
setTimeout
_ @ build.js?v=3.5.0.2:1
OGe.H.onKeyup.t..t. @ build.js?v=3.5.0.2:1
t5 @ build.js?v=3.5.0.2:1
Rh @ build.js?v=3.5.0.2:1
n @ build.js?v=3.5.0.2:1

Additional Information

No response

[asmecher]

@Hafsa-Naeem, this appears to have been caused by a change in the way the api/v1/users/reviewers endpoint delivers results -- as a collection indexed by ID (which translates into a JSON object), rather than a list indexed sequentially (which translates into a JSON array).

At a glance, you can return to the expected behaviour with this change:

diff --git a/api/v1/users/PKPUserController.php b/api/v1/users/PKPUserController.php
index 900ac6dc3f..c88c84c4da 100644
--- a/api/v1/users/PKPUserController.php
+++ b/api/v1/users/PKPUserController.php
@@ -253,7 +253,7 @@ class PKPUserController extends PKPBaseController
 
         return response()->json([
             'itemsMax' => $collector->getCount(),
-            'items' => $items,
+            'items' => array_values($items),
         ], Response::HTTP_OK);
     }

...however, I'm not sure if other parts of the code need checking too.

[Rofypkr]

The file path is:
lib>pkp>api>v1>users>PKPUserController.php

Line no. 256

'items' => $items,
changed to
'items' => array_values($items),

Search is working now

[mjumbewu]

Thanks for the workaround!

FWIW, I can see that the same 'items' => $items, is in the return value for the getMany route in that file as well. That route is called from the main users & roles search, yet it seems to be working fine. The difference that I see is that in getMany, $items is defined as:

pkp-lib/api/v1/users/PKPUserController.php

Line 196 in ee9ff04

$items = $map->summarizeMany($users, $options)->values()->all();

Whereas in getManyReviewers it's defined as:

pkp-lib/api/v1/users/PKPUserController.php

Line 252 in ee9ff04

$items = $map->summarizeManyReviewers($usersCollection, $options)->all();

(no ->values())

I don't know PHP well enough to know which change should be made (i.e. add ->values() to $items definition in getManyReviewers, or add array_values to the return in getMany?), but I made a best guess and opened a PR.