Feature #13320

Increase treeview pager limit setting from 1,000 to a max value of 10,000

Added by Dan Gillean 5 months ago. Updated 4 months ago.

Status:VerifiedStart date:05/18/2020
Priority:MediumDue date:
Assignee:-% Done:

100%

Category:TreeviewEstimated time:16.00 hours
Target version:Release 2.6.0
Google Code Legacy ID: Tested version:2.5, 2.6
Sponsored:Yes Requires documentation:

Description

In issue #12611 we added paging to the full-width treeview to help address scalability issues with large hierarchies, when there were many immediate children under a top-level description. Unfortunately, this change introduced a number of unexpected issues, captured in #13186.

The solution added in #13186 was a workaround, adding a new option to allow users to configure the amount of records loaded per page - previously it was hard-coded at 50; the new setting allows users to raise this limit to a maximum value of 1,000. However, the underlying issue identified in #13186 was not actually addressed.

AtoM wishlist ticket #13293 includes an attached PDF that fully describes the history of these changes, their rationale, and the resulting issues. It concludes by proposing two development options to fully resolve the issue: adding a setting to allow users to disable paging entirely if desired, and adding support for bi-directional paging.

This enhancement was originally going to add the first of these options - a new administrative setting to enable or disable paging in the treeview. However, instead, this enhancement now allows the treeview pager setting value to be set as high as 10,000. For users who would like to set the value even higher, changes can be made by adding the following line to apps/qubit/config/app.yml:

  treeview_items_per_page_max: 100000

Note: After the change, system administrators should clear the previously cached value by running php symfony cc and restarting the PHP-FPM and memcached (if used) services.

biggar-photo-collection.csv Magnifier (2.13 MB) Dan Gillean, 06/23/2020 05:42 PM


Related issues

Related to Access to Memory (AtoM) - Feature #12611: Add paging to full-width treeview for top-level records w... Verified 12/03/2018
Related to Access to Memory (AtoM) - Bug #13186: Full width treeview autoload not working for descriptions... Verified 09/25/2019
Related to AtoM Wishlist - Feature #13293: Improve full-width paging options for large hierarchies New 04/24/2020

History

#1 Updated by Dan Gillean 5 months ago

  • Related to Feature #12611: Add paging to full-width treeview for top-level records with more than 50 immediate children, to improve performance added

#2 Updated by Dan Gillean 5 months ago

  • Related to Bug #13186: Full width treeview autoload not working for descriptions outside the initial limit added

#3 Updated by Dan Gillean 5 months ago

  • Related to Feature #13293: Improve full-width paging options for large hierarchies added

#4 Updated by Dan Gillean 5 months ago

  • Target version deleted (Release 2.6.0)

#5 Updated by David Juhasz 4 months ago

I've submitted a pull request (PR1126) that increases the maximum value of the "items per page" treeview setting to 10,000 items, and allows higher values via a config setting in app.yml.

I think allowing setting the "items per page" setting to arbitrarily large value is preferable to introducing a new toggle because:

  1. It requires fewer code changes to than adding a new setting, and adding logic to turn off the treeview paging logic altogether Because there is less code being added, it's also easier to maintain the feature going forward,
  2. It avoids adding another new setting to AtoM, which prevents adding UI clutter,
  3. It avoids management of the interaction between the "items per page" and "enable/disable paging" settings, e.g. disabling the "limit per page" setting in the UI when paging is toggled off,
  4. At some point having no limit on the items loaded in the treeview is going to trigger fatal errors in AtoM due to exceeding timeout or memory limits. I think having some limit, even if it is very large, is better than removing the limit altogether

#6 Updated by David Juhasz 4 months ago

  • Status changed from New to Code Review

#7 Updated by David Juhasz 4 months ago

  • Assignee set to David Juhasz

And, Radda has already approved the PR, so this is back to me to merge. :)

#8 Updated by David Juhasz 4 months ago

  • Status changed from Code Review to QA/Review
  • Assignee changed from David Juhasz to Sarah Mason

The ability to increase the fullwidth treeview "items per page" max has been merged to qa/2.6.x (commit c53d257).

Note, to increase the limit above the 10 000 item default, add the following line to apps/qubit/config/app.yml

  treeview_items_per_page_max: 100000

After the change clear the cached value by doing symfony cc and restarting the php-fpm and memcached (if used) services.

#9 Updated by José Raddaoui Marín 4 months ago

  • Target version set to Release 2.6.0

#10 Updated by Dan Gillean 4 months ago

  • Subject changed from Add new administrative setting to disable paging in full-width treeview to Increase treeview pager limit setting from 1,000 to a max value of 10,000
  • Description updated (diff)

#11 Updated by Dan Gillean 4 months ago

Hi David!

The change you made seems to work for increasing the pager limit. However, I'm wondering if you can quickly investigate a related issue, that might be a regression?

Essentially, when we first investigated the original pager issues in ticket #13186, we discovered 2 distinct issues - one was the treeview focus (making sure the target record was actually in the frame and highlighted), and the other was the problems the paging was causing.

In my test, it seems like the paging limit change is properly loading up to 10K records when arriving on a top-level description.

However, if I search for a lower-level description and navigate straight to it, we're back to the original problem reported in #13186 - the treeview remains collapsed and the selected node is not visible. Wondering if this is something we can address?

I'm attaching a CSV for testing that has a collection level record, and thousands of lower-level item descriptions. To reproduce:

  • Import the CSV
  • Find the description, and make sure everything from the collection level down is set to published
  • Open a separate incognito browser and go to your test site
  • Search for Verla Tapp
  • Click on the result, and observe the treeview

See also my comments on #13186 at note-8 about what was working with Mike's changes, for context. I was hoping the same would be the case here, i guess. Not sure if this PR is related to this change, or if it's something deeper?

#12 Updated by David Juhasz 4 months ago

I'll look into it Dan. :-/

#13 Updated by David Juhasz 4 months ago

I've confirmed that the fullwidth treeview won't focus on a the current item if that item comes after the 1000th item (e.g. item index > 1000). It looks like a 1000 item hard cap is enforced in a in a few places in the treeview code that I missed.

#14 Updated by David Juhasz 4 months ago

This PR should fix the hard cap for focusing the current tree node: https://github.com/artefactual/atom/pull/1136

#15 Updated by David Juhasz 4 months ago

  • Status changed from Feedback to QA/Review
  • Assignee changed from David Juhasz to Dan Gillean

#16 Updated by Dan Gillean 4 months ago

  • Status changed from QA/Review to Verified

It works!

#17 Updated by Dan Gillean 4 months ago

  • Assignee deleted (Dan Gillean)
  • % Done changed from 0 to 100
  • Requires documentation deleted (Yes)

Also available in: Atom PDF