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.

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

Collections Pagination #554

Merged
merged 28 commits into from
Apr 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8318def
fleshing out collections pagination
Apr 11, 2018
ead347e
fleshing out actions/reducers for pagination
Apr 12, 2018
47bd9fe
fleshing out hoc for paged collections
Apr 12, 2018
108f30d
add paging parameters to api getCollections call
adorsk Apr 13, 2018
0a8ceff
adding initial state for currentPage, adding handler for set_current_…
adorsk Apr 13, 2018
53e22b5
add add actioncreator for set current page
adorsk Apr 13, 2018
f2c20fc
combining collectionlistpage w/ hoc withPagedCollections
adorsk Apr 13, 2018
b272802
adding paginator handlers/styling
adorsk Apr 13, 2018
eeee2b1
adding start of paginator to collectionlistpage
adorsk Apr 13, 2018
2c0501f
Merge branch 'dorska/add_page_indices' into dorska/collections_pagina…
adorsk Apr 13, 2018
fdcf642
Merge branch 'dorska/add_page_indices' into dorska/collections_pagina…
adorsk Apr 13, 2018
1a7beb0
tweak pagination styling
adorsk Apr 13, 2018
a5555f1
updating paginations reducer tests
adorsk Apr 13, 2018
caef827
updating pagination actions
adorsk Apr 13, 2018
4c1365d
update api to use pagination parameters
adorsk Apr 13, 2018
d6fd10d
adding tests for loading state to collection list page
adorsk Apr 13, 2018
7bc573d
updating withPagedCollections hoc tests
adorsk Apr 13, 2018
9d5c624
fleshing out paginator tests
adorsk Apr 13, 2018
11e1a30
add flow checks
adorsk Apr 17, 2018
8aef430
Merge branch 'master' into dorska/collections_pagination
adorsk Apr 17, 2018
a8af5b7
travis bump
adorsk Apr 17, 2018
e3fd0c9
Revert "travis bump"
adorsk Apr 17, 2018
1cd3f8a
change travis install to build instead of run
adorsk Apr 17, 2018
f5bc851
fix pip string for pip 10 (which tox force installs >:( )
adorsk Apr 17, 2018
b563c0b
scss lint fixup
adorsk Apr 17, 2018
6d1d463
lower default collections page size to 50
adorsk Apr 23, 2018
05157d5
revert .travis.yml change
adorsk Apr 23, 2018
92b1d4b
revert '-e' changes for requirements, no need for '-e' w/ bug fix fro…
adorsk Apr 24, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion odl_video/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,4 +537,4 @@ def get_all_config_keys():
# Paging parameters
PAGE_SIZE_QUERY_PARAM = get_string('PAGE_SIZE_QUERY_PARAM', 'page_size')
PAGE_SIZE_MAXIMUM = get_int('PAGE_SIZE_MAXIMUM', 1000)
PAGE_SIZE_COLLECTIONS = get_int('PAGE_SIZE_COLLECTIONS', 1000)
PAGE_SIZE_COLLECTIONS = get_int('PAGE_SIZE_COLLECTIONS', 50)
56 changes: 56 additions & 0 deletions static/js/actions/collectionsPagination.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// @flow
import { createAction } from "redux-actions"
import type { Dispatch } from "redux"
import * as api from "../lib/api"


const qualifiedName = (name: string) => `COLLECTIONS_PAGINATION_${name}`
const constants = {}
const actionCreators = {}

constants.REQUEST_GET_PAGE = qualifiedName("REQUEST_GET_PAGE")
actionCreators.requestGetPage = createAction(
constants.REQUEST_GET_PAGE)

constants.RECEIVE_GET_PAGE_SUCCESS = qualifiedName("RECEIVE_GET_PAGE_SUCCESS")
actionCreators.receiveGetPageSuccess = createAction(
constants.RECEIVE_GET_PAGE_SUCCESS)

constants.RECEIVE_GET_PAGE_FAILURE = qualifiedName("RECEIVE_GET_PAGE_FAILURE")
actionCreators.receiveGetPageFailure = createAction(
constants.RECEIVE_GET_PAGE_FAILURE)

actionCreators.getPage = (opts: {page: number}) => {
const { page } = opts
const thunk = async (dispatch: Dispatch) => {
dispatch(actionCreators.requestGetPage({page}))
try {
const response = await api.getCollections({pagination: {page}})
// @TODO: ideally we would dispatch an action here to save collections to
// a single place in state (e.g. state.collections).
// However, it take a non-trivial refactor to implement this schema
// change. So in the interest of scope, we store collections here.
// This will likely be confusing for future developers, and I recommend
// refactoring.
dispatch(actionCreators.receiveGetPageSuccess({
page,
count: response.count,
collections: response.results,
numPages: response.num_pages,
startIndex: response.start_index,
endIndex: response.end_index,
}))
} catch (error) {
dispatch(actionCreators.receiveGetPageFailure({
page,
error
}))
}
}
return thunk
}

constants.SET_CURRENT_PAGE = qualifiedName("SET_CURRENT_PAGE")
actionCreators.setCurrentPage = createAction(constants.SET_CURRENT_PAGE)

export { actionCreators, constants }
135 changes: 135 additions & 0 deletions static/js/actions/collectionsPagination_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// @flow
import { assert } from "chai"
import sinon from "sinon"

import * as api from "../lib/api"
import { constants, actionCreators } from "./collectionsPagination"


describe("collectionsPagination actions", () => {
let sandbox, dispatch

beforeEach(() => {
sandbox = sinon.sandbox.create()
dispatch = sandbox.spy()
})

afterEach(() => {
sandbox.restore()
})

describe("basic action creators", () => {
[
{
actionCreatorName: "requestGetPage",
constantName: "REQUEST_GET_PAGE",
},
{
actionCreatorName: "receiveGetPageSuccess",
constantName: "RECEIVE_GET_PAGE_SUCCESS",
},
{
actionCreatorName: "receiveGetPageFailure",
constantName: "RECEIVE_GET_PAGE_FAILURE",
},
].forEach((actionSpec) => {
it(`has ${actionSpec.actionCreatorName} actionCreator`, () => {
const payload = 'some payload'
const action = actionCreators[actionSpec.actionCreatorName](payload)
const expectedAction = {
type: constants[actionSpec.constantName],
payload,
}
assert.deepEqual(action, expectedAction)
})
})
})

describe("getPage", () => {
const page = 42
let stubs = {}

beforeEach(() => {
stubs = {
getCollections: sandbox.stub(
api, 'getCollections'),
requestGetPage: sandbox.stub(
actionCreators, 'requestGetPage'),
receiveGetPageSuccess: sandbox.stub(
actionCreators, 'receiveGetPageSuccess'),
receiveGetPageFailure: sandbox.stub(
actionCreators, 'receiveGetPageFailure'),
}
})

const _getPage = async () => {
return actionCreators.getPage({page})(dispatch)
}

it("dispatches REQUEST_GET_PAGE", async () => {
await _getPage()
sinon.assert.calledWith(
dispatch,
stubs.requestGetPage.returnValues[0]
)
})

it("makes api call", async () => {
await _getPage()
sinon.assert.calledWith(stubs.getCollections, {pagination: {page} })
})

describe("when api call succeeds", () => {
const response = {
count: 4242,
num_pages: 424242,
results: [...Array(3).keys()].map((i) => ({key: i})),
start_index: 99,
end_index: 999,
}

beforeEach(() => {
stubs.getCollections.returns(Promise.resolve(response))
})

it("dispatches RECEIVE_GET_PAGE_SUCCESS", async () => {
await _getPage()
sinon.assert.calledWith(
stubs.receiveGetPageSuccess,
{
page,
count: response.count,
collections: response.results,
numPages: response.num_pages,
startIndex: response.start_index,
endIndex: response.end_index,
}
)
sinon.assert.calledWith(
dispatch,
stubs.receiveGetPageSuccess.returnValues[0]
)
})
})

describe("when api call fails", async () => {
const error = 'some error'

beforeEach(() => {
stubs.getCollections.returns(Promise.reject(error))
})

it("dispatches RECEIVE_GET_PAGE_FAILURE", async () => {
await _getPage()
sinon.assert.calledWith(
stubs.receiveGetPageFailure,
{ error, page }
)
sinon.assert.calledWith(
dispatch,
stubs.receiveGetPageFailure.returnValues[0]
)
})
})
})
})
6 changes: 5 additions & 1 deletion static/js/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { deriveActions } from "redux-hammock"

import { endpoints } from "../lib/redux_rest"

const actions: Object = {}
import * as collectionsPagination from './collectionsPagination'

const actions: Object = {
collectionsPagination: collectionsPagination.actionCreators,
}
endpoints.forEach(endpoint => {
actions[endpoint.name] = deriveActions(endpoint)
})
Expand Down
67 changes: 67 additions & 0 deletions static/js/components/Paginator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// @flow
/* global SETTINGS: false */
import React from "react"


class Paginator extends React.Component<*, void> {
render () {
const { currentPage, totalPages } = this.props
return (
<div className="paginator">
<span className="paginator-current-range">
Page <span className="paginator-current-page">{currentPage}</span>
&nbsp;of&nbsp;
<span className="paginator-total-pages">{totalPages}</span>
</span>

<span className="paginator-buttons"
style={{
marginLeft: '.5em',
position: 'relative',
top: '-.1em',
}}
>
{ this.renderPrevNextButton("prev") }
{ this.renderPrevNextButton("next") }
</span>
</div>
)
}

renderPrevNextButton (nextPrevType: string) {
const { currentPage, totalPages, onClickPrev, onClickNext } = this.props
let iconKey
let disabled = true
let clickHandler = this.noop
if (nextPrevType === 'next') {
iconKey = 'chevron_right'
if (currentPage < totalPages) {
clickHandler = onClickNext
disabled = false
}
} else if (nextPrevType === 'prev') {
iconKey = 'chevron_left'
if (currentPage > 1) {
clickHandler = onClickPrev
disabled = false
}
}
let className = `paginator-button paginator-${nextPrevType}-button`
if (disabled) {
className += ' disabled'
}
return (
<span
className={className}
onClick={clickHandler}
disabled={disabled}
>
<i className="material-icons">{iconKey}</i>
</span>
)
}

noop () {}
}

export default Paginator
112 changes: 112 additions & 0 deletions static/js/components/Paginator_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// @flow
import React from "react"
import sinon from "sinon"
import { shallow } from "enzyme"
import { assert } from "chai"

import Paginator from "./Paginator"

describe("Paginator", () => {
let sandbox, stubs, props

beforeEach(() => {
sandbox = sinon.sandbox.create()
stubs = {
onClickNext: sandbox.spy(),
onClickPrev: sandbox.spy(),
}
props = {
currentPage: 42,
totalPages: 4242,
onClickNext: stubs.onClickNext,
onClickPrev: stubs.onClickPrev,
}
})

afterEach(() => {
sandbox.restore()
})

const renderComponent = (overrides = {}) => {
return shallow(<Paginator {...props} {...overrides} />)
}

it("shows current page", () => {
const wrapper = renderComponent()
assert.equal(
wrapper.find('.paginator-current-page').text(),
props.currentPage,
)
})

it("shows total pages", () => {
const wrapper = renderComponent()
assert.equal(
wrapper.find('.paginator-total-pages').text(),
props.totalPages,
)
})

describe("when in the middle of the full range", () => {

beforeEach(() => {
props = {
...props,
currentPage: 2,
totalPages: 3
}
})

it("triggers onClickNext when next button clicked", () => {
const wrapper = renderComponent()
sinon.assert.notCalled(stubs.onClickNext)
wrapper.find('.paginator-next-button').simulate('click')
sinon.assert.called(stubs.onClickNext)
})

it("triggers onClickPrev when prev button clicked", () => {
const wrapper = renderComponent()
sinon.assert.notCalled(stubs.onClickPrev)
wrapper.find('.paginator-prev-button').simulate('click')
sinon.assert.called(stubs.onClickPrev)
})
})

describe("when at the end of the full range", () => {
beforeEach(() => {
props = {
...props,
currentPage: 3,
totalPages: 3
}
})

it("disables the next button", () => {
const wrapper = renderComponent()
sinon.assert.notCalled(stubs.onClickNext)
const nextButton = wrapper.find('.paginator-next-button')
nextButton.simulate('click')
sinon.assert.notCalled(stubs.onClickNext)
assert.isTrue(nextButton.hasClass('disabled'))
})
})

describe("when at the beginning of the full range", () => {
beforeEach(() => {
props = {
...props,
currentPage: 1,
totalPages: 3
}
})

it("disables the prev button", () => {
const wrapper = renderComponent()
sinon.assert.notCalled(stubs.onClickPrev)
const prevButton = wrapper.find('.paginator-prev-button')
prevButton.simulate('click')
sinon.assert.notCalled(stubs.onClickPrev)
assert.isTrue(prevButton.hasClass('disabled'))
})
})
})
Loading