Bug 29530

Created attachment 127965 [details] [review]
Bug 29530: Fix handling of NumSavedReports preference in reports table

This patch updates the way the NumSavedReports preference value is used
on the saved reports page so that the setting is correctly incorporated
into the DataTable.

The patch also expands the description of the NumSavedReports preference
in order to clarify the expected behavior when no value is saved.

To test, apply the patch and go to Administration -> System
preferences and note the value of NumSavedReports.

 - Go to Reports -> Use saved.
 - Confirm that the first page of saved reports shows the number
   specified in NumSavedReports.
 - In the "Show" dropdown menu, confirm that the number from
   NumSavedReports is preselected by default.
   - Expand the dropdown menu to confirm that the NumSavedReports number
     is positioned sequentially with the default values. For example, if
     NumSavedReports = "78," the menu options should be
     "10, 20, 50, 78, 100, All".
 - Test with various values of NumSavedReports. A blank value should
   result in the "All" option being selected. A non-numeric or
   non-positive value should result in the default set of options being
   used ("10, 20, 50, 100, All").

Created attachment 128016 [details] [review]
Bug 29530: Fix handling of NumSavedReports preference in reports table

This patch updates the way the NumSavedReports preference value is used
on the saved reports page so that the setting is correctly incorporated
into the DataTable.

The patch also expands the description of the NumSavedReports preference
in order to clarify the expected behavior when no value is saved.

To test, apply the patch and go to Administration -> System
preferences and note the value of NumSavedReports.

 - Go to Reports -> Use saved.
 - Confirm that the first page of saved reports shows the number
   specified in NumSavedReports.
 - In the "Show" dropdown menu, confirm that the number from
   NumSavedReports is preselected by default.
   - Expand the dropdown menu to confirm that the NumSavedReports number
     is positioned sequentially with the default values. For example, if
     NumSavedReports = "78," the menu options should be
     "10, 20, 50, 78, 100, All".
 - Test with various values of NumSavedReports. A blank value should
   result in the "All" option being selected. A non-numeric or
   non-positive value should result in the default set of options being
   used ("10, 20, 50, 100, All").

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 128172 [details] [review]
Bug 29530: Fix handling of NumSavedReports preference in reports table

This patch updates the way the NumSavedReports preference value is used
on the saved reports page so that the setting is correctly incorporated
into the DataTable.

The patch also expands the description of the NumSavedReports preference
in order to clarify the expected behavior when no value is saved.

To test, apply the patch and go to Administration -> System
preferences and note the value of NumSavedReports.

 - Go to Reports -> Use saved.
 - Confirm that the first page of saved reports shows the number
   specified in NumSavedReports.
 - In the "Show" dropdown menu, confirm that the number from
   NumSavedReports is preselected by default.
   - Expand the dropdown menu to confirm that the NumSavedReports number
     is positioned sequentially with the default values. For example, if
     NumSavedReports = "78," the menu options should be
     "10, 20, 50, 78, 100, All".
 - Test with various values of NumSavedReports. A blank value should
   result in the "All" option being selected. A non-numeric or
   non-positive value should result in the default set of options being
   used ("10, 20, 50, 100, All").

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

It's working but we need to make this code generic.

First the NumSavedReports syspref should be removed in favour of the "default display length" table setting we now have.
Then it would be actually great to make this setting work for all the tables, everywhere KohaTable is used. Owen, is that something you would like to work on? I could help of course.

(In reply to Jonathan Druart from comment #4)

> First the NumSavedReports syspref should be removed in favour of the
> "default display length" table setting we now have.

Bug 17465 didn't have any discussion on it about why we needed a separate system preference, but since the numSearchResults predates it I think we must assume that the intention of Bug 17465 was to create an exception to the numSearchResults setting.

On the other hand, perhaps the need for NumSavedReports is not as great now that we have better searching via DataTables?

If we have a generic way of doing this for all tables with DataTables, then we should probably do that - whatever way, it should work.

I'm all for making things as consistent as possible in the UI, and one less system preference is probably a good thing!

Maybe a separate bug, since you have got this back working...

(In reply to David Nind from comment #6)
> If we have a generic way of doing this for all tables with DataTables, then
> we should probably do that - whatever way, it should work.
> 
> I'm all for making things as consistent as possible in the UI, and one less
> system preference is probably a good thing!
> 
> Maybe a separate bug, since you have got this back working...

That's my thought too - let's make the move to table configuration but treat this as a bug fix until we have the better solution.

(In reply to Katrin Fischer from comment #7)

> That's my thought too - let's make the move to table configuration but treat
> this as a bug fix until we have the better solution.

+1

Created attachment 128644 [details] [review]
Bug 29530: Fix handling of NumSavedReports preference in reports table

This patch updates the way the NumSavedReports preference value is used
on the saved reports page so that the setting is correctly incorporated
into the DataTable.

The patch also expands the description of the NumSavedReports preference
in order to clarify the expected behavior when no value is saved.

To test, apply the patch and go to Administration -> System
preferences and note the value of NumSavedReports.

 - Go to Reports -> Use saved.
 - Confirm that the first page of saved reports shows the number
   specified in NumSavedReports.
 - In the "Show" dropdown menu, confirm that the number from
   NumSavedReports is preselected by default.
   - Expand the dropdown menu to confirm that the NumSavedReports number
     is positioned sequentially with the default values. For example, if
     NumSavedReports = "78," the menu options should be
     "10, 20, 50, 78, 100, All".
 - Test with various values of NumSavedReports. A blank value should
   result in the "All" option being selected. A non-numeric or
   non-positive value should result in the default set of options being
   used ("10, 20, 50, 100, All").

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Rebased on top of bug 29488.

Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑

Pushed to 21.11.x for 21.11.02

Pushed to 21.05.x for 21.05.09

Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.