fix: trash page sorting and show more #4967
Conversation
| @@ -216,6 +216,10 @@ | |||
| // eslint-disable-next-line kolibri/vue-no-undefined-string-uses | |||
| return showMoreTranslator.$tr('showMore'); | |||
| }, | |||
| // "hasMore" returns true only if "more" is a nonempty plain object. | |||
There was a problem hiding this comment.
What I meant is that it's not null—not that it returns an empty object.
There was a problem hiding this comment.
Right, so I was wondering why this extra check was required, as it should either be null, or be an object with values.
There was a problem hiding this comment.
I usually write code with a future-proof mindset—anticipating that regardless of any backend changes, it will consistently return one of three possibilities: either an object with values, an empty object ( can be cases if we change something in code), or null.
I just like adding extra checks in the front-end to cover all conditions, making sure everything functions correctly even if the backend behavior changes in the future.
That extra check is a defensive coding measure to ensure the front-end logic remains robust.
I can remove it if you'd prefer.
contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js
Outdated
Show resolved
Hide resolved
| // Prevent further calls if "more" is falsy or already loading. | ||
| if (!this.more || this.moreLoading) return; | ||
| this.moreLoading = true; | ||
| // Capture current item count and clone "more" parameters. |
There was a problem hiding this comment.
so it is a temporary fix for this will resolve it after this pr seperately but just add a small fix for this currently
#4851
| @@ -541,6 +541,14 @@ class IndexedDBResource { | |||
| collection = collection.filter(filterFn); | |||
| } | |||
| if (paginationActive) { | |||
| // Default pagination field is 'lft' | |||
There was a problem hiding this comment.
You are making updates here, but there are no updates to make this configurable in the API itself. This will work if all of the results have already been loaded into IndexedDB, but if you are loading the trash page for the first time, I am not sure how this is meant to work.
See here how the pagination for contentnodes is hardcoded to be by lft: https://github.com/learningequality/studio/blob/unstable/contentcuration/contentcuration/viewsets/contentnode.py#L670
There was a problem hiding this comment.
done trying to fix it in backend but not sure did i did it right or not
|
Hi @GarvitSinghal47, is this ready for re-review? |
Yes just waiting for reply on this if confirmed i will remove it otherwise we can keep it as it is as well. |
Fix #4850
Summary
In this PR, I have updated the load children functionality to accept an ordering parameter, defaulting to lft. Additionally, the "show more" issue mentioned in #4851 is temporarily resolved by hiding the button if no new data is loaded after clicking it. I will investigate further and provide a comprehensive PR explaining why the network request sends data after all nodes have been loaded and resolve it in another pr seperately.
Screen-Recording.1.mp4
…
References
…
Reviewer guidance
…