Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor students page fetching data through Gutenberg and avoiding subqueries #5104

Merged
merged 10 commits into from May 16, 2022
Merged

Refactor students page fetching data through Gutenberg and avoiding subqueries #5104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1421b76
Separate learners subquery
renatho May 9, 2022
5c93235
Fix PHPCS
renatho May 9, 2022
3397c4a
Remove unused enqueue
renatho May 10, 2022
f610415
Add hook to handle useSelect with debounce
renatho May 10, 2022
b60e246
Refactor course list to use core store
renatho May 10, 2022
2467160
Merge branch 'trunk' of github.com:Automattic/sensei into change/stud…
renatho May 10, 2022
4fd0b31
Remove apiFetch middleware that changed the URL arguments to queryStr…
renatho May 11, 2022
2e9c6bf
Add div custom kses for specific envs
renatho May 11, 2022
859125c
Add some tests to useSelectWithDebounce
renatho May 13, 2022
4e602d0
Fix PHPCS ignores
renatho May 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 0 additions & 18 deletions assets/admin/lib/http-client/index.js
View file
Open in desktop

This file was deleted.

35 changes: 0 additions & 35 deletions assets/admin/lib/http-client/index.test.js
View file
Open in desktop

This file was deleted.

10 changes: 5 additions & 5 deletions assets/admin/students/student-action-menu/index.test.js
View file
Open in desktop
Expand Up @@ -7,18 +7,18 @@ import { act, fireEvent, render, screen } from '@testing-library/react';
* WordPress dependencies
*/
import { DOWN } from '@wordpress/keycodes';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { StudentActionMenu } from './index';
import httpClient from '../../lib/http-client';

jest.mock( '../../lib/http-client' );
jest.mock( '@wordpress/data' );

describe( '<StudentActionMenu />', () => {
it( 'Should display modal when "Add to Course" is selected', async () => {
httpClient.mockImplementation( () => Promise.resolve( [] ) );
useSelect.mockReturnValue( { courses: [], isFetching: false } );
render( <StudentActionMenu /> );

// Open the dropdown menu.
Expand All @@ -41,7 +41,7 @@ describe( '<StudentActionMenu />', () => {
} );

it( 'Should display modal when "Remove from Course" is selected', async () => {
httpClient.mockImplementation( () => Promise.resolve( [] ) );
useSelect.mockReturnValue( { courses: [], isFetching: false } );
render( <StudentActionMenu /> );

// Open the dropdown menu.
Expand All @@ -64,7 +64,7 @@ describe( '<StudentActionMenu />', () => {
} );

it( 'Should display modal when "Reset or Remove progress" is selected', async () => {
httpClient.mockImplementation( () => Promise.resolve( [] ) );
useSelect.mockReturnValue( { courses: [], isFetching: false } );
render( <StudentActionMenu /> );

// Open the dropdown menu.
Expand Down
16 changes: 9 additions & 7 deletions assets/admin/students/student-bulk-action-button/index.test.js
View file
Open in desktop
Expand Up @@ -2,11 +2,18 @@
* External dependencies
*/
import { render, screen } from '@testing-library/react';

/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { StudentBulkActionButton } from './index';
import nock from 'nock';

jest.mock( '@wordpress/data' );

/**
* Create a custom screen selector that ignores text inside html tags like hello <strong> world</strong>
Expand All @@ -32,7 +39,6 @@ const courses = [
title: { rendered: 'My Course' },
},
];
const NOCK_HOST_URL = 'http://localhost';

// Create a bulk action selector with enrol student option selected.
beforeAll( () => {
Expand All @@ -45,11 +51,7 @@ describe( '<StudentBulkActionButton />', () => {
} );

beforeAll( () => {
nock( NOCK_HOST_URL )
.persist()
.get( '/wp-json/wp/v2/courses' )
.query( { per_page: 100 } )
.reply( 200, courses );
useSelect.mockReturnValue( { courses, isFetching: false } );
} );

it( 'Should be disabled by default on render', () => {
Expand Down
50 changes: 0 additions & 50 deletions assets/admin/students/student-modal/course-list-fetch.test.js
View file
Open in desktop

This file was deleted.

64 changes: 25 additions & 39 deletions assets/admin/students/student-modal/course-list.js
View file
Open in desktop
Expand Up @@ -2,20 +2,15 @@
* WordPress dependencies
*/
import { CheckboxControl, Spinner } from '@wordpress/components';
import { useCallback, useEffect, useRef, useState } from '@wordpress/element';
import { useCallback, useRef, useState } from '@wordpress/element';
import { decodeEntities } from '@wordpress/html-entities';
import { __ } from '@wordpress/i18n';

/**
* External dependencies
*/
import { debounce } from 'lodash';
import { store as coreDataStore } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import httpClient from '../../lib/http-client';
import useAbortController from '../hooks/use-abort-controller';
import useSelectWithDebounce from '../../../react-hooks/use-select-with-debounce';

/**
* Callback for select or unselect courseItem
Expand Down Expand Up @@ -97,10 +92,7 @@ const CourseItem = ( { course, checked = false, onChange } ) => {
* @param {onCourseSelectionChange} props.onChange Event triggered when a course is selected or unselected
*/
export const CourseList = ( { searchQuery, onChange } ) => {
const [ isFetching, setIsFetching ] = useState( true );
const [ courses, setCourses ] = useState( [] );
const selectedCourses = useRef( [] );
const { getSignal } = useAbortController();

const selectCourse = useCallback(
( { isSelected, course } ) => {
Expand All @@ -113,34 +105,28 @@ export const CourseList = ( { searchQuery, onChange } ) => {
[ onChange ]
);

// Fetch the courses.
const fetchCourses = useCallback(
debounce( ( query ) => {
setIsFetching( true );

httpClient( {
path:
'/wp/v2/courses?per_page=100' +
( query ? `&search=${ query }` : '' ),
method: 'GET',
signal: getSignal(),
} )
.then( ( result ) => setCourses( result ) )
.catch( ( error ) => {
console.log( error ); // eslint-disable-line no-console
} )
.finally( () => {
if ( ! getSignal().aborted ) {
setIsFetching( false );
}
} );
}, 400 ),
[]
); // eslint-disable-line react-hooks/exhaustive-deps

useEffect( () => {
fetchCourses( searchQuery );
}, [ fetchCourses, searchQuery ] );
const { courses, isFetching } = useSelectWithDebounce(
( select ) => {
const store = select( coreDataStore );

const query = {
per_page: 100,
search: searchQuery,
};

return {
courses:
store.getEntityRecords( 'postType', 'course', query ) || [],
isFetching: ! store.hasFinishedResolution( 'getEntityRecords', [
'postType',
'course',
query,
] ),
};
},
[ searchQuery ],
500
);

return (
<>
Expand Down
35 changes: 15 additions & 20 deletions assets/admin/students/student-modal/course-list.test.js
View file
Open in desktop
Expand Up @@ -2,7 +2,11 @@
* External dependencies
*/
import { render, screen, fireEvent } from '@testing-library/react';
import nock from 'nock';

/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
Expand All @@ -23,16 +27,13 @@ const courses = [
title: { rendered: 'Course 3' },
},
];
const NOCK_HOST_URL = 'http://localhost';

jest.mock( '@wordpress/data' );

describe( '<CourseList />', () => {
beforeAll( () => {
nock( NOCK_HOST_URL )
.persist()
.get( '/wp/v2/courses' )
.query( { per_page: 100, _locale: 'user' } )
.reply( 200, courses );
useSelect.mockReturnValue( { courses, isFetching: false } );
} );
afterAll( () => nock.cleanAll() );

it( 'Should display courses in the list', async () => {
render( <CourseList /> );
Expand Down Expand Up @@ -81,12 +82,7 @@ describe( '<CourseList />', () => {

describe( 'When there is no course', () => {
beforeEach( () => {
nock.cleanAll();
nock( NOCK_HOST_URL )
.get( '/wp/v2/courses' )
.query( { per_page: 100, _locale: 'user' } )
.once()
.reply( 200, [] );
useSelect.mockReturnValue( { courses: [], isFetching: false } );
} );

it( 'Should show a message when there are no courses', async () => {
Expand All @@ -100,16 +96,15 @@ describe( '<CourseList />', () => {

describe( 'When there are HTML-Entities in course titles', () => {
beforeEach( () => {
nock.cleanAll();
nock( NOCK_HOST_URL )
.get( '/wp/v2/courses' )
.query( { per_page: 100, _locale: 'user' } )
.reply( 200, [
useSelect.mockReturnValue( {
courses: [
{
id: 1,
title: { rendered: 'Course&#8217;s' },
},
] );
],
isFetching: false,
} );
} );

it( 'Should show the course title without HTML-Entities', async () => {
Expand Down
14 changes: 7 additions & 7 deletions assets/admin/students/student-modal/index.js
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@ import { Button, Modal, Notice, Spinner } from '@wordpress/components';
import { useCallback, useState, RawHTML } from '@wordpress/element';
import { search } from '@wordpress/icons';
import { __, _n, sprintf } from '@wordpress/i18n';
import apiFetch from '@wordpress/api-fetch';

/**
* External dependencies
Expand All @@ -16,7 +17,6 @@ import { escape } from 'lodash';
*/
import CourseList from './course-list';
import InputControl from '../../../blocks/editor-components/input-control';
import httpClient from '../../lib/http-client';
import useAbortController from '../hooks/use-abort-controller';

const getAction = ( action, studentCount ) => {
Expand All @@ -38,8 +38,8 @@ const getAction = ( action, studentCount ) => {
'sensei-lms'
),
sendAction: ( students, courses, { signal } ) =>
httpClient( {
restRoute: '/sensei-internal/v1/course-students/batch',
apiFetch( {
path: '/sensei-internal/v1/course-students/batch',
method: 'POST',
data: { student_ids: students, course_ids: courses },
signal,
Expand All @@ -63,8 +63,8 @@ const getAction = ( action, studentCount ) => {
'sensei-lms'
),
sendAction: ( students, courses, { signal } ) =>
httpClient( {
restRoute: '/sensei-internal/v1/course-students/batch',
apiFetch( {
path: '/sensei-internal/v1/course-students/batch',
method: 'DELETE',
data: { student_ids: students, course_ids: courses },
signal,
Expand All @@ -88,8 +88,8 @@ const getAction = ( action, studentCount ) => {
'sensei-lms'
),
sendAction: ( students, courses, { signal } ) =>
httpClient( {
restRoute: '/sensei-internal/v1/course-progress/batch',
apiFetch( {
path: '/sensei-internal/v1/course-progress/batch',
method: 'DELETE',
data: { student_ids: students, course_ids: courses },
signal,
Expand Down