fix: assign users order

resources/views/books/assign.blade.php

@@ -7,7 +7,7 @@
 <div class="wrap">
     <h1 class="wp-heading-inline">{{ __('Assign Books', 'pressbooks-multi-institution') }}</h1>
 
-    @if(! empty($params['s']) && ! empty($params['orderby']))
+    @if(! empty($params['s']))
         <div class="filtering">
             <ul>
                 <li>

resources/views/users/assign.blade.php

@@ -7,10 +7,10 @@
 <div class="wrap">
 	<h1 class="wp-heading-inline">{{ __('Assign Users', 'pressbooks-multi-institution') }}</h1>
 
-	@if( !empty($params['searchQuery']) && !empty($params['orderBy']))
+	@if(!empty($params['s']))
 		<div class="filtering">
 			<ul>
-				<li>{!! sprintf( __( '<strong>Showing results for:</strong> %s', 'pressbooks-multi-institution' ), $params['searchQuery'] ) !!}</li>
+				<li>{!! sprintf( __( '<strong>Showing results for:</strong> %s', 'pressbooks-multi-institution' ), $params['s'] ) !!}</li>
 			</ul>
 			<a href="{{ $list_url }}" class="button">{{ __('Clear filters', 'pressbooks-multi-institution') }}</a>
 		</div>
@@ -21,10 +21,16 @@
 	<form id="pressbooks-multi-institution-assign-users" method="GET">
 		<p class="search-box">
 			<label class="screen-reader-text" for="search-input">{{ __( 'Search', 'pressbooks-multi-institution') }}:</label>
-			<input type="search" id="search-input" name="s" value="">
+			<input type="search" id="search-input" name="s" value="{{ $params['s'] ?? '' }}">
 			<button id="search-apply" class="button" type="submit">{{ __( 'Search', 'pressbooks-multi-institution') }}</button>
 		</p>
 		<input type="hidden" name="page" value="{{ $page }}" />
+		@foreach ($params as $name => $value)
+			@if($name !== 's')
+				<input type="hidden" name="{{ $name }}" value="{{ $value }}" />
+			@endif
+		@endforeach
+
 		{!! $table->display() !!}
 	</form>
 </div>

src/Controllers/AssignBooksController.php

@@ -35,7 +35,6 @@ public function index(): string
             'page' => 'pb_multi_institution_assign_book',
             'params' => collect($filters)
                 ->flatMap(fn (string $filter, string $key) => [$key => $_REQUEST[$key] ?? $filter])
-                ->filter()
                 ->toArray(),
             'result' => $result,
             'table' => $this->table,

src/Controllers/InstitutionsUsersController.php

@@ -4,34 +4,39 @@
 
 use PressbooksMultiInstitution\Models\Institution;
 use PressbooksMultiInstitution\Models\InstitutionUser;
-use PressbooksMultiInstitution\Views\InstitutionsUsersTable;
+use PressbooksMultiInstitution\Views\AssignUsersTable;
 
 class InstitutionsUsersController extends BaseController
 {
-    private InstitutionsUsersTable $table;
+    private AssignUsersTable $table;
 
     public function __construct()
     {
         parent::__construct();
-        $this->table = new InstitutionsUsersTable;
+        $this->table = new AssignUsersTable;
     }
 
     public function assign(): string
     {
         $result = $this->processBulkActions();
 
+        $filters = [
+            'orderby' => 'username',
+            'order' => 'asc',
+            'paged' => 1,
+            's' => '',
+        ];
+
         $this->table->prepare_items();
 
         return $this->renderView('users.assign', [
             'page' => 'pb_multi_institutions_users',
             'list_url' => network_admin_url('admin.php?page=pb_multi_institutions_users'),
             'table' => $this->table,
             'result' => $result,
-            'params' => [
-                'searchQuery' => $_REQUEST['s'] ?? '',
-                'orderBy' => $_REQUEST['orderby'] ?? 'ID',
-                'order' => $_REQUEST['order'] ?? 'ASC',
-            ]
+            'params' => collect($filters)
+                ->flatMap(fn (string $filter, string $key) => [$key => sanitize_text_field($_REQUEST[$key] ?? $filter)])
+                ->toArray(),
         ]);
     }
 
@@ -49,11 +54,13 @@ protected function processBulkActions(): array
             return [];
         }
 
+        $successMsg = _n('User updated.', 'Users updated.', count($items), 'pressbooks-multi-institution');
+
         if ($action === '0') {
             InstitutionUser::query()->whereIn('user_id', $items)->delete();
             return [
                 'success' => true,
-                'message' => __('User/s unassigned.', 'pressbooks-multi-institution'),
+                'message' => $successMsg,
             ];
         }
 
@@ -74,7 +81,7 @@ protected function processBulkActions(): array
 
         return [
             'success' => true,
-            'message' => __('User/s assigned.', 'pressbooks-multi-institution'),
+            'message' => $successMsg,
         ];
     }
 }

src/Views/InstitutionsUsersTable.php → src/Views/AssignUsersTable.php

@@ -5,7 +5,7 @@
 use PressbooksMultiInstitution\Traits\OverridesBulkActions;
 use WP_List_Table;
 
-class InstitutionsUsersTable extends WP_List_Table
+class AssignUsersTable extends WP_List_Table
 {
     use OverridesBulkActions;
 
@@ -59,9 +59,7 @@ public function prepare_items(): void
     {
         $users = $this->getUsers($_REQUEST);
 
-        $columns = $this->get_columns();
-        $sortable = $this->get_sortable_columns();
-        $this->_column_headers = [$columns, [], $sortable];
+        $this->_column_headers = [$this->get_columns(), [], $this->get_sortable_columns()];
 
         $this->items = array_map(function ($user) {
             return [
@@ -82,6 +80,8 @@ public function prepare_items(): void
 
     private function getUsers(array $request): object
     {
+        $request = $this->validateRequest($request);
+
         $search = $request['s'] ?? '';
         $orderBy = $request['orderby'] ?? 'ID';
         $order = $request['order'] ?? 'ASC';
@@ -138,4 +138,26 @@ private function getUsers(array $request): object
             })
             ->paginate($this->paginationSize, ['*'], 'page', $request['paged'] ?? 1);
     }
+
+    private function validateRequest(array $request): array
+    {
+        $request['orderby'] = sanitize_text_field($request['orderby'] ?? '');
+        $request['order'] = sanitize_text_field($request['order'] ?? '');
+        $request['s'] = sanitize_text_field($request['s'] ?? '');
+        $request['paged'] = sanitize_text_field($request['paged'] ?? '');
+
+        if (isset($request['ID'])) {
+            $request['ID'] = array_map('intval', $request['ID']);
+        }
+
+        if (!in_array($request['orderby'], ['username', 'name', 'email', 'institution'])) {
+            $request['orderby'] = 'username';
+        }
+
+        if (!in_array(strtolower($request['order']), ['asc', 'desc'])) {
+            $request['order'] = 'asc';
+        }
+
+        return $request;
+    }
 }

tests/Feature/Controllers/InstitutionsUsersControllerTest.php

@@ -74,6 +74,30 @@ public function it_processes_bulk_actions(): void
         $this->assertEquals(0, InstitutionUser::query()->where('user_id', $users[1])->count());
     }
 
+    /**
+     * @test
+     */
+    public function it_keeps_ordering_after_bulk_action(): void
+    {
+        $_REQUEST['orderby'] = 'username';
+        $_REQUEST['order'] = 'DESC';
+        $index = $this->institutionsUsersController->assign();
+
+        $this->assertMatchesRegularExpression('/johndoe9/', $index);
+        $this->assertMatchesRegularExpression('/johndoe0/', $index);
+
+        $institutionsUsers = InstitutionUser::all();
+        $users = $institutionsUsers->pluck('user_id')->toArray();
+
+        $_REQUEST['ID'] = [$users[0], $users[1]];
+        $_REQUEST['action'] = $institutionsUsers->first()->institution_id;
+        $this->institutionsUsersController->assign();
+
+        $this->assertMatchesRegularExpression('/johndoe9/', $index);
+        $this->assertMatchesRegularExpression('/johndoe0/', $index);
+    }
+
+
     public function tearDown(): void
     {
         parent::tearDown();

tests/Feature/Views/InstitutionsUsersTableTest.php → tests/Feature/Views/AssignUsersTableTest.php

@@ -2,23 +2,23 @@
 
 namespace Tests\Feature\Views;
 
-use PressbooksMultiInstitution\Views\InstitutionsUsersTable;
+use PressbooksMultiInstitution\Views\AssignUsersTable;
 use Tests\TestCase;
 use Tests\Traits\CreatesModels;
 
 /**
  * @group institutions-users-table
  */
-class InstitutionsUsersTableTest extends TestCase
+class AssignUsersTableTest extends TestCase
 {
     use CreatesModels;
 
-    private InstitutionsUsersTable $institutionsUsersTable;
+    private AssignUsersTable $institutionsUsersTable;
 
     public function setUp(): void
     {
         parent::setUp();
-        $this->institutionsUsersTable = new InstitutionsUsersTable;
+        $this->institutionsUsersTable = new AssignUsersTable;
     }
 
     /**