Jump to content

Hard-coded max session bookmarks


MarkE
 Share

Recommended Posts

I have been making use of the ProcessPageLister::addSessionBookmark() method to create drill-downs of financial data in a table - so that the user can see the pages comprising the amount in the table cell. This works really well and has been well-received by the client. However, I soon ran into a problem in that the method sets $maxBookmarks = 30;  hard-coded just like that. I changed the code to $maxBookmarks = 400; and haven't noticed any ill-effects so far, so some questions arise:

  1. Why is it hard-coded rather than modifiable?
  2. If the answer to (1) is because too many bookmarks cause a problem, what is the problem?
  3. If there is no particular problem with more than 30 bookmarks, is there any other practical limit?
  4. Clearly changing the code in my copy of PW is not a good idea as it will get over-written at the next update. Is there a way of dealing with this until (if ever) the variable can be modified through the API?
Link to comment
Share on other sites

38 minutes ago, MarkE said:

Clearly changing the code in my copy of PW is not a good idea as it will get over-written at the next update. Is there a way of dealing with this until (if ever) the variable can be modified through the API?

I don't know why the limit is hardcoded, but I can offer a workaround for this. If you want to modify a core module without having to worry about updates, just copy the entire module folder / file from the core to your site's module directory (/site/modules/). Then execute Modules -> Refresh through the CMS. This will create a notice about the duplicate module asking you to select which module file you use. There you can select the copied module. Once whatever is not working is fixed in the core, just change that setting back and delete the duplicated module.

You still have to check for changes inside the core module when you update the core, in case your older copied version is not compatible anymore. In this case, just copy the module again and reapply your changes.

  • Like 5
Link to comment
Share on other sites

3 hours ago, MoritzLost said:

just copy the entire module folder / file from the core to your site's module directory

Done that, thanks. Any views on whether this should be raised as a fix, or if the hard-coded max is there for a good reason? The fix is very simple - just add an argument $max=30 to the method and set $maxBookmarks = $max;

Link to comment
Share on other sites

Some sorts of hypotheses.

 

1 - Yes, too many bookmarks could cause problems ??

2 - The potential problem is: the size of the data stored in this session (session are written to disk, and re-write every request) ??

3 - The practical limit will be the memory that PHP can take on the server

4 - What @MoritzLost explained

 

I hope it give you an idea of the potential issue with a lots of pages.

  • Like 1
Link to comment
Share on other sites

I guess the alternative is to put the call in Process module so that the Listers are created on the fly. However, the load time for a table of 300+ cells is currently 1.5 sec, of  which I suspect only a fraction is for creating the Listers. I may give it a try to see which is best.

Update: using the module cuts the time to 1.4 sec on my dev m/c (there's a lot of processing to produce the table anyway) and makes no appreciable difference to the time to open the drill-down lister (if anything, it's a bit faster). So I can ditch my mod to ProcessPageLister, but I'm still curious about the limit.

Link to comment
Share on other sites

Yes, perfect use case for it! RockGrid still works, but I'll not develop it further. I switched to tabulator.info some time ago, because ag-grid (used by RockGrid) is not fully open source and quite expensive if you need excel-export for example.

I have to note that I want to rebuild RockTabulator soon, but not sure when I can manage to do that. If anybody has a project that could benefit from such listings and would be willing to outsource the work and sponsor the development of a new (and well documented) RockTabulator write me a message ? 

PS: If you want to keep things simple, you could even use your own implementation of tabulator.info or ag-grid. RockFinder2 will do the "hard" work of getting the data and you can simply display that data in a nice way with those js libraries. If you don't need any editing stuff, this should be really quick and easy!

  • Like 2
Link to comment
Share on other sites

Thanks @bernhard. Tabulator certainly looks interesting. My use case is slightly different from the standard table, however. It is a 2-dimensional tabulation of summary data (each cell based on the sum of values meeting the criteria of both column and row). Each cell than has a link to a process module which takes the criteria and turns them into a Lister session bookmark, to which it redirects, thus providing a drill down for the components of each cell. I'm not sure that tabulator handles this, but it does look like a potential enhancement of the standard MarkupAdminDataTable.

See image:

 

Financials.jpg

Link to comment
Share on other sites

5 minutes ago, MarkE said:

Each cell than has a link to a process module which takes the criteria and turns them into a Lister session bookmark, to which it redirects, thus providing a drill down for the components of each cell.

I don't understand this part. Doesn't that mean you could replace the lister bookmarks by a process module showing a RockTabulator and this RockTabulator would show data based on the 2 get-parameters? eg /your-admin/financials/?income=2015&cash=2018

This would mean unlimited number of RockTabulators with just one process module instead of one lister per income/cash combination?

  • Like 1
Link to comment
Share on other sites

4 minutes ago, bernhard said:

RockTabulator would show data based on the 2 get-parameters? eg /your-admin/financials/?income=2015&cash=2018

It's actually a bit more complicated than that at the moment, because a urlencode(serialize($array))  is the get parameter with all the data for the lister session bookmark, as the latter is general-purpose (there is more than one 2-dim table and they have different features). I don't see that tabulator replaces the 2-dim display I showed in the image, but it might make for a better drill-down display instead of the lister. I'll take a look into that while the client is user-testing the current version.

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...