Use server-side paging in aips page. (#209)

[mcantelon]

To avoid slow page responses when there are a large amount of AIPs use server-side paging to limit the number of AIP HTML sent to the browser.

[sevein]

Hi @mcantelon! This looks good to me. I haven't tested but I can see that what you're doing is going to bring a big improvement. I've made some small suggestions, but nothing important functionally speaking. Thank you!

[sevein]

Do you know if we can include query parameters directly in url_for, e.g. something like the following:

url_for('reporter.view_aips', page=pager.next_num, **state_querystring)

[mcantelon]

Nice! Yeah, that's cleaner.

[sevein]

You may want to use pytest.mark.parametrize, but I must say that sometimes I find pytest's parametrize not as legible as I wish. Yours is fine.

class MockPagination(object):
    pass

@pytest.mark.parametrize("page, per_page, total, expected_first_item, expected_last_item", [
    # Test paging window at start of results.
    (1, 5, 17, 1, 5),  
    # ...
    (3, 5, 17, 11, 15),  
    # ...
    (4, 5, 17, 16, 17),
    # add more scenarios as needed
])
def test_calculate_paging_window(page, per_page, total, expected_first_item, expected_last_item):
    pagination = MockPagination()
    pagination.per_page = per_page
    pagination.total = total

    first_item, last_item = helpers.calculate_paging_window(page, pagination)

    assert first_item == expected_first_item
    assert last_item == expected_last_item

[sevein]

Maybe a good use case for values.items(), e.g.:

def urlencode_without_none_values(values):
    """URL encode the values dict converting any None values to empty strings."""
    for key, value in values.items():
        if value is None:
            values[key] = ""
    return urlencode(values)

Here's an alternative using a dictionary comprenhension, I think this can be useful if you want to avoid mutating the input dictionary or doing it during iteration.

def urlencode_without_none_values(values):
    """URL encode the values dict converting any None values to empty strings."""
    return urlencode(
        {
            key: (value if value is not None else '')
            for key, value in values.items()
        }
    )

[sevein]

An alternative to calculate last_item that is more concise by using min():

Suggested change

	last_item = page * pagination.per_page
	if last_item > pagination.total:
	last_item = pagination.total
	last_item = min(page * pagination.per_page, pagination.total)

[mcantelon]

Very handy!

[sevein]

Should we handle ValueError when int doesn't receive a valid base 10 literal?

[mcantelon]

Thanks @sevein !

[mcantelon]

I've updated the PR @sevein .... thanks for the tips!

[sevein]

LGTM