learningequality / learningequality/studio

Change cursor behaviour for content node pagination to use `id` instead of `lft` for consistent frontend and backend ordering

Open
#5,021 0 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
191
Forks
307
Avg merge
5d 6h
Merged PRs (30d)
10

Description

This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.

Observed behavior

We currently update lft values in the frontend in order to optimistically update content node ordering when moving or adding new nodes.
This causes issues because when we try to use the lft values for pagination, there is now a mismatch between the frontend's lft values and the lft values in the backend. This is most evident when the lft values are floats in the frontend, but in the backend must always be integers.

Expected behavior

Regardless of whether the lft value in the frontend and backend are in sync, when we make a pagination call, we should be getting the subsequent resources.

We should achieve this by using the contentnode id as the pagination cursor, so that on the frontend and on the backend we look up that specific node, and then use the local value of lft in order to do pagination.

This could also later be extended to having a contextually defined ordering - so if we were in the trash page, we could use the modified date instead of lft to order the pagination instead.

User-facing consequences

At worst, the pagination request returns a 400 because of a float lft value. Less severely the pagination request could simply be inconsistent because the frontend and backend lft values are not in sync.

Additional information

Ultimately, this may be better resolved by using a different tree algorithm for contentnodes - using something like a materialized path algorithm, for example, would allow us to define only an entirely sibling scoped sort order, which would be updated in both the frontend and the backend, and there would be less chance of the two getting out of sync (any difference would instead be a result of changes not yet being applied on the backend). However, this would be a very large undertaking, and this should be a shorter term and simpler fix!

Steps to reproduce the issue

Create a topic with sufficient resources to require pagination.
Logout and log back in again to clear indexedDB.
Move multiple resources within the paginated resources and then quickly click "show more" - this should either give an inconsistent response or a 400.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the content node pagination request and its frontend/backend cursor handling described here, including the move-and-quickly-show-more reproduction. Done means pagination uses the content node id to resolve local lft ordering and returns subsequent resources consistently without the float-related 400.

Written by the indexing model from the issue text.

Assessment

Domain
full-stack
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.