Skip to content

Commit

Permalink
Merge pull request #1483 from clpetersonucf/react/my-widgets-rework-a…
Browse files Browse the repository at this point in the history
…gain

[React branch] Improved pagination behavior in My Widgets
  • Loading branch information
clpetersonucf authored Jul 27, 2023
2 parents a52e0d8 + c679d79 commit 5daaa65
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 156 deletions.
8 changes: 4 additions & 4 deletions fuel/app/classes/materia/api/v1.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ static public function widget_instances_get($inst_ids = null, bool $deleted = fa
* Takes a page number, and returns objects containing the total_num_pages and
* widget instances that are visible to the user.
*
* @param page_number The page to be retreated. By default it is set to 1.
* @param page_number The page to be requested. By default it is set to 1.
*
* @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 = 1)
static public function widget_paginate_instances_get($page_number = 0)
{
if (\Service_User::verify_session() !== true) return []; // shortcut to returning noting
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);
return $data;
}
Expand Down Expand Up @@ -146,7 +146,7 @@ static public function widget_instance_copy(string $inst_id, string $new_name, b
// retain access - if true, grant access to the copy to all original owners
$current_user_id = \Model_User::find_current_id();
$duplicate = $inst->duplicate($current_user_id, $new_name, $copy_existing_perms);
return $duplicate->id;
return $duplicate;
}
catch (\Exception $e)
{
Expand Down
12 changes: 11 additions & 1 deletion fuel/app/classes/materia/widget/instance.php
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,17 @@ public function duplicate(int $owner_id, string $new_name = null, bool $copy_exi
$duplicate->id = 0; // mark as a new game
$duplicate->user_id = $owner_id; // set current user as owner in instance table

if ( ! empty($new_name)) $duplicate->name = $new_name; // update name
// update name
if ( ! empty($new_name)) $duplicate->name = $new_name;

// these values aren't saved to the db - but the frontend will make use of them
$duplicate->clean_name = \Inflector::friendly_title($duplicate->name, '-', true);
$base_url = "{$duplicate->id}/{$duplicate->clean_name}";
$duplicate->preview_url = \Config::get('materia.urls.preview').$base_url;
$duplicate->play_url = $duplicate->is_draft === false ? \Config::get('materia.urls.play').$base_url : '';
$duplicate->embed_url = $duplicate->is_draft === false ? \Config::get('materia.urls.embed').$base_url : '';

$duplicate->created_at = time(); // manually update created_at, the actual value saved to the db is created in db_store

// if original widget is student made - verify if new owner is a student or not
// if they have a basic_author role or above, turn off the is_student_made flag
Expand Down
11 changes: 7 additions & 4 deletions fuel/app/classes/materia/widget/instance/manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,23 @@ 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 = 1)
public static function get_paginated_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 - 1);
$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);

$data = [
'total_num_pages' => $total_num_pages,
'pagination' => $displayable_inst,
'pagination' => $displayable_inst,
];

if ($has_next_page) $data['next_page'] = $page_number + 1;

return $data;
}
Expand Down
8 changes: 4 additions & 4 deletions fuel/app/tests/api/v1.php
Original file line number Diff line number Diff line change
Expand Up @@ -557,9 +557,9 @@ public function test_widget_instance_copy()


$output = Api_V1::widget_instance_copy($inst_id, 'Copied Widget');
$this->assert_is_valid_id($output);
$this->assert_is_valid_id($output->id);

$insts = Api_V1::widget_instances_get($output);
$insts = Api_V1::widget_instances_get($output->id);
$this->assert_is_widget_instance($insts[0], true);
$this->assertEquals('Copied Widget', $insts[0]->name);
$this->assertEquals(true, $insts[0]->is_draft);
Expand All @@ -577,9 +577,9 @@ public function test_widget_instance_copy()


$output = Api_V1::widget_instance_copy($inst_id, 'Copied Widget');
$this->assert_is_valid_id($output);
$this->assert_is_valid_id($output->id);

$insts = Api_V1::widget_instances_get($output);
$insts = Api_V1::widget_instances_get($output->id);
$this->assert_is_widget_instance($insts[0], true);
$this->assertEquals('Copied Widget', $insts[0]->name);
$this->assertEquals(true, $insts[0]->is_draft);
Expand Down
33 changes: 26 additions & 7 deletions src/components/hooks/useCopyWidget.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ export default function useCopyWidget() {
{
onMutate: async inst => {
await queryClient.cancelQueries('widgets', { exact: true, active: true, })
// 'getQueryData()' is a sync method
const previousValue = queryClient.getQueryData('widgets')

// dummy data that's appended to the query cache as an optimistic update
// this will be replaced with actual data returned from the API
const newInst = {
id: 'tmp',
widget: {
Expand All @@ -28,19 +29,37 @@ export default function useCopyWidget() {
is_fake: true
}

let updateData = previousValue
if (updateData) updateData.pagination?.unshift(newInst)
// setQueryClient must treat the query cache as immutable!!!
// previous will contain the cached value, the function argument creates a new object from previous
queryClient.setQueryData('widgets', (previous) => ({
...previous,
pages: previous.pages.map((page, index) => {
if (index == 0) return { ...page, pagination: [ newInst, ...page.pagination] }
else return page
})
}))

// 'setQueryData()' is a sync method
queryClient.setQueryData('widgets', updateData) // can confirm 'widgets' is updating
return { previousValue }
},
onSuccess: (data, variables) => {
queryClient.invalidateQueries('widgets')
// update the query cache, which previously contained a dummy instance, with the real instance info
queryClient.setQueryData('widgets', (previous) => ({
...previous,
pages: previous.pages.map((page, index) => {
if (index == 0) return { ...page, pagination: page.pagination.map((inst) => {
if (inst.id == 'tmp') inst = data
return inst
}) }
else return page
})
}))
variables.successFunc(data)
},
onError: (err, newWidget, context) => {
queryClient.setQueryData('widgets', context.previousValue)
console.error(err)
queryClient.setQueryData('widgets', (previous) => {
return context.previousValue
})
}
}
)
Expand Down
20 changes: 13 additions & 7 deletions src/components/hooks/useDeleteWidget.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,30 @@ export default function useDeleteWidget() {
// Handles the optimistic update for deleting a widget
onMutate: async inst => {
await queryClient.cancelQueries('widgets')

const previousValue = queryClient.getQueryData('widgets')
const delID = inst.instId

queryClient.setQueryData('widgets', old => {
if (!old || !old.pagination) return old
return {...old, pagination: old.pagination.filter(widget => widget.id !== delID)}
queryClient.setQueryData('widgets', previous => {
if (!previous || !previous.pages) return previous
return {
...previous,
pages: previous.pages.map((page) => ({
...page,
pagination: page.pagination.filter(widget => widget.id !== inst.instId)
}))
}
})

// Stores the old value for use if there is an error
return { previousValue }
},
onSuccess: (data, variables) => {
queryClient.invalidateQueries('widgets')
variables.successFunc(data)
},
onError: (err, newWidget, context) => {
queryClient.setQueryData('widgets', context.previousValue)
console.error(err)
queryClient.setQueryData('widgets', (previous) => {
return context.previousValue
})
}
}
)
Expand Down
61 changes: 61 additions & 0 deletions src/components/hooks/useInstanceList.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { useState, useEffect, useMemo } from 'react'
import { useInfiniteQuery } from 'react-query'
import { apiGetWidgetInstances } from '../../util/api'

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) => {
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))].sort(_compareWidgets)
} else return []
}

const getWidgetInstances = ({ pageParam = 0}) => {
return apiGetWidgetInstances(pageParam)
}

const {
data,
error,
fetchNextPage,
hasNextPage,
isFetching,
isFetchingNextPage,
status,
} = useInfiniteQuery({
queryKey: ['widgets'],
queryFn: getWidgetInstances,
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,
...(errorState == true ? {error: true} : {}) // the error value is only provided if errorState is true
}
}
49 changes: 14 additions & 35 deletions src/components/lti/select-item.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { useState, useEffect, useMemo, useRef } from 'react'
import { useQuery } from 'react-query';
import { apiGetWidgetInstances } from '../../util/api'
import useInstanceList from '../hooks/useInstanceList'
import { iconUrl } from '../../util/icon-url'
import LoadingIcon from '../loading-icon';

Expand All @@ -14,27 +13,7 @@ const SelectItem = () => {
const fillRef = useRef(null)
const [progressComplete, setProgressComplete] = useState(false)

const [state, setState] = useState({
page: 1,
instances: [],
})

const { data, isFetching: isFetching, refetch: refetchInstances} = useQuery({
queryKey: 'instances',
queryFn: () => apiGetWidgetInstances(state.page),
staleTime: Infinity,
onSuccess: (data) => {
if (data) {
data.pagination.map((instance, index) => {
instance.img = iconUrl(BASE_URL + 'widget/', instance.widget.dir, 60)
instance.preview_url = BASE_URL + 'preview/' + instance.id
instance.edit_url = BASE_URL + 'my-widgets/#' + instance.id
})

setState({...state, instances: data.pagination})
}
}
})
const instanceList = useInstanceList()

useEffect(() => {
if (window.SYSTEM) {
Expand All @@ -47,15 +26,15 @@ const SelectItem = () => {
if(searchText == '') return result

const re = RegExp(searchText, 'i')
if (state.instances && state.instances.length > 0)
state.instances.forEach(i => {
if (instanceList.instances && instanceList.instances.length > 0)
instanceList.instances.forEach(i => {
if(!re.test(`${i.name} ${i.widget.name} ${i.id}`)){
result.add(i.id)
}
})

return result
}, [searchText, state.instances])
}, [searchText, instanceList.instances])

const handleChange = (e) => {
setSearchText(e.target.value)
Expand Down Expand Up @@ -137,11 +116,11 @@ const SelectItem = () => {
}
}, [selectedInstance, progressComplete])

let instanceList = null
if (state.instances && state.instances.length > 0) {
if (hiddenSet.size >= state.instances.length) instanceList = <p>No widgets match your search.</p>
let instanceListRender = null
if (instanceList.instances && instanceList.instances.length > 0) {
if (hiddenSet.size >= instanceList.instances.length) instanceListRender = <p>No widgets match your search.</p>
else {
instanceList = state.instances.map((instance, index) => {
instanceListRender = instanceList.instances.map((instance, index) => {
var classList = []
if (instance.is_draft) classList.push('draft')
if (instance.selected) classList.push('selected')
Expand All @@ -150,7 +129,7 @@ const SelectItem = () => {

return <li className={classList.join(' ')} key={index}>
<div className={`widget-info ${instance.is_draft ? 'draft' : ''} ${instance.guest_access ? 'guest' : ''}`}>
<img className="widget-icon" src={instance.img}/>
<img className="widget-icon" src={iconUrl(BASE_URL + 'widget/', instance.widget.dir, 60)}/>
<h2 className="searchable">{instance.name}</h2>
<h3 className="searchable">{instance.widget.name}</h3>
{instance.guest_access ? <h3 className="guest-notice">Guest instances cannot be embedded in courses. </h3> : <></>}
Expand All @@ -162,7 +141,7 @@ const SelectItem = () => {
<a className="preview external" target="_blank" href={instance.preview_url}>Preview</a>
{
(instance.guest_access || instance.is_draft) ?
<a className="action_button embed-button" target="_blank" href={instance.edit_url}>Edit at Materia</a>
<a className="action_button embed-button" target="_blank" href={`${BASE_URL}my-widgets/#${instance.id}`}>Edit at Materia</a>
:
<a role="button" className={index == 0 ? 'first action_button embed-button' : 'action_button embed-button'} onClick={() => embedInstance(instance)}>Use this widget</a>
}
Expand All @@ -174,7 +153,7 @@ const SelectItem = () => {

let noInstanceRender = null
let createNewInstanceLink = null
if (state.instances && state.instances.length < 1) {
if (instanceList.instances && instanceList.instances.length < 1) {
noInstanceRender = <div id="no-widgets-container">
<div id="no-instances">
<p>You don't have any widgets yet. Click this button to create a widget, then return to this tab/window and select your new widget.</p>
Expand All @@ -186,7 +165,7 @@ const SelectItem = () => {
}

let sectionRender = null
if (isFetching) {
if (instanceList.isFetching) {
sectionRender =
<section id="loading">
<LoadingIcon size="med" />
Expand Down Expand Up @@ -214,7 +193,7 @@ const SelectItem = () => {
</section>
<div id="list-container">
<ul>
{instanceList}
{instanceListRender}
</ul>
</div>
{createNewInstanceLink}
Expand Down
Loading

0 comments on commit 5daaa65

Please sign in to comment.