Skip to content
This repository has been archived by the owner on Feb 6, 2019. It is now read-only.

Add latest route #103

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Add latest route #103

wants to merge 26 commits into from

Conversation

ezmiller
Copy link
Collaborator

@ezmiller ezmiller commented Jul 25, 2016

Adds a latest view to the beavy core accessible at /latest. This view loads a paginated list of items in the object database, and presents them in an infinite scrolling context.

@ezmiller
Copy link
Collaborator Author

ezmiller commented Jul 25, 2016

@ligthyear here’s the code that (seems) to get us in the right direction. The way the data is being packaged up in the post_dump on the pagination schema is still not quite right. I’m not totally sure what the links prop that’s being return on that object is supposed to contain. I'll add some notes in-line. The main change is of course the MorphingNested that we are using.

@ezmiller ezmiller closed this Jul 25, 2016

def _serialize(self, nested_obj, attr, obj):
rv = [self._get_serializer(obj).dump(obj).data
for obj in nested_obj]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure at all that this is sufficient for the wider range of cases that this function might have to handle. Thoughts?

@ezmiller
Copy link
Collaborator Author

ezmiller commented Aug 4, 2016

@ligthyear whenever you have time to look at this, i'm still working on it.

@ezmiller ezmiller reopened this Aug 4, 2016
Without these fields added marshamllow_jsonapi was throwing
errors asking for these. This may be a hack solution and we
may remove later, or entirely replace the marshamllow_jsonapi
with ripozo.
Was not the right fix.

This reverts commit 056c917.
Was getting error related to order of arguments: "got multiple
values for argument...", in this case the two keyword args. This
seems to fix it. See https://stackoverflow.com/questions/28336270/how-to-fix-got-multiple-values-for-argument-error-for-args-and-kwargs
This seems to be the right way to set the key, which on the
front end maps to the Redux reducer. The default is None,
which has no reducer obviously. We might want to think of
some better ways of handling the case where the key does
not match an existing reducer. Perhaps the default should be
something other than None.
This extracts the nested data in a way that can
be parsed by the front end. Just returning empty
links here because not yet supporting pagination links.
Lots here that is probably not right:
- Not paginated, ignores pagination data
- Is the state.entities object being used the way it is supposed to be? No.
This was causing links to load incorrectly and should not be hardcoded
anyway probably. Default is half of the container size.
Need isEqual rather than just the default = operator b/c redux
always returns new objects even when props have not changed.
So this was wrong sometimes with ==.
Switching to this for now because we were just calculating the containerHeight
according to the window anyway, and when using this we can also make the
import preloadBatchSize automatic in relation to the container height.
Needed for the react-infinite Infinite component, determines
whether the loading indicator is visible or not.
This was important b/c supplying "undefined" to the infiniteLoadingBeginEdgeOffset
prop is how you disable the infinite loading. Because this was  wrong the Infinite
component was trying to load more and that was causing the loading indicator to
appear even at the end of the pagination list.
@ezmiller
Copy link
Collaborator Author

ezmiller commented Aug 28, 2016

@ligthyear this PR (with the /latest route added) should be ready now. Can you review?

@ezmiller
Copy link
Collaborator Author

ezmiller commented Oct 7, 2016

@gnunicorn we might as well get this in, no?

@gnunicorn
Copy link
Member

@ezmiller yes, sure. Sorry, for the delay. I'll review it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants