From e35432c704a61e5c33193c103d7cba88c9307d3a Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Wed, 11 Oct 2023 12:21:11 -0400 Subject: [PATCH 01/13] piggyback on my-widgets-page pagination hook --- fuel/app/classes/controller/api/admin.php | 13 ++++++++--- fuel/app/classes/materia/perm/manager.php | 2 +- .../materia/widget/instance/manager.php | 22 +++++++++++++++++- src/components/hooks/useInstanceList.jsx | 12 ++++++---- src/components/support-search.jsx | 23 ++++++++----------- src/util/api.js | 12 ++++++---- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/fuel/app/classes/controller/api/admin.php b/fuel/app/classes/controller/api/admin.php index df6e1bb2b..da13cdd33 100644 --- a/fuel/app/classes/controller/api/admin.php +++ b/fuel/app/classes/controller/api/admin.php @@ -69,13 +69,20 @@ public function post_user($user_id) return \Service_User::update_user($user_id, $user); } - public function get_widget_search(string $input) + public function get_widget_paginated_search(string $input, string $page_number) { $input = trim($input); $input = urldecode($input); + $page_number = (int) $page_number; //no need to search if for some reason an empty string is passed - if ($input == '') return []; - return \Materia\Widget_Instance_Manager::get_search($input); + if ($input == '') + { + return [ + 'pagination' => [], + 'next_page' => $page_number + ]; + } + return \Materia\Widget_Instance_Manager::get_paginated_search($input, $page_number); } public function get_extra_attempts(string $inst_id) diff --git a/fuel/app/classes/materia/perm/manager.php b/fuel/app/classes/materia/perm/manager.php index bf3e703da..9986c66e0 100644 --- a/fuel/app/classes/materia/perm/manager.php +++ b/fuel/app/classes/materia/perm/manager.php @@ -38,7 +38,7 @@ static public function is_super_user() // The session caching has been removed due to issues related to the cache when the role is added or revoked // Ideally we can still find a way to cache this and make it more performant!! return (\Fuel::$is_cli === true && ! \Fuel::$is_test) || self::does_user_have_role([\Materia\Perm_Role::SU]); - + } /** diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index d6ef5f140..f23775947 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -91,7 +91,7 @@ public static function get_paginated_for_user($user_id, $page_number = 0) $data = [ 'pagination' => $displayable_inst, ]; - + if ($has_next_page) $data['next_page'] = $page_number + 1; return $data; @@ -135,6 +135,26 @@ public static function lock($inst_id) return $locked_by == $me; } + public static function get_paginated_search(string $input, $page_number = 0) + { + $displayable_insts = self::get_search($input); + $widgets_per_page = 80; + $total_num_pages = ceil(sizeof($displayable_insts) / $widgets_per_page); + $offset = $widgets_per_page * $page_number; + $has_next_page = $offset + $widgets_per_page < sizeof($displayable_insts) ? true : false; + + // inst_ids corresponds to a single page's worth of instances + $displayable_insts = array_slice($displayable_insts, $offset, $widgets_per_page); + + $data = [ + 'pagination' => $displayable_insts, + ]; + + if ($has_next_page) $data['next_page'] = $page_number + 1; + + return $data; + } + /** * Gets all widget instances related to a given input, including id or name. * diff --git a/src/components/hooks/useInstanceList.jsx b/src/components/hooks/useInstanceList.jsx index 41577b6d2..5811154b4 100644 --- a/src/components/hooks/useInstanceList.jsx +++ b/src/components/hooks/useInstanceList.jsx @@ -1,9 +1,9 @@ import { useState, useEffect, useMemo } from 'react' import { useInfiniteQuery } from 'react-query' -import { apiGetWidgetInstances } from '../../util/api' +import { apiGetWidgetInstances, apiSearchWidgets } from '../../util/api' import { iconUrl } from '../../util/icon-url' -export default function useInstanceList() { +export default function useInstanceList(userOnly = true, query = "") { const [errorState, setErrorState] = useState(false) @@ -36,7 +36,11 @@ export default function useInstanceList() { } const getWidgetInstances = ({ pageParam = 0}) => { - return apiGetWidgetInstances(pageParam) + if (userOnly) { + return apiGetWidgetInstances(pageParam) + } else { + return apiSearchWidgets(query, pageParam) + } } const { @@ -49,7 +53,7 @@ export default function useInstanceList() { status, refetch } = useInfiniteQuery({ - queryKey: ['widgets'], + queryKey: userOnly ? ['widgets'] : ['widgets', query], queryFn: getWidgetInstances, getNextPageParam: (lastPage, pages) => lastPage.next_page, refetchOnWindowFocus: false diff --git a/src/components/support-search.jsx b/src/components/support-search.jsx index 58718ab12..9350c3cb5 100644 --- a/src/components/support-search.jsx +++ b/src/components/support-search.jsx @@ -1,20 +1,17 @@ -import React, { useState } from 'react' +import React, { useState, useEffect } from 'react' import { iconUrl } from '../util/icon-url' -import { useQuery } from 'react-query' -import { apiSearchWidgets } from '../util/api' +import useInstanceList from './hooks/useInstanceList' import useDebounce from './hooks/useDebounce' const SupportSearch = ({onClick = () => {}}) => { const [searchText, setSearchText] = useState('') const [showDeleted, setShowDeleted] = useState(false) const debouncedSearchTerm = useDebounce(searchText, 500) - const { data: searchedWidgets, isFetching} = useQuery({ - queryKey: ['search-widgets', debouncedSearchTerm], - queryFn: () => apiSearchWidgets(debouncedSearchTerm), - enabled: !!debouncedSearchTerm && debouncedSearchTerm.length > 0, - placeholderData: null, - staleTime: Infinity - }) + const instanceList = useInstanceList(false, debouncedSearchTerm) + + useEffect(() => { + if (instanceList.error) console.log(instanceList.error) + }, [instanceList.instances]) const handleSearchChange = e => setSearchText(e.target.value) const handleShowDeletedClick = () => setShowDeleted(!showDeleted) @@ -24,16 +21,16 @@ const SupportSearch = ({onClick = () => {}}) => {

{`${searchText.length == 0 ? 'Search for a widget instance by entering its name or ID' : 'No widgets match your description'}`}

) - if ((isFetching || !searchedWidgets) && searchText.length > 0) { + if ((instanceList.isFetching || !instanceList.instances) && searchText.length > 0) { searchResultsRender = (
Searching Widget Instances ...
) - } else if (searchedWidgets && searchedWidgets.length !== 0) { + } else if (instanceList.instances && instanceList.instances.length !== 0) { searchResultsRender = (
- {searchedWidgets.map((match) => + {instanceList.instances.map((match) =>
{ * @returns {array} if matches were found * @returns {bool} if input does not match pattern */ -export const apiSearchWidgets = input => { +export const apiSearchWidgets = (input, page_number) => { let pattern = /[A-Za-z]+/g - if (!input.match(pattern).length) return false - return fetch(`/api/admin/widget_search/${input}`) + let match = input.match(pattern) + if (!match || !match.length) input = ' ' + return fetch(`/api/admin/widget_paginated_search/${input}/${page_number}`) .then(resp => { if (resp.status === 204 || resp.status === 502) return [] return resp.json() }) - .then(widgets => widgets) + .then(resp => { + writeToStorage('widgets', resp) + return resp + }) } export const apiGetWidgetsAdmin = () => { From 9c128bb284a762ce3c5ee36ca03acb4b832d98d5 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Thu, 19 Oct 2023 11:23:14 -0400 Subject: [PATCH 02/13] Increase pagination speeds and add loading icon --- fuel/app/classes/controller/api/admin.php | 4 +-- .../materia/widget/instance/manager.php | 29 +++++++++++---- src/components/hooks/useInstanceList.jsx | 10 ++++-- src/components/loading-icon.jsx | 4 +-- src/components/support-page.scss | 9 +++++ src/components/support-search.jsx | 36 +++++++++++++------ src/util/api.js | 4 +-- 7 files changed, 70 insertions(+), 26 deletions(-) diff --git a/fuel/app/classes/controller/api/admin.php b/fuel/app/classes/controller/api/admin.php index da13cdd33..c7a53ddac 100644 --- a/fuel/app/classes/controller/api/admin.php +++ b/fuel/app/classes/controller/api/admin.php @@ -69,7 +69,7 @@ public function post_user($user_id) return \Service_User::update_user($user_id, $user); } - public function get_widget_paginated_search(string $input, string $page_number) + public function get_widget_paginated_search(string $input, string $page_number, string $total_num_pages) { $input = trim($input); $input = urldecode($input); @@ -82,7 +82,7 @@ public function get_widget_paginated_search(string $input, string $page_number) 'next_page' => $page_number ]; } - return \Materia\Widget_Instance_Manager::get_paginated_search($input, $page_number); + return \Materia\Widget_Instance_Manager::get_paginated_search($input, $page_number, $total_num_pages); } public function get_extra_attempts(string $inst_id) diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index f23775947..e4643e266 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -135,22 +135,35 @@ public static function lock($inst_id) return $locked_by == $me; } - public static function get_paginated_search(string $input, $page_number = 0) + public static function get_paginated_search(string $input, $page_number = 0, $total_num_pages = -1) { - $displayable_insts = self::get_search($input); $widgets_per_page = 80; - $total_num_pages = ceil(sizeof($displayable_insts) / $widgets_per_page); $offset = $widgets_per_page * $page_number; - $has_next_page = $offset + $widgets_per_page < sizeof($displayable_insts) ? true : false; - // inst_ids corresponds to a single page's worth of instances - $displayable_insts = array_slice($displayable_insts, $offset, $widgets_per_page); + // if this is the first request, fetch all to get total num pages + if ($total_num_pages == -1) + { + $displayable_insts = self::get_search($input); + $total_num_pages = ceil(sizeof($displayable_insts) / $widgets_per_page); + + // inst_ids corresponds to a single page's worth of instances + $displayable_insts = array_slice($displayable_insts, $offset, $widgets_per_page); + } + else + { + // query DB for only a single page of instances + $displayable_insts = self::get_search($input, $offset, $widgets_per_page + 20); + } + + $has_next_page = $page_number < $total_num_pages ? true : false; $data = [ 'pagination' => $displayable_insts, ]; if ($has_next_page) $data['next_page'] = $page_number + 1; + // return the total num of pages to client + $data['total_num_pages'] = $total_num_pages; return $data; } @@ -162,13 +175,15 @@ public static function get_paginated_search(string $input, $page_number = 0) * * @return array of widget instances related to the given input */ - public static function get_search(string $input): array + public static function get_search(string $input, int $offset = 0, int $limit = 2147483647): array { $results = \DB::select() ->from('widget_instance') ->where('id', 'LIKE', "%$input%") ->or_where('name', 'LIKE', "%$input%") ->order_by('created_at', 'desc') + ->limit("$limit") + ->offset("$offset") ->execute() ->as_array(); diff --git a/src/components/hooks/useInstanceList.jsx b/src/components/hooks/useInstanceList.jsx index 5811154b4..ce55318bb 100644 --- a/src/components/hooks/useInstanceList.jsx +++ b/src/components/hooks/useInstanceList.jsx @@ -35,11 +35,13 @@ export default function useInstanceList(userOnly = true, query = "") { } else return [] } - const getWidgetInstances = ({ pageParam = 0}) => { + const getWidgetInstances = ({ pageParam = 0 }) => { + let totalPages = -1 + if (!!data) totalPages = data.pages[0].total_num_pages if (userOnly) { return apiGetWidgetInstances(pageParam) } else { - return apiSearchWidgets(query, pageParam) + return apiSearchWidgets(query, pageParam, totalPages) } } @@ -55,7 +57,9 @@ export default function useInstanceList(userOnly = true, query = "") { } = useInfiniteQuery({ queryKey: userOnly ? ['widgets'] : ['widgets', query], queryFn: getWidgetInstances, - getNextPageParam: (lastPage, pages) => lastPage.next_page, + getNextPageParam: (lastPage, pages) => { + return lastPage.next_page + }, refetchOnWindowFocus: false }) diff --git a/src/components/loading-icon.jsx b/src/components/loading-icon.jsx index 9085c5ea3..4a0ec1f98 100644 --- a/src/components/loading-icon.jsx +++ b/src/components/loading-icon.jsx @@ -1,10 +1,10 @@ import React from 'react' import './loading-icon.scss' -const LoadingIcon = ({size='med', width='100%', top= '0', left='0'}) => { +const LoadingIcon = ({size='med', width='100%', top= '0', left='0', position='absolute'}) => { // Supported sizes: sm, med, lrg return ( -
+
diff --git a/src/components/support-page.scss b/src/components/support-page.scss index eba3e84d0..b1a222276 100644 --- a/src/components/support-page.scss +++ b/src/components/support-page.scss @@ -48,6 +48,15 @@ // position: relative; // top: 20px; + .loading { + position: relative; + height: 30px; + + .loading-text { + margin-left: 50px; + } + } + .top { position: relative; top: 0; diff --git a/src/components/support-search.jsx b/src/components/support-search.jsx index 9350c3cb5..6fac4ad2b 100644 --- a/src/components/support-search.jsx +++ b/src/components/support-search.jsx @@ -2,6 +2,7 @@ import React, { useState, useEffect } from 'react' import { iconUrl } from '../util/icon-url' import useInstanceList from './hooks/useInstanceList' import useDebounce from './hooks/useDebounce' +import LoadingIcon from './loading-icon' const SupportSearch = ({onClick = () => {}}) => { const [searchText, setSearchText] = useState('') @@ -16,18 +17,32 @@ const SupportSearch = ({onClick = () => {}}) => { const handleSearchChange = e => setSearchText(e.target.value) const handleShowDeletedClick = () => setShowDeleted(!showDeleted) - let searchResultsRender = ( -
-

{`${searchText.length == 0 ? 'Search for a widget instance by entering its name or ID' : 'No widgets match your description'}`}

-
- ) + let searchPromptRender = null + let loadingRender = null + if ((instanceList.isFetching || !instanceList.instances) && searchText.length > 0) { - searchResultsRender = ( -
- Searching Widget Instances ... + loadingRender = ( +
+ +

Searching Widget Instances ...

) - } else if (instanceList.instances && instanceList.instances.length !== 0) { + } else if (instanceList.isFetching) { + loadingRender =
+ +

Loading widget instances...

+
+ } else { + searchPromptRender = ( +
+

{`${searchText.length == 0 ? 'Search for a widget instance by entering its name or ID' : 'No widgets match your description'}`}

+
+ ) + } + + let searchResultsRender = null + + if (instanceList.instances && instanceList.instances.length !== 0) { searchResultsRender = (
{instanceList.instances.map((match) => @@ -60,6 +75,7 @@ const SupportSearch = ({onClick = () => {}}) => {

Instance Admin

+ { searchPromptRender } {}}) => { Show Deleted Instances?
+ { loadingRender } { searchResultsRender } - ) } diff --git a/src/util/api.js b/src/util/api.js index 619821e8b..d2617fad2 100644 --- a/src/util/api.js +++ b/src/util/api.js @@ -377,11 +377,11 @@ export const apiGetWidgetLock = (id = null) => { * @returns {array} if matches were found * @returns {bool} if input does not match pattern */ -export const apiSearchWidgets = (input, page_number) => { +export const apiSearchWidgets = (input, page_number, total_num_pages = -1) => { let pattern = /[A-Za-z]+/g let match = input.match(pattern) if (!match || !match.length) input = ' ' - return fetch(`/api/admin/widget_paginated_search/${input}/${page_number}`) + return fetch(`/api/admin/widget_paginated_search/${input}/${page_number}/${total_num_pages}`) .then(resp => { if (resp.status === 204 || resp.status === 502) return [] return resp.json() From 7c1e4414f39e3133d2e9f29954edaa35294faa57 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Tue, 24 Oct 2023 16:21:05 -0400 Subject: [PATCH 03/13] Apply pagination rules to all components using apiSearchUsers and fix image size of user avatars in search results --- fuel/app/classes/controller/api/admin.php | 4 +- fuel/app/classes/materia/api/v1.php | 37 +++-- .../materia/widget/instance/manager.php | 69 +++++---- fuel/app/classes/model/user.php | 14 +- fuel/app/tests/api/v1.php | 21 +-- src/components/extra-attempts-dialog.scss | 5 +- src/components/hooks/useInstanceList.jsx | 4 +- src/components/hooks/useUserList.jsx | 66 ++++++++ .../my-widgets-collaborate-dialog.jsx | 39 ++--- src/components/student-search.jsx | 15 +- src/components/support-page.scss | 6 +- src/components/support-search.jsx | 14 +- src/components/user-admin-page.scss | 145 +++++++++--------- src/components/user-admin-search.jsx | 44 ++++-- src/util/api.js | 13 +- 15 files changed, 300 insertions(+), 196 deletions(-) create mode 100644 src/components/hooks/useUserList.jsx diff --git a/fuel/app/classes/controller/api/admin.php b/fuel/app/classes/controller/api/admin.php index c7a53ddac..43150c011 100644 --- a/fuel/app/classes/controller/api/admin.php +++ b/fuel/app/classes/controller/api/admin.php @@ -69,7 +69,7 @@ public function post_user($user_id) return \Service_User::update_user($user_id, $user); } - public function get_widget_paginated_search(string $input, string $page_number, string $total_num_pages) + public function get_widget_paginated_search(string $input, string $page_number) { $input = trim($input); $input = urldecode($input); @@ -82,7 +82,7 @@ public function get_widget_paginated_search(string $input, string $page_number, 'next_page' => $page_number ]; } - return \Materia\Widget_Instance_Manager::get_paginated_search($input, $page_number, $total_num_pages); + return \Materia\Widget_Instance_Manager::get_paginated_instance_search($input, $page_number); } public function get_extra_attempts(string $inst_id) diff --git a/fuel/app/classes/materia/api/v1.php b/fuel/app/classes/materia/api/v1.php index 509e66ab8..179d565a3 100644 --- a/fuel/app/classes/materia/api/v1.php +++ b/fuel/app/classes/materia/api/v1.php @@ -67,7 +67,7 @@ static public function widget_instances_get($inst_ids = null, bool $deleted = fa static public function widget_paginate_instances_get($page_number = 0) { if (\Service_User::verify_session() !== true) return Msg::no_login(); - $data = Widget_Instance_Manager::get_paginated_for_user(\Model_User::find_current_id(), $page_number); + $data = Widget_Instance_Manager::get_paginated_instances_for_user(\Model_User::find_current_id(), $page_number); return $data; } @@ -881,23 +881,40 @@ static public function semester_date_ranges_get() return Utils::get_date_ranges(); } - static public function users_search($search) + /** + * Paginated search for users that match input + * + * @param string Search query + * @param string Page number + * @return array List of users + */ + static public function users_search($input, $page_number = 0) { if (\Service_User::verify_session() !== true) return Msg::no_login(); - $user_objects = \Model_User::find_by_name_search($search); - $user_arrays = []; + $items_per_page = 50; + $offset = $items_per_page * $page_number; + + // query DB for only a single page + 1 item + $displayable_items = \Model_User::find_by_name_search($input, $offset, $items_per_page + 1); + + $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; // scrub the user models with to_array - if (count($user_objects)) + if ($has_next_page) array_pop($displayable_items); + foreach ($displayable_items as $key => $person) { - foreach ($user_objects as $key => $person) - { - $user_arrays[$key] = $person->to_array(); - } + $displayable_items[$key] = $person->to_array(); } - return $user_arrays; + $data = [ + 'pagination' => $displayable_items, + ]; + + if ($has_next_page) $data['next_page'] = $page_number + 1; + else $data['next_page'] = $page_number; + + return $data; } /** * Gets information about the current user diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index e4643e266..fb9d3614c 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -14,7 +14,7 @@ static public function get($inst_id, $load_qset=false, $timestamp=false, $delete return count($instances) > 0 ? $instances[0] : false; } - static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=false, bool $deleted=false): array + static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=false, bool $deleted=false, $offset=0, $limit=2147483647): array { if ( ! is_array($inst_ids) || count($inst_ids) < 1) return []; @@ -27,6 +27,8 @@ static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=fal ->where('id', 'IN', $inst_ids) ->and_where('is_deleted', '=', $deleted ? '1' : '0') ->order_by('created_at', 'desc') + ->offset("$offset") + ->limit("$limit") ->execute() ->as_array(); @@ -76,20 +78,25 @@ public static function get_all_for_user($user_id, $load_qset=false) * * @return array of widget instances that are visible to the user. */ - public static function get_paginated_for_user($user_id, $page_number = 0) + public static function get_paginated_instances_for_user($user_id, $page_number = 0) { $inst_ids = Perm_Manager::get_all_objects_for_user($user_id, Perm::INSTANCE, [Perm::FULL, Perm::VISIBLE]); - $displayable_inst = self::get_all($inst_ids); - $widgets_per_page = 80; - $total_num_pages = ceil(sizeof($displayable_inst) / $widgets_per_page); - $offset = $widgets_per_page * $page_number; - $has_next_page = $offset + $widgets_per_page < sizeof($displayable_inst) ? true : false; - // inst_ids corresponds to a single page's worth of instances - $displayable_inst = array_slice($displayable_inst, $offset, $widgets_per_page); + $items_per_page = 80; + $offset = $items_per_page * $page_number; + + // query DB for only a single page of instances + 1 + $displayable_items = self::get_all($inst_ids, false, false, false, $offset, $items_per_page + 1); + \Log::Error(count($displayable_items)); + + // if the returned number of instances is greater than a page, there's more pages + $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; + + // remove last instance if there are multiple pages + if ($has_next_page) array_pop($displayable_items); $data = [ - 'pagination' => $displayable_inst, + 'pagination' => $displayable_items, ]; if ($has_next_page) $data['next_page'] = $page_number + 1; @@ -135,35 +142,33 @@ public static function lock($inst_id) return $locked_by == $me; } - public static function get_paginated_search(string $input, $page_number = 0, $total_num_pages = -1) + /** + * Widget instance paginated search results + * + * @param input search input + * @param page_number page number + * + * @return array of items related to the given input + */ + public static function get_paginated_instance_search(string $input, $page_number = 0) { - $widgets_per_page = 80; - $offset = $widgets_per_page * $page_number; + $items_per_page = 80; + $offset = $items_per_page * $page_number; - // if this is the first request, fetch all to get total num pages - if ($total_num_pages == -1) - { - $displayable_insts = self::get_search($input); - $total_num_pages = ceil(sizeof($displayable_insts) / $widgets_per_page); + // query DB for only a single page of instances + 1 + $displayable_items = self::get_widget_instance_search($input, $offset, $items_per_page + 1); - // inst_ids corresponds to a single page's worth of instances - $displayable_insts = array_slice($displayable_insts, $offset, $widgets_per_page); - } - else - { - // query DB for only a single page of instances - $displayable_insts = self::get_search($input, $offset, $widgets_per_page + 20); - } + // if the returned number of instances is greater than a page, there's more pages + $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; - $has_next_page = $page_number < $total_num_pages ? true : false; + // remove last instance if there are multiple pages + if ($has_next_page) array_pop($displayable_items); $data = [ - 'pagination' => $displayable_insts, + 'pagination' => $displayable_items, ]; if ($has_next_page) $data['next_page'] = $page_number + 1; - // return the total num of pages to client - $data['total_num_pages'] = $total_num_pages; return $data; } @@ -172,10 +177,12 @@ public static function get_paginated_search(string $input, $page_number = 0, $to * Gets all widget instances related to a given input, including id or name. * * @param input search input + * @param offset start search at this row in results + * @param limit number of rows to include * * @return array of widget instances related to the given input */ - public static function get_search(string $input, int $offset = 0, int $limit = 2147483647): array + public static function get_widget_instance_search(string $input, int $offset = 0, int $limit = 2147483647): array { $results = \DB::select() ->from('widget_instance') diff --git a/fuel/app/classes/model/user.php b/fuel/app/classes/model/user.php index c29874777..9419d1c91 100644 --- a/fuel/app/classes/model/user.php +++ b/fuel/app/classes/model/user.php @@ -87,7 +87,7 @@ public static function find_by_username($username) ->get_one(); } - static public function find_by_name_search($name) + static public function find_by_name_search($name, $offset = 0, $limit = 50) { $name = preg_replace('/\s+/', '', $name); // remove spaces @@ -108,11 +108,19 @@ static public function find_by_name_search($name) ->or_where(\DB::expr('REPLACE(CONCAT(first, last), " ", "")'), 'LIKE', "%$name%") ->or_where('email', 'LIKE', "$name%") ->and_where_close() - ->limit(50) + ->offset($offset) + ->limit($limit) ->as_object('Model_User') ->execute(); - return $matches; + // convert object to array + $list = []; + foreach ($matches as $match) + { + $list[] = $match; + } + + return $list; } public static function validate($factory) diff --git a/fuel/app/tests/api/v1.php b/fuel/app/tests/api/v1.php index 42b524c6d..d6b905426 100644 --- a/fuel/app/tests/api/v1.php +++ b/fuel/app/tests/api/v1.php @@ -1523,9 +1523,10 @@ public function test_users_search_as_student() $output = Api_V1::users_search('droptables'); $this->assertIsArray($output); $this->assertCount(2, $output); - $this->assert_is_user_array($output[0]); - $this->assertFalse(array_key_exists('password', $output)); - $this->assertFalse(array_key_exists('login_hash', $output)); + $this->assertIsArray($output['pagination']); + $this->assert_is_user_array($output['pagination'][0]); + $this->assertFalse(array_key_exists('password', $output['pagination'])); + $this->assertFalse(array_key_exists('login_hash', $output['pagination'])); } public function test_users_search_as_author() @@ -1539,9 +1540,10 @@ public function test_users_search_as_author() $output = Api_V1::users_search('droptables'); $this->assertIsArray($output); $this->assertCount(2, $output); - $this->assert_is_user_array($output[0]); - $this->assertFalse(array_key_exists('password', $output)); - $this->assertFalse(array_key_exists('login_hash', $output)); + $this->assertIsArray($output['pagination']); + $this->assert_is_user_array($output['pagination'][0]); + $this->assertFalse(array_key_exists('password', $output['pagination'])); + $this->assertFalse(array_key_exists('login_hash', $output['pagination'])); } public function test_users_search_as_super_user() @@ -1555,9 +1557,10 @@ public function test_users_search_as_super_user() $output = Api_V1::users_search('droptables'); $this->assertIsArray($output); $this->assertCount(2, $output); - $this->assert_is_user_array($output[0]); - $this->assertFalse(array_key_exists('password', $output[0])); - $this->assertFalse(array_key_exists('login_hash', $output[0])); + $this->assertIsArray($output['pagination']); + $this->assert_is_user_array($output['pagination'][0]); + $this->assertFalse(array_key_exists('password', $output['pagination'])); + $this->assertFalse(array_key_exists('login_hash', $output['pagination'])); } protected function assert_is_semester_rage($semester) diff --git a/src/components/extra-attempts-dialog.scss b/src/components/extra-attempts-dialog.scss index 28fc47225..7f8482b4b 100644 --- a/src/components/extra-attempts-dialog.scss +++ b/src/components/extra-attempts-dialog.scss @@ -37,7 +37,7 @@ } .attempts_search_list { - width: 447px; + width: 449px; position: absolute; background-color: #ffffff; border: #bfbfbf 1px solid; @@ -45,7 +45,8 @@ overflow: auto; z-index: 3; text-align: left; - left: 114px; + left: 121px; + top: 111px; display: flex; flex-wrap: wrap; align-items: flex-start; diff --git a/src/components/hooks/useInstanceList.jsx b/src/components/hooks/useInstanceList.jsx index ce55318bb..56a83a0be 100644 --- a/src/components/hooks/useInstanceList.jsx +++ b/src/components/hooks/useInstanceList.jsx @@ -36,12 +36,10 @@ export default function useInstanceList(userOnly = true, query = "") { } const getWidgetInstances = ({ pageParam = 0 }) => { - let totalPages = -1 - if (!!data) totalPages = data.pages[0].total_num_pages if (userOnly) { return apiGetWidgetInstances(pageParam) } else { - return apiSearchWidgets(query, pageParam, totalPages) + return apiSearchWidgets(query, pageParam) } } diff --git a/src/components/hooks/useUserList.jsx b/src/components/hooks/useUserList.jsx new file mode 100644 index 000000000..b9691b455 --- /dev/null +++ b/src/components/hooks/useUserList.jsx @@ -0,0 +1,66 @@ +import { useState, useEffect, useMemo } from 'react' +import { useInfiniteQuery } from 'react-query' +import { apiSearchUsers } from '../../util/api' + +export default function useUserList(query = "") { + + const [errorState, setErrorState] = useState(false) + + // this creates a flat list of users from the paginated list + const formatData = (list) => { + if (list?.type == 'error') { + console.error(`Users failed to load with error: ${list.msg}`); + setErrorState(true) + return [] + } + if (list?.pages) { + let dataMap = [] + list.pages.forEach(page => { + dataMap.push(...page.pagination) + }) + return dataMap + } + + return [] + } + + const getData = ({ pageParam = 0 }) => { + return apiSearchUsers(query, pageParam) + } + + const { + data, + error, + fetchNextPage, + hasNextPage, + isFetching, + isFetchingNextPage, + status, + refetch + } = useInfiniteQuery({ + queryKey: ['users', query], + queryFn: getData, + getNextPageParam: (lastPage, pages) => { + return lastPage.next_page + }, + refetchOnWindowFocus: false + }) + + useEffect(() => { + if (error != null && error != undefined) setErrorState(true) + },[error]) + + // memoize the user list since this is a large, expensive query + const users = useMemo(() => formatData(data), [data]) + + useEffect(() => { + if (hasNextPage) fetchNextPage() + },[users]) + + return { + users: users, + isFetching: isFetching || hasNextPage, + refresh: () => refetch(), + ...(errorState == true ? {error: true} : {}) // the error value is only provided if errorState is true + } +} diff --git a/src/components/my-widgets-collaborate-dialog.jsx b/src/components/my-widgets-collaborate-dialog.jsx index 560e63062..b71212f0f 100644 --- a/src/components/my-widgets-collaborate-dialog.jsx +++ b/src/components/my-widgets-collaborate-dialog.jsx @@ -1,6 +1,6 @@ import React, { useEffect, useState, useRef, useMemo } from 'react' import { useQuery, useQueryClient } from 'react-query' -import { apiGetUsers, apiSearchUsers } from '../util/api' +import { apiGetUsers } from '../util/api' import setUserInstancePerms from './hooks/useSetUserInstancePerms' import Modal from './modal' import useDebounce from './hooks/useDebounce' @@ -9,6 +9,7 @@ import NoContentIcon from './no-content-icon' import CollaborateUserRow from './my-widgets-collaborate-user-row' import './my-widgets-collaborate-dialog.scss' import { access } from './materia-constants' +import useUserList from './hooks/useUserList' const initDialogState = (state) => { return ({ @@ -25,6 +26,8 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set const setUserPerms = setUserInstancePerms() const mounted = useRef(false) const popperRef = useRef(null) + const userList = useUserList(debouncedSearchTerm) + const { data: collabUsers, remove: clearUsers, isFetching} = useQuery({ queryKey: ['collab-users', inst.id, (otherUserPerms != null ? Array.from(otherUserPerms.keys()) : otherUserPerms)], // check for changes in otherUserPerms enabled: !!otherUserPerms, @@ -33,26 +36,15 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set placeholderData: {} }) - const { data: searchResults, remove: clearSearch, refetch: refetchSearch } = useQuery({ - queryKey: 'user-search', - enabled: !!debouncedSearchTerm, - queryFn: () => apiSearchUsers(debouncedSearchTerm), - staleTime: Infinity, - placeholderData: [], - retry: false, - onSuccess: (data) => { - if (data && data.type == 'error') + useEffect(() => { + if (userList.error) { + console.error(`User search failed with error: ${data.msg}`); + if (userList.error.title == "Invalid Login") { - console.error(`User search failed with error: ${data.msg}`); - if (data.title == "Invalid Login") - { - setInvalidLogin(true) - } - } else if (!data) { - console.error(`User search failed.`); + setInvalidLogin(true) } } - }) + }, [userList.error]) useEffect(() => { mounted.current = true @@ -61,12 +53,6 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set } }, []) - // Handles the search with debounce - useEffect(() => { - if(debouncedSearchTerm === '') clearSearch() - else refetchSearch() - }, [debouncedSearchTerm]) - // updatedAllUserPerms is assigned the value of otherUserPerms (a read-only prop) when the component loads useEffect(() => { if (otherUserPerms != null) @@ -205,7 +191,6 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set const customClose = () => { clearUsers() - clearSearch() onClose() } @@ -219,8 +204,8 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set let searchContainerRender = null if (myPerms?.shareable || myPerms?.isSupportUser) { let searchResultsRender = null - if (debouncedSearchTerm !== '' && state.searchText !== '' && searchResults.length && searchResults?.length !== 0) { - const searchResultElements = searchResults?.map(match => + if (debouncedSearchTerm !== '' && state.searchText !== '' && userList.users?.length && userList.users?.length !== 0) { + const searchResultElements = userList.users?.map(match =>
onClickMatch(match)}> diff --git a/src/components/student-search.jsx b/src/components/student-search.jsx index 5122fa312..783bdc07f 100644 --- a/src/components/student-search.jsx +++ b/src/components/student-search.jsx @@ -1,7 +1,6 @@ import React, {useState, useEffect} from 'react' -import { useQuery } from 'react-query' -import { apiSearchUsers } from '../util/api' import useDebounce from './hooks/useDebounce' +import useUserList from './hooks/useUserList' const initState = () => ({ searchText: '', @@ -11,13 +10,7 @@ const initState = () => ({ const StudentSearch = ({addUser, debounceTime=300}) => { const [state, setState] = useState(initState()) const debouncedSearchTerm = useDebounce(state.searchText, debounceTime) - const { data: studentsSearched } = useQuery({ - queryKey: ['student-search',debouncedSearchTerm], - queryFn: () => apiSearchUsers(debouncedSearchTerm), - placeholderData: [], - enabled: !!debouncedSearchTerm && debouncedSearchTerm.length > 0, - staleTime: Infinity - }) + const userList = useUserList(debouncedSearchTerm) // Handles closign the search window immediately on click without debounce delay useEffect(() => { @@ -30,8 +23,8 @@ const StudentSearch = ({addUser, debounceTime=300}) => { } let searchMatchElementsRender = null - if (!state.clicked && studentsSearched && studentsSearched.filter(res => res.is_student === true).length !== 0) { - const searchMatchElements = studentsSearched.filter(res => res.is_student === true).map(match => ( + if (!state.clicked && userList.users && userList.users.filter(res => res.is_student === true).length !== 0) { + const searchMatchElements = userList.users.filter(res => res.is_student === true).map(match => (
onClickMatch(match)}> diff --git a/src/components/support-page.scss b/src/components/support-page.scss index b1a222276..eea7ac416 100644 --- a/src/components/support-page.scss +++ b/src/components/support-page.scss @@ -50,7 +50,11 @@ .loading { position: relative; - height: 30px; + height: 40px; + display: flex; + flex-direction: row; + align-items: center; + margin: 5px 0; .loading-text { margin-left: 50px; diff --git a/src/components/support-search.jsx b/src/components/support-search.jsx index 6fac4ad2b..52efcb10e 100644 --- a/src/components/support-search.jsx +++ b/src/components/support-search.jsx @@ -17,9 +17,7 @@ const SupportSearch = ({onClick = () => {}}) => { const handleSearchChange = e => setSearchText(e.target.value) const handleShowDeletedClick = () => setShowDeleted(!showDeleted) - let searchPromptRender = null let loadingRender = null - if ((instanceList.isFetching || !instanceList.instances) && searchText.length > 0) { loadingRender = (
@@ -32,14 +30,14 @@ const SupportSearch = ({onClick = () => {}}) => {

Loading widget instances...

- } else { - searchPromptRender = ( -
-

{`${searchText.length == 0 ? 'Search for a widget instance by entering its name or ID' : 'No widgets match your description'}`}

-
- ) } + let searchPromptRender = ( +
+

{`${searchText.length == 0 || (instanceList.instances && instanceList.instances.length > 0) || instanceList.isFetching ? 'Search for a widget instance by entering its name or ID' : 'No widgets match your description'}`}

+
+ ) + let searchResultsRender = null if (instanceList.instances && instanceList.instances.length !== 0) { diff --git a/src/components/user-admin-page.scss b/src/components/user-admin-page.scss index dd626469b..42205bd32 100644 --- a/src/components/user-admin-page.scss +++ b/src/components/user-admin-page.scss @@ -13,6 +13,19 @@ .page { + .loading { + position: relative; + height: 40px; + display: flex; + flex-direction: row; + align-items: center; + margin: 5px 0; + + .loading-text { + margin-left: 50px; + } + } + #breadcrumb-container { display: inline-block; position: absolute; @@ -60,12 +73,12 @@ } .search { - margin: 10px 0 10px 10px; + margin: 10px 0 10px 15px; input.user_search { width: 60%; height: 30px; - margin: 10px 5px; + margin: 10px 0; padding: 5px; border-radius: 5px; @@ -88,6 +101,11 @@ > div { display: inline-block; } + + img { + width: 50px; + height: 50px; + } } .searching { @@ -197,6 +215,57 @@ vertical-align: top; width: 160px; } + + .radio { + display: inline-block; + // width: 200px; + + input { + margin: 0px 5px; + } + + label { + width: auto; + } + } + + .url { + display: inline-block; + vertical-align: middle; + // width: 500px; + font-size: 70%; + margin: 0px 0px; + } + + .right-justify { + display: flex; + justify-content: flex-end; + margin-top: 20px; + + .apply-changes { + display: flex; + flex-direction: column; + justify-content: flex-end; + width: 220px; + margin-right: 20px; + + .apply { + padding: 6px 10px 6px 10px; + } + + .error-text { + color: red; + font-size: 0.8em; + text-align: center; + } + + .success-text { + color: green; + font-size: 0.8em; + text-align: center; + } + } + } } &.role-manager { @@ -314,76 +383,14 @@ .info-holder { position: relative; display: block; + padding: 15px; + background: $very-light-gray; + border-bottom-left-radius: 3px; + border-bottom-right-radius: 3px; - padding: 10px; - - span:not(.long), - label { - display: inline-block; - vertical-align: top; - } - - label:not(.normal) { - width: 145px; - } - - .normal input { - margin: 0; - padding: 0; - } - } - .overview { - padding: 10px; - - div { - margin: 5px 0px; - } - - label { - display: inline-block; - vertical-align: top; - width: 150px; - } - - .url { - display: inline-block; - vertical-align: middle; - width: 500px; - font-size: 70%; - margin: 0px 0px; - } - - .right-justify { - margin-top: 20px; - display: flex; - justify-content: flex-end; - - .apply-changes { - display: flex; - flex-direction: column; - justify-content: flex-end; - width: 220px; - - .apply { - padding: 6px 10px 6px 10px; - } - - .error-text { - color: red; - font-size: 0.8em; - text-align: center; - } - - .success-text { - color: green; - font-size: 0.8em; - text-align: center; - } - } - } + line-height: 1.5em; } } - } } } diff --git a/src/components/user-admin-search.jsx b/src/components/user-admin-search.jsx index 99184151d..38200a9b7 100644 --- a/src/components/user-admin-search.jsx +++ b/src/components/user-admin-search.jsx @@ -1,25 +1,18 @@ import React, { useState } from 'react' -import { iconUrl } from '../util/icon-url' -import { useQuery } from 'react-query' -import { apiSearchUsers } from '../util/api' import useDebounce from './hooks/useDebounce' +import useUserList from './hooks/useUserList' +import LoadingIcon from './loading-icon' const UserAdminSearch = ({onClick = () => {}}) => { const [searchText, setSearchText] = useState('') - // const [showDeleted, setShowDeleted] = useState(false) const debouncedSearchTerm = useDebounce(searchText, 500) - const { data: searchedUsers, isFetching} = useQuery({ - queryKey: ['search-users', debouncedSearchTerm], - queryFn: () => apiSearchUsers(debouncedSearchTerm), - enabled: !!debouncedSearchTerm && debouncedSearchTerm.length > 0, - placeholderData: null, - staleTime: Infinity - }) + const userList = useUserList(debouncedSearchTerm) - const userSearchList = searchedUsers?.map((user, index) => { + const userSearchList = userList.users?.map((user, index) => { return (
onClick(user)}> + className="search_match clickable" key={index} + onClick={() => onClick(user)}>
@@ -30,12 +23,36 @@ const UserAdminSearch = ({onClick = () => {}}) => { ) }) + let loadingRender = null + if ((userList.isFetching || !userList.users) && searchText.length > 0) { + loadingRender = ( +
+ +

Searching Users ...

+
+ ) + } else if (userList.isFetching) { + loadingRender =
+ +

Loading users...

+
+ } + + let searchPromptRender = ( +
+

{`${searchText.length == 0 || (userList.users && userList.users.length > 0) + || userList.isFetching ? 'Search for a user by entering their name' + : 'No users match your description'}`}

+
+ ) + return (

User Admin

+ { searchPromptRender } {}}) => { type="text" placeholder="Enter a Materia user's name or email address"/>
+ { loadingRender }
{ userSearchList }
diff --git a/src/util/api.js b/src/util/api.js index abd1b1dd0..bad56b5f3 100644 --- a/src/util/api.js +++ b/src/util/api.js @@ -316,8 +316,8 @@ export const apiSetAttempts = ({ instId, attempts }) => { }) } -export const apiSearchUsers = (input = '') => { - return fetch('/api/json/users_search', fetchOptions({ body: `data=${formatFetchBody([input])}` })) +export const apiSearchUsers = (input = '', page_number = 0) => { + return fetch('/api/json/users_search', fetchOptions({ body: `data=${formatFetchBody([input, page_number])}` })) .then(resp => { if (resp.status === 204 || resp.status === 502) return [] return resp.json() @@ -377,15 +377,14 @@ export const apiGetWidgetLock = (id = null) => { /** * It searches for widgets by name or ID - * @param {string} input (must contain letters) - * @returns {array} if matches were found - * @returns {bool} if input does not match pattern + * @param {string} input (letters only) + * @returns {array} of matches */ -export const apiSearchWidgets = (input, page_number, total_num_pages = -1) => { +export const apiSearchWidgets = (input, page_number) => { let pattern = /[A-Za-z]+/g let match = input.match(pattern) if (!match || !match.length) input = ' ' - return fetch(`/api/admin/widget_paginated_search/${input}/${page_number}/${total_num_pages}`) + return fetch(`/api/admin/widget_paginated_search/${input}/${page_number}`) .then(resp => { if (resp.status === 204 || resp.status === 502) return [] return resp.json() From afeebcacb0104785d91197cce92dd3d213f59e29 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Tue, 24 Oct 2023 16:23:21 -0400 Subject: [PATCH 04/13] Update tests for search user --- fuel/app/tests/model/user.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fuel/app/tests/model/user.php b/fuel/app/tests/model/user.php index 4cd42d0c4..7d7c13c2e 100644 --- a/fuel/app/tests/model/user.php +++ b/fuel/app/tests/model/user.php @@ -15,12 +15,12 @@ public function test_find_by_name_search_doesnt_find_super_users() { // su should't be found $su = $this->make_random_super_user(); - $x = Model_User::find_by_name_search($su->email)->as_array(); + $x = Model_User::find_by_name_search($su->email); self::assertEmpty($x); // add a student with the same name, should only find the one student $this->make_random_student(); - $x = Model_User::find_by_name_search('drop')->as_array(); + $x = Model_User::find_by_name_search('drop'); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertNotEquals($su->id, $x[0]->id); @@ -30,7 +30,7 @@ public function test_find_by_name_search_finds_students_by_email() { $user = $this->make_random_student(); - $x = Model_User::find_by_name_search($user->email)->as_array(); + $x = Model_User::find_by_name_search($user->email); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertEquals($user->id, $x[0]->id); @@ -40,7 +40,7 @@ public function test_find_by_name_search_finds_students_by_first_name() { $user = $this->make_random_student(); - $x = Model_User::find_by_name_search($user->first)->as_array(); + $x = Model_User::find_by_name_search($user->first); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertEquals($user->id, $x[0]->id); @@ -50,7 +50,7 @@ public function test_find_by_name_search_finds_students_by_last_name() { $user = $this->make_random_student(); - $x = Model_User::find_by_name_search($user->last)->as_array(); + $x = Model_User::find_by_name_search($user->last); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertEquals($user->id, $x[0]->id); @@ -60,7 +60,7 @@ public function test_find_by_name_search_finds_students_by_username() { $user = $this->make_random_student(); - $x = Model_User::find_by_name_search($user->username)->as_array(); + $x = Model_User::find_by_name_search($user->username); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertEquals($user->id, $x[0]->id); @@ -70,7 +70,7 @@ public function test_find_by_name_search_finds_students() { $user = $this->make_random_student(); - $x = Model_User::find_by_name_search($user->email)->as_array(); + $x = Model_User::find_by_name_search($user->email); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertEquals($user->id, $x[0]->id); @@ -79,7 +79,7 @@ public function test_find_by_name_search_finds_students() public function test_find_by_name_search_finds_authors() { $user = $this->make_random_author(); - $x = Model_User::find_by_name_search($user->email)->as_array(); + $x = Model_User::find_by_name_search($user->email); self::assertCount(1, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertEquals($user->id, $x[0]->id); @@ -90,7 +90,7 @@ public function test_find_by_name_search_finds_multiple_matches() $user1 = $this->make_random_author(); $user2 = $this->make_random_student(); - $x = Model_User::find_by_name_search('drop')->as_array(); + $x = Model_User::find_by_name_search('drop'); self::assertCount(2, $x); self::assertInstanceOf('Model_User', $x[0]); self::assertInstanceOf('Model_User', $x[1]); From 6d754c914e66b6152956dbfc2cd33ac733122639 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Tue, 24 Oct 2023 16:30:55 -0400 Subject: [PATCH 05/13] Remove log --- fuel/app/classes/materia/widget/instance/manager.php | 1 - 1 file changed, 1 deletion(-) diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index fb9d3614c..b3067ef86 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -87,7 +87,6 @@ public static function get_paginated_instances_for_user($user_id, $page_number = // query DB for only a single page of instances + 1 $displayable_items = self::get_all($inst_ids, false, false, false, $offset, $items_per_page + 1); - \Log::Error(count($displayable_items)); // if the returned number of instances is greater than a page, there's more pages $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; From 5a81bffd8f68a7ad21a3c8deca5b9cb27d15ba0a Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Tue, 24 Oct 2023 17:06:23 -0400 Subject: [PATCH 06/13] User search fix and collaborator results scroll --- docker/run_create_me.sh | 13 +++++++++++++ fuel/app/classes/materia/api/v1.php | 1 - fuel/app/tests/api/v1.php | 3 --- src/components/my-widgets-collaborate-dialog.jsx | 4 +--- src/components/my-widgets-collaborate-dialog.scss | 13 +++++++------ 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/docker/run_create_me.sh b/docker/run_create_me.sh index 3fffc3d05..a82cdafd5 100755 --- a/docker/run_create_me.sh +++ b/docker/run_create_me.sh @@ -15,5 +15,18 @@ PASS=${MATERIA_DEV_PASS:-kogneato} # create or update the user and pw docker-compose run --rm app bash -c "php oil r admin:new_user $USER $USER M Lastname $USER@mail.com $PASS || php oil r admin:reset_password $USER $PASS" +for n in {1..100}; +do + docker-compose run --rm app bash -c "php oil r admin:new_user $USER-$n $USER-$n M Lastname $USER-$n@mail.com $PASS || php oil r admin:reset_password $USER-$n $PASS" + docker-compose run --rm app bash -c "php oil r admin:give_user_role $USER-$n super_user || true && php oil r admin:give_user_role $USER-$n basic_author" +done + +for n in {1..20}; +do + docker-compose run --rm app bash -c "php oil r admin:new_user student-$n student-$n M Lastname student-$n@mail.com $PASS || php oil r admin:reset_password student-$n $PASS" + docker-compose run --rm app bash -c "php oil r admin:give_user_role student-$n basic_author" + break +done + # give them super_user and basic_author docker-compose run --rm app bash -c "php oil r admin:give_user_role $USER super_user || true && php oil r admin:give_user_role $USER basic_author" diff --git a/fuel/app/classes/materia/api/v1.php b/fuel/app/classes/materia/api/v1.php index 179d565a3..b557b57c5 100644 --- a/fuel/app/classes/materia/api/v1.php +++ b/fuel/app/classes/materia/api/v1.php @@ -912,7 +912,6 @@ static public function users_search($input, $page_number = 0) ]; if ($has_next_page) $data['next_page'] = $page_number + 1; - else $data['next_page'] = $page_number; return $data; } diff --git a/fuel/app/tests/api/v1.php b/fuel/app/tests/api/v1.php index d6b905426..a4c336dd7 100644 --- a/fuel/app/tests/api/v1.php +++ b/fuel/app/tests/api/v1.php @@ -1522,7 +1522,6 @@ public function test_users_search_as_student() $output = Api_V1::users_search('droptables'); $this->assertIsArray($output); - $this->assertCount(2, $output); $this->assertIsArray($output['pagination']); $this->assert_is_user_array($output['pagination'][0]); $this->assertFalse(array_key_exists('password', $output['pagination'])); @@ -1539,7 +1538,6 @@ public function test_users_search_as_author() $output = Api_V1::users_search('droptables'); $this->assertIsArray($output); - $this->assertCount(2, $output); $this->assertIsArray($output['pagination']); $this->assert_is_user_array($output['pagination'][0]); $this->assertFalse(array_key_exists('password', $output['pagination'])); @@ -1556,7 +1554,6 @@ public function test_users_search_as_super_user() $output = Api_V1::users_search('droptables'); $this->assertIsArray($output); - $this->assertCount(2, $output); $this->assertIsArray($output['pagination']); $this->assert_is_user_array($output['pagination'][0]); $this->assertFalse(array_key_exists('password', $output['pagination'])); diff --git a/src/components/my-widgets-collaborate-dialog.jsx b/src/components/my-widgets-collaborate-dialog.jsx index b71212f0f..1d6c5d131 100644 --- a/src/components/my-widgets-collaborate-dialog.jsx +++ b/src/components/my-widgets-collaborate-dialog.jsx @@ -235,9 +235,7 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set className='user-add' type='text' placeholder="Enter a Materia user's name or e-mail"/> -
- { searchResultsRender } -
+ { searchResultsRender }
) } diff --git a/src/components/my-widgets-collaborate-dialog.scss b/src/components/my-widgets-collaborate-dialog.scss index 49c664af7..816d75f75 100644 --- a/src/components/my-widgets-collaborate-dialog.scss +++ b/src/components/my-widgets-collaborate-dialog.scss @@ -16,7 +16,7 @@ position: relative; text-align: left; display: block; - + font-weight: bold; } @@ -67,15 +67,16 @@ flex-wrap: wrap; align-items: flex-start; - top: 32px; + top: 34px; right: 0px; - width: 447px; + width: 449px; + max-height: 300px; padding-bottom: 5px; overflow: auto; - + background-color: #ffffff; border: #bfbfbf 1px solid; - + text-align: left; .collab-search-match { @@ -101,7 +102,7 @@ margin: 5px 0 0 5px; font-size: 14px; text-align: left; - + font-family: 'Lucida Grande', sans-serif; } } From ac422c9e3c2cf78e455fa82847c7ac886b13e66a Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Tue, 24 Oct 2023 17:09:14 -0400 Subject: [PATCH 07/13] Remove user tests --- docker/run_create_me.sh | 13 ------ .../my-widgets-collaborate-dialog.jsx | 46 +++++++++---------- 2 files changed, 22 insertions(+), 37 deletions(-) diff --git a/docker/run_create_me.sh b/docker/run_create_me.sh index a82cdafd5..3fffc3d05 100755 --- a/docker/run_create_me.sh +++ b/docker/run_create_me.sh @@ -15,18 +15,5 @@ PASS=${MATERIA_DEV_PASS:-kogneato} # create or update the user and pw docker-compose run --rm app bash -c "php oil r admin:new_user $USER $USER M Lastname $USER@mail.com $PASS || php oil r admin:reset_password $USER $PASS" -for n in {1..100}; -do - docker-compose run --rm app bash -c "php oil r admin:new_user $USER-$n $USER-$n M Lastname $USER-$n@mail.com $PASS || php oil r admin:reset_password $USER-$n $PASS" - docker-compose run --rm app bash -c "php oil r admin:give_user_role $USER-$n super_user || true && php oil r admin:give_user_role $USER-$n basic_author" -done - -for n in {1..20}; -do - docker-compose run --rm app bash -c "php oil r admin:new_user student-$n student-$n M Lastname student-$n@mail.com $PASS || php oil r admin:reset_password student-$n $PASS" - docker-compose run --rm app bash -c "php oil r admin:give_user_role student-$n basic_author" - break -done - # give them super_user and basic_author docker-compose run --rm app bash -c "php oil r admin:give_user_role $USER super_user || true && php oil r admin:give_user_role $USER basic_author" diff --git a/src/components/my-widgets-collaborate-dialog.jsx b/src/components/my-widgets-collaborate-dialog.jsx index 1d6c5d131..596578760 100644 --- a/src/components/my-widgets-collaborate-dialog.jsx +++ b/src/components/my-widgets-collaborate-dialog.jsx @@ -290,30 +290,28 @@ const MyWidgetsCollaborateDialog = ({onClose, inst, myPerms, otherUserPerms, set ignoreClose={state.shareNotAllowed}>
Collaborate -
-
- { searchContainerRender } -
- { mainContentRender } -
- {/* Calendar portal used to bring calendar popup out of access-list to avoid cutting off the overflow */} -
-

- Users with full access can edit or copy this widget and can - add or remove people in this list. -

- +
+ { searchContainerRender } +
+ { mainContentRender } +
+ {/* Calendar portal used to bring calendar popup out of access-list to avoid cutting off the overflow */} +
+

+ Users with full access can edit or copy this widget and can + add or remove people in this list. +

+
From e3427cf91e1381f443f91a85618507a9ca5cdaf2 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Thu, 25 Jan 2024 12:52:57 -0500 Subject: [PATCH 08/13] split hooks, rename funcs + prevent search on page load --- fuel/app/classes/controller/api/admin.php | 2 +- src/components/hooks/useInstanceList.jsx | 16 ++--- src/components/hooks/useSearchInstances.jsx | 74 +++++++++++++++++++++ src/components/support-page.jsx | 4 +- src/components/support-page.test.js | 20 +++--- src/components/support-search.jsx | 4 +- src/util/api.js | 4 +- 7 files changed, 96 insertions(+), 28 deletions(-) create mode 100644 src/components/hooks/useSearchInstances.jsx diff --git a/fuel/app/classes/controller/api/admin.php b/fuel/app/classes/controller/api/admin.php index 43150c011..9f3b080f8 100644 --- a/fuel/app/classes/controller/api/admin.php +++ b/fuel/app/classes/controller/api/admin.php @@ -69,7 +69,7 @@ public function post_user($user_id) return \Service_User::update_user($user_id, $user); } - public function get_widget_paginated_search(string $input, string $page_number) + public function get_instance_search(string $input, string $page_number) { $input = trim($input); $input = urldecode($input); diff --git a/src/components/hooks/useInstanceList.jsx b/src/components/hooks/useInstanceList.jsx index 56a83a0be..3527549ab 100644 --- a/src/components/hooks/useInstanceList.jsx +++ b/src/components/hooks/useInstanceList.jsx @@ -1,9 +1,9 @@ import { useState, useEffect, useMemo } from 'react' import { useInfiniteQuery } from 'react-query' -import { apiGetWidgetInstances, apiSearchWidgets } from '../../util/api' +import { apiGetWidgetInstances } from '../../util/api' import { iconUrl } from '../../util/icon-url' -export default function useInstanceList(userOnly = true, query = "") { +export default function useInstanceList() { const [errorState, setErrorState] = useState(false) @@ -36,11 +36,7 @@ export default function useInstanceList(userOnly = true, query = "") { } const getWidgetInstances = ({ pageParam = 0 }) => { - if (userOnly) { - return apiGetWidgetInstances(pageParam) - } else { - return apiSearchWidgets(query, pageParam) - } + return apiGetWidgetInstances(pageParam) } const { @@ -53,11 +49,9 @@ export default function useInstanceList(userOnly = true, query = "") { status, refetch } = useInfiniteQuery({ - queryKey: userOnly ? ['widgets'] : ['widgets', query], + queryKey: ['widgets'], queryFn: getWidgetInstances, - getNextPageParam: (lastPage, pages) => { - return lastPage.next_page - }, + getNextPageParam: (lastPage, pages) => lastPage.next_page, refetchOnWindowFocus: false }) diff --git a/src/components/hooks/useSearchInstances.jsx b/src/components/hooks/useSearchInstances.jsx new file mode 100644 index 000000000..d65222c58 --- /dev/null +++ b/src/components/hooks/useSearchInstances.jsx @@ -0,0 +1,74 @@ +import { useState, useEffect, useMemo } from 'react' +import { useInfiniteQuery } from 'react-query' +import { apiSearchInstances } from '../../util/api' +import { iconUrl } from '../../util/icon-url' + +export default function useSearchInstances(query = "") { + + const [errorState, setErrorState] = useState(false) + + // Helper function to sort widgets + const _compareWidgets = (a, b) => { return (b.created_at - a.created_at) } + + // transforms data object returned from infinite query + const formatData = (list) => { + if (list?.type == 'error') { + console.error(`Widget instances failed to load with error: ${list.msg}`); + setErrorState(true) + return [] + } + if (list?.pages) { + let dataMap = [] + return [ + ...dataMap.concat( + ...list.pages.map(page => page.pagination.map(instance => { + // adding an 'img' property to widget instance objects + return { + ...instance, + img: iconUrl(BASE_URL + 'widget/', instance.widget.dir, 275) + } + })) + ) + ].sort(_compareWidgets) // sort instances by creation date + } else return [] + } + + const getWidgetInstances = ({ pageParam = 0 }) => { + return apiSearchInstances(query, pageParam) + } + + const { + data, + error, + fetchNextPage, + hasNextPage, + isFetching, + isFetchingNextPage, + status, + refetch + } = useInfiniteQuery({ + queryKey: ['searched_instances', query], + queryFn: getWidgetInstances, + enabled: query.length > 0, + getNextPageParam: (lastPage, pages) => lastPage.next_page, + refetchOnWindowFocus: false + }) + + useEffect(() => { + if (error != null && error != undefined) setErrorState(true) + },[error]) + + // memoize the instance list since this is a large, expensive query + const instances = useMemo(() => formatData(data), [data]) + + useEffect(() => { + if (hasNextPage) fetchNextPage() + },[instances]) + + return { + instances: instances, + isFetching: isFetching || hasNextPage, + refresh: () => refetch(), + ...(errorState == true ? {error: true} : {}) // the error value is only provided if errorState is true + } +} diff --git a/src/components/support-page.jsx b/src/components/support-page.jsx index 552db1523..fa5fed772 100644 --- a/src/components/support-page.jsx +++ b/src/components/support-page.jsx @@ -1,6 +1,6 @@ import React, { useState, useRef, useEffect } from 'react' import { useQuery } from 'react-query' -import { apiGetUser, apiSearchWidgets} from '../util/api' +import { apiGetUser, apiSearchInstances} from '../util/api' import SupportSearch from './support-search' import SupportSelectedInstance from './support-selected-instance' import Header from './header' @@ -18,7 +18,7 @@ const SupportPage = () => { const { data: instFromHash } = useQuery({ queryKey: ['search-widgets', widgetHash], - queryFn: () => apiSearchWidgets(widgetHash), + queryFn: () => apiSearchInstances(widgetHash), enabled: widgetHash != undefined && widgetHash != selectedInstance?.id, staleTime: Infinity }) diff --git a/src/components/support-page.test.js b/src/components/support-page.test.js index f75fd04f0..57d60ec64 100644 --- a/src/components/support-page.test.js +++ b/src/components/support-page.test.js @@ -69,10 +69,10 @@ const renderWithClient = (children) => { describe('SupportSearch', () => { let rendered; let container; - let mockApiSearchWidgets; + let mockApiSearchInstances; beforeEach(() => { - mockApiSearchWidgets = jest.spyOn(api, 'apiSearchWidgets').mockImplementation(async input => search(input, instances)); + mockApiSearchInstances = jest.spyOn(api, 'apiSearchInstances').mockImplementation(async input => search(input, instances)); act(() => { rendered = renderWithClient() @@ -121,7 +121,7 @@ describe('SupportSearch', () => { }) // Was the API function called? - expect(mockApiSearchWidgets).toHaveBeenCalledTimes(1); + expect(mockApiSearchInstances).toHaveBeenCalledTimes(1); // jest.clearAllTimers(); @@ -180,7 +180,7 @@ describe('SupportSearch', () => { }) }) - // This does not work in the app yet because apiSearchWidgets does not search by created at + // This does not work in the app yet because apiSearchInstances does not search by created at it('searches by created_at', async () => { let input1 = "3/21" let input2 = "2023" @@ -265,7 +265,7 @@ describe('SupportSearch', () => { describe('SupportSelectedInstance', () => { let rendered; let container; - let mockApiSearchWidgets; + let mockApiSearchInstances; let mockApiGetUserPermsForInstance; let mockApiDeleteWidget; let mockApiUnDeleteWidget; @@ -280,7 +280,7 @@ describe('SupportSelectedInstance', () => { let modal = null; beforeEach(async () => { - mockApiSearchWidgets = jest.spyOn(api, 'apiSearchWidgets').mockImplementation(async input => search(input, instances)); + mockApiSearchInstances = jest.spyOn(api, 'apiSearchInstances').mockImplementation(async input => search(input, instances)); mockApiGetUserPermsForInstance = jest.spyOn(api, 'apiGetUserPermsForInstance').mockResolvedValue({ user_perms: { 5: [ @@ -408,7 +408,7 @@ describe('SupportSelectedInstance', () => { // }) // // Search should return updated widgets - // mockApiSearchWidgets = jest.spyOn(api, 'apiSearchWidgets').mockImplementation(async input => search(input, updatedInstances)); + // mockApiSearchInstances = jest.spyOn(api, 'apiSearchInstances').mockImplementation(async input => search(input, updatedInstances)); // await waitFor(() => { // expect(screen.getByText('Instance Admin')).not.toBeNull(); @@ -509,7 +509,7 @@ describe('SupportSelectedInstance', () => { let grant_access_checkbox = screen.getByLabelText("Grant Access to Original Owner(s)"); userEvent.click(grant_access_checkbox); - mockApiSearchWidgets = jest.spyOn(api, 'apiSearchWidgets').mockImplementation(async input => search(input, updatedInstances)); + mockApiSearchInstances = jest.spyOn(api, 'apiSearchInstances').mockImplementation(async input => search(input, updatedInstances)); // Closes copy dialog let save_btn = screen.getByText('Copy'); @@ -517,10 +517,10 @@ describe('SupportSelectedInstance', () => { userEvent.click(save_btn); }) - // Should call apiCopyInstance, which returns the new id, and then call mockApiSearchWidgets with the new id + // Should call apiCopyInstance, which returns the new id, and then call mockApiSearchInstances with the new id await waitFor(() => { expect(mockApiCopyWidget).toHaveBeenCalled(); - expect(mockApiSearchWidgets).toHaveBeenCalled(); + expect(mockApiSearchInstances).toHaveBeenCalled(); }) // Navigate to copied widget diff --git a/src/components/support-search.jsx b/src/components/support-search.jsx index 52efcb10e..7cb7209ae 100644 --- a/src/components/support-search.jsx +++ b/src/components/support-search.jsx @@ -1,6 +1,6 @@ import React, { useState, useEffect } from 'react' import { iconUrl } from '../util/icon-url' -import useInstanceList from './hooks/useInstanceList' +import useSearchInstances from './hooks/useSearchInstances' import useDebounce from './hooks/useDebounce' import LoadingIcon from './loading-icon' @@ -8,7 +8,7 @@ const SupportSearch = ({onClick = () => {}}) => { const [searchText, setSearchText] = useState('') const [showDeleted, setShowDeleted] = useState(false) const debouncedSearchTerm = useDebounce(searchText, 500) - const instanceList = useInstanceList(false, debouncedSearchTerm) + const instanceList = useSearchInstances(debouncedSearchTerm) useEffect(() => { if (instanceList.error) console.log(instanceList.error) diff --git a/src/util/api.js b/src/util/api.js index bad56b5f3..1f3192711 100644 --- a/src/util/api.js +++ b/src/util/api.js @@ -380,11 +380,11 @@ export const apiGetWidgetLock = (id = null) => { * @param {string} input (letters only) * @returns {array} of matches */ -export const apiSearchWidgets = (input, page_number) => { +export const apiSearchInstances = (input, page_number) => { let pattern = /[A-Za-z]+/g let match = input.match(pattern) if (!match || !match.length) input = ' ' - return fetch(`/api/admin/widget_paginated_search/${input}/${page_number}`) + return fetch(`/api/admin/instance_search/${input}/${page_number}`) .then(resp => { if (resp.status === 204 || resp.status === 502) return [] return resp.json() From d735b60530054eef5c1376cf9e80760d37598a3f Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Wed, 31 Jan 2024 10:17:30 -0500 Subject: [PATCH 09/13] Fix issue with double instances and reduce default limit --- fuel/app/classes/materia/api/v1.php | 8 +++----- .../materia/widget/instance/manager.php | 20 ++++++------------- fuel/app/classes/model/user.php | 2 +- fuel/app/tests/api/v1.php | 2 +- src/components/hooks/useInstanceList.jsx | 4 ++-- src/components/hooks/useUserList.jsx | 1 + src/util/api.js | 4 ++-- 7 files changed, 16 insertions(+), 25 deletions(-) diff --git a/fuel/app/classes/materia/api/v1.php b/fuel/app/classes/materia/api/v1.php index 3cd797ca6..b4a480fbf 100644 --- a/fuel/app/classes/materia/api/v1.php +++ b/fuel/app/classes/materia/api/v1.php @@ -64,7 +64,7 @@ static public function widget_instances_get($inst_ids = null, bool $deleted = fa * * @return array of objects containing total_num_pages and widget instances that are visible to the user. */ - static public function widget_paginate_instances_get($page_number = 0) + static public function widget_paginate_user_instances_get($page_number = 0) { if (\Service_User::verify_session() !== true) return Msg::no_login(); $data = Widget_Instance_Manager::get_paginated_instances_for_user(\Model_User::find_current_id(), $page_number); @@ -901,12 +901,10 @@ static public function users_search($input, $page_number = 0) $offset = $items_per_page * $page_number; // query DB for only a single page + 1 item - $displayable_items = \Model_User::find_by_name_search($input, $offset, $items_per_page + 1); + $displayable_items = \Model_User::find_by_name_search($input, $offset, $items_per_page); - $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; + $has_next_page = sizeof($displayable_items) > ($items_per_page - 1) ? true : false; - // scrub the user models with to_array - if ($has_next_page) array_pop($displayable_items); foreach ($displayable_items as $key => $person) { $displayable_items[$key] = $person->to_array(); diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index 48f75051c..7929b0520 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -14,7 +14,7 @@ static public function get($inst_id, $load_qset=false, $timestamp=false, $delete return count($instances) > 0 ? $instances[0] : false; } - static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=false, bool $deleted=false, $offset=0, $limit=2147483647): array + static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=false, bool $deleted=false, $offset=0, $limit=80): array { if ( ! is_array($inst_ids) || count($inst_ids) < 1) return []; @@ -26,7 +26,6 @@ static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=fal ->from('widget_instance') ->where('id', 'IN', $inst_ids) ->and_where('is_deleted', '=', $deleted ? '1' : '0') - ->order_by('created_at', 'desc') ->offset("$offset") ->limit("$limit") ->execute() @@ -65,7 +64,7 @@ public static function get_all_for_user($user_id, $load_qset=false) { $inst_ids = Perm_Manager::get_all_objects_for_user($user_id, Perm::INSTANCE, [Perm::FULL, Perm::VISIBLE]); - if ( ! empty($inst_ids)) return Widget_Instance_Manager::get_all($inst_ids, $load_qset); + if ( ! empty($inst_ids)) return self::get_all($inst_ids, $load_qset); else return []; } @@ -86,13 +85,10 @@ public static function get_paginated_instances_for_user($user_id, $page_number = $offset = $items_per_page * $page_number; // query DB for only a single page of instances + 1 - $displayable_items = self::get_all($inst_ids, false, false, false, $offset, $items_per_page + 1); + $displayable_items = self::get_all($inst_ids, false, false, false, $offset, $items_per_page); // if the returned number of instances is greater than a page, there's more pages - $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; - - // remove last instance if there are multiple pages - if ($has_next_page) array_pop($displayable_items); + $has_next_page = sizeof($displayable_items) > ($items_per_page - 1) ? true : false; $data = [ 'pagination' => $displayable_items @@ -155,13 +151,10 @@ public static function get_paginated_instance_search(string $input, $page_number $offset = $items_per_page * $page_number; // query DB for only a single page of instances + 1 - $displayable_items = self::get_widget_instance_search($input, $offset, $items_per_page + 1); + $displayable_items = self::get_widget_instance_search($input, $offset, $items_per_page); // if the returned number of instances is greater than a page, there's more pages - $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; - - // remove last instance if there are multiple pages - if ($has_next_page) array_pop($displayable_items); + $has_next_page = sizeof($displayable_items) > ($items_per_page - 1) ? true : false; $data = [ 'pagination' => $displayable_items, @@ -187,7 +180,6 @@ public static function get_widget_instance_search(string $input, int $offset = 0 ->from('widget_instance') ->where('id', 'LIKE', "%$input%") ->or_where('name', 'LIKE', "%$input%") - ->order_by('created_at', 'desc') ->limit("$limit") ->offset("$offset") ->execute() diff --git a/fuel/app/classes/model/user.php b/fuel/app/classes/model/user.php index 9419d1c91..122456326 100644 --- a/fuel/app/classes/model/user.php +++ b/fuel/app/classes/model/user.php @@ -87,7 +87,7 @@ public static function find_by_username($username) ->get_one(); } - static public function find_by_name_search($name, $offset = 0, $limit = 50) + static public function find_by_name_search($name, $offset = 0, $limit=80) { $name = preg_replace('/\s+/', '', $name); // remove spaces diff --git a/fuel/app/tests/api/v1.php b/fuel/app/tests/api/v1.php index a4c336dd7..8075e0f25 100644 --- a/fuel/app/tests/api/v1.php +++ b/fuel/app/tests/api/v1.php @@ -180,7 +180,7 @@ public function test_widget_instances_get() } - public function test_widget_paginate_instances_get() + public function test_widget_paginate_user_instances_get() { } diff --git a/src/components/hooks/useInstanceList.jsx b/src/components/hooks/useInstanceList.jsx index 3527549ab..c97b28a84 100644 --- a/src/components/hooks/useInstanceList.jsx +++ b/src/components/hooks/useInstanceList.jsx @@ -1,6 +1,6 @@ import { useState, useEffect, useMemo } from 'react' import { useInfiniteQuery } from 'react-query' -import { apiGetWidgetInstances } from '../../util/api' +import { apiGetUserWidgetInstances } from '../../util/api' import { iconUrl } from '../../util/icon-url' export default function useInstanceList() { @@ -36,7 +36,7 @@ export default function useInstanceList() { } const getWidgetInstances = ({ pageParam = 0 }) => { - return apiGetWidgetInstances(pageParam) + return apiGetUserWidgetInstances(pageParam) } const { diff --git a/src/components/hooks/useUserList.jsx b/src/components/hooks/useUserList.jsx index b9691b455..c82af878e 100644 --- a/src/components/hooks/useUserList.jsx +++ b/src/components/hooks/useUserList.jsx @@ -40,6 +40,7 @@ export default function useUserList(query = "") { } = useInfiniteQuery({ queryKey: ['users', query], queryFn: getData, + enabled: query.length > 0, getNextPageParam: (lastPage, pages) => { return lastPage.next_page }, diff --git a/src/util/api.js b/src/util/api.js index 4b74891d7..effe242b4 100644 --- a/src/util/api.js +++ b/src/util/api.js @@ -35,8 +35,8 @@ export const apiGetWidgetInstance = (instId, loadQset=false) => { * storage * @returns An array of objects. */ -export const apiGetWidgetInstances = (page_number = 0) => { - return fetch(`/api/json/widget_paginate_instances_get/${page_number}`, fetchOptions({ body: `data=${formatFetchBody([page_number])}` })) +export const apiGetUserWidgetInstances = (page_number = 0) => { + return fetch(`/api/json/widget_paginate_user_instances_get/${page_number}`, fetchOptions({ body: `data=${formatFetchBody([page_number])}` })) .then(resp => { if (resp.status === 204 || resp.status === 502) return [] return resp.json() From 6e1bf19b241effb3fa3bb3535c2004569f128afe Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Wed, 31 Jan 2024 10:19:24 -0500 Subject: [PATCH 10/13] Reduce another default limit --- fuel/app/classes/materia/widget/instance/manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index 7929b0520..4b3f5877a 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -174,7 +174,7 @@ public static function get_paginated_instance_search(string $input, $page_number * * @return array of widget instances related to the given input */ - public static function get_widget_instance_search(string $input, int $offset = 0, int $limit = 2147483647): array + public static function get_widget_instance_search(string $input, int $offset = 0, int $limit = 80): array { $results = \DB::select() ->from('widget_instance') From 0c9200eabf85be10507b7716c45ccd078ced2067 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Thu, 1 Feb 2024 09:06:13 -0500 Subject: [PATCH 11/13] Add additional order_by query to get predictable results --- fuel/app/classes/materia/api/v1.php | 6 ++++-- .../materia/widget/instance/manager.php | 20 +++++++++++++------ src/components/include.scss | 2 +- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/fuel/app/classes/materia/api/v1.php b/fuel/app/classes/materia/api/v1.php index b4a480fbf..8fc26b70d 100644 --- a/fuel/app/classes/materia/api/v1.php +++ b/fuel/app/classes/materia/api/v1.php @@ -901,9 +901,11 @@ static public function users_search($input, $page_number = 0) $offset = $items_per_page * $page_number; // query DB for only a single page + 1 item - $displayable_items = \Model_User::find_by_name_search($input, $offset, $items_per_page); + $displayable_items = \Model_User::find_by_name_search($input, $offset, $items_per_page + 1); - $has_next_page = sizeof($displayable_items) > ($items_per_page - 1) ? true : false; + $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; + + if ($has_next_page) array_pop($displayable_items); foreach ($displayable_items as $key => $person) { diff --git a/fuel/app/classes/materia/widget/instance/manager.php b/fuel/app/classes/materia/widget/instance/manager.php index 4b3f5877a..65f7251c1 100644 --- a/fuel/app/classes/materia/widget/instance/manager.php +++ b/fuel/app/classes/materia/widget/instance/manager.php @@ -26,6 +26,8 @@ static public function get_all(Array $inst_ids, $load_qset=false, $timestamp=fal ->from('widget_instance') ->where('id', 'IN', $inst_ids) ->and_where('is_deleted', '=', $deleted ? '1' : '0') + ->order_by('created_at', 'desc') + ->order_by('id', 'desc') ->offset("$offset") ->limit("$limit") ->execute() @@ -85,10 +87,12 @@ public static function get_paginated_instances_for_user($user_id, $page_number = $offset = $items_per_page * $page_number; // query DB for only a single page of instances + 1 - $displayable_items = self::get_all($inst_ids, false, false, false, $offset, $items_per_page); + $displayable_items = self::get_all($inst_ids, false, false, false, $offset, $items_per_page + 1); // if the returned number of instances is greater than a page, there's more pages - $has_next_page = sizeof($displayable_items) > ($items_per_page - 1) ? true : false; + $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; + + if ($has_next_page) array_pop($displayable_items); $data = [ 'pagination' => $displayable_items @@ -151,10 +155,12 @@ public static function get_paginated_instance_search(string $input, $page_number $offset = $items_per_page * $page_number; // query DB for only a single page of instances + 1 - $displayable_items = self::get_widget_instance_search($input, $offset, $items_per_page); + $displayable_items = self::get_widget_instance_search($input, $offset, $items_per_page + 1); // if the returned number of instances is greater than a page, there's more pages - $has_next_page = sizeof($displayable_items) > ($items_per_page - 1) ? true : false; + $has_next_page = sizeof($displayable_items) > $items_per_page ? true : false; + + if ($has_next_page) array_pop($displayable_items); $data = [ 'pagination' => $displayable_items, @@ -180,8 +186,10 @@ public static function get_widget_instance_search(string $input, int $offset = 0 ->from('widget_instance') ->where('id', 'LIKE', "%$input%") ->or_where('name', 'LIKE', "%$input%") - ->limit("$limit") - ->offset("$offset") + ->order_by('created_at', 'desc') + ->order_by('id', 'desc') + ->offset($offset) + ->limit($limit) ->execute() ->as_array(); diff --git a/src/components/include.scss b/src/components/include.scss index 5f8cc04b0..ccf11801a 100644 --- a/src/components/include.scss +++ b/src/components/include.scss @@ -295,7 +295,7 @@ header { background-color: #ffffff; padding: 0; position: absolute; - bottom: -150%; + bottom: -140%; left: -10px; border-left: 1px solid #d3d3d3; border-right: 1px solid #d3d3d3; From c3549e019c8b9a3fff0a62829eb6c535b249d150 Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Thu, 1 Feb 2024 10:12:41 -0500 Subject: [PATCH 12/13] Remove unnecessary front-end sort --- src/components/hooks/useInstanceList.jsx | 5 +---- src/components/hooks/useSearchInstances.jsx | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/components/hooks/useInstanceList.jsx b/src/components/hooks/useInstanceList.jsx index c97b28a84..3254ea43c 100644 --- a/src/components/hooks/useInstanceList.jsx +++ b/src/components/hooks/useInstanceList.jsx @@ -7,9 +7,6 @@ export default function useInstanceList() { const [errorState, setErrorState] = useState(false) - // Helper function to sort widgets - const _compareWidgets = (a, b) => { return (b.created_at - a.created_at) } - // transforms data object returned from infinite query into one we can use in the my-widgets-page component // this creates a flat list of instances from the paginated list that's subsequently sorted const formatData = (list) => { @@ -31,7 +28,7 @@ export default function useInstanceList() { } })) ) - ].sort(_compareWidgets) + ] } else return [] } diff --git a/src/components/hooks/useSearchInstances.jsx b/src/components/hooks/useSearchInstances.jsx index d65222c58..406533739 100644 --- a/src/components/hooks/useSearchInstances.jsx +++ b/src/components/hooks/useSearchInstances.jsx @@ -7,9 +7,6 @@ export default function useSearchInstances(query = "") { const [errorState, setErrorState] = useState(false) - // Helper function to sort widgets - const _compareWidgets = (a, b) => { return (b.created_at - a.created_at) } - // transforms data object returned from infinite query const formatData = (list) => { if (list?.type == 'error') { @@ -29,7 +26,7 @@ export default function useSearchInstances(query = "") { } })) ) - ].sort(_compareWidgets) // sort instances by creation date + ] } else return [] } From 7598e84bc8c9371539eadf9859bdc389975ff73f Mon Sep 17 00:00:00 2001 From: Cay Henning Date: Thu, 1 Feb 2024 10:26:50 -0500 Subject: [PATCH 13/13] Add test --- fuel/app/tests/api/v1.php | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/fuel/app/tests/api/v1.php b/fuel/app/tests/api/v1.php index 8075e0f25..2300c9529 100644 --- a/fuel/app/tests/api/v1.php +++ b/fuel/app/tests/api/v1.php @@ -182,7 +182,30 @@ public function test_widget_instances_get() public function test_widget_paginate_user_instances_get() { + // Create widget instance + $this->_as_author(); + $title = "My Test Widget"; + $question = 'What rhymes with harvest fests but are half as exciting (or tasty)'; + $answer = 'Tests'; + $qset = $this->create_new_qset($question, $answer); + $widget = $this->make_disposable_widget(); + + $instance = Api_V1::widget_instance_new($widget->id, $title, $qset, true); + + // ----- loads author's instances -------- + $output = Api_V1::widget_paginate_user_instances_get(); + $this->assertIsArray($output); + $this->assertArrayHasKey('pagination', $output); + foreach ($output['pagination'] as $key => $value) + { + $this->assert_is_widget_instance($value, true); + } + // ======= AS NO ONE ======== + \Auth::logout(); + // ----- returns no login -------- + $output = Api_V1::widget_paginate_user_instances_get(); + $this->assert_invalid_login_message($output); } public function test_widget_instance_new() @@ -1024,10 +1047,6 @@ public function test_play_logs_get() } - public function test_paginated_play_logs_get() - { - } - public function test_score_summary_get() { // ======= AS NO ONE ========