Hi! Some rules are hidden when a syspref is disabled (like the rules for recalls, and the article request fee and limit of open article requests), but even if the ArticleRequests system preference is disabled, there is still an "Article requests" column in the big circulation rules matrix. Like the recalls rules and the other article requests rules, this column should be hidden if the system preference is disabled. To reproduce: 1) With ArticleRequests disabled, go to Administration > Circulation and fines rules 2) Scroll right in the first table --> There should be an article requests yes/no column (this should be hidden) 3) Scroll down to see the other tables in the page --> There are no other article requests tables (this is ok) 4) Enable ArticleRequests in Administration > System preferences 5) Go back to the Circulation and fines rules and refresh the page 6) 2) Scroll right in the first table --> There should be an article requests yes/no column (this is ok) 7) Scroll down to see the other tables in the page --> There are other article requests tables, Default open article requests limit and Default article request fees (this is ok) At step 2, users don't need to see the Article request column since the system preference is disabled.
Created attachment 138541 [details] [review] Bug 31289: Hide Article requests column in circulation rules when ArticleRequests syspref is disabled This patch hides the Article requests column in the large circulation rules table when the ArticleRequests system preference is disabled. To test: 0) Apply patch 1) With ArticleRequests disabled, go to Administration > Circulation and fines rules 2) Scroll right in the first table --> There isn't any mention of article requests in the table 3) Scroll down to see the other tables in the page --> There are no other article requests tables 4) Enable ArticleRequests in Administration > System preferences 5) Go back to the Circulation and fines rules and refresh the page 6) Scroll right in the first table --> There should be an article requests yes/no column 7) Scroll down to see the other tables in the page --> There are other article requests tables, Default open article requests limit and Default article request fees
I did some copy-pasting from the UseRecalls mentions in smart-rules.tt... I hope it's ok
Created attachment 138544 [details] [review] Bug 31289: Hide Article requests column in circulation rules when ArticleRequests syspref is disabled This patch hides the Article requests column in the large circulation rules table when the ArticleRequests system preference is disabled. To test: 0) Apply patch 1) With ArticleRequests disabled, go to Administration > Circulation and fines rules 2) Scroll right in the first table --> There isn't any mention of article requests in the table 3) Scroll down to see the other tables in the page --> There are no other article requests tables 4) Enable ArticleRequests in Administration > System preferences 5) Go back to the Circulation and fines rules and refresh the page 6) Scroll right in the first table --> There should be an article requests yes/no column 7) Scroll down to see the other tables in the page --> There are other article requests tables, Default open article requests limit and Default article request fees Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Please don't forget to set Assignee! :)
Created attachment 138688 [details] [review] Bug 31289: Hide Article requests column in circulation rules when ArticleRequests syspref is disabled This patch hides the Article requests column in the large circulation rules table when the ArticleRequests system preference is disabled. To test: 0) Apply patch 1) With ArticleRequests disabled, go to Administration > Circulation and fines rules 2) Scroll right in the first table --> There isn't any mention of article requests in the table 3) Scroll down to see the other tables in the page --> There are no other article requests tables 4) Enable ArticleRequests in Administration > System preferences 5) Go back to the Circulation and fines rules and refresh the page 6) Scroll right in the first table --> There should be an article requests yes/no column 7) Scroll down to see the other tables in the page --> There are other article requests tables, Default open article requests limit and Default article request fees Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This makes things more consistent and simplifies for those not using Article requests, so I am going to pass it. There might be some discussion to be had if it makes sense to be able to configure a feature without having it exposed to users immediately. But that's complicated and should probably be a general decision (maybe something like having 3 options for the prefs? (Enable config|Enable feature|Disable)
(In reply to Katrin Fischer from comment #6) > This makes things more consistent and simplifies for those not using Article > requests, so I am going to pass it. > > There might be some discussion to be had if it makes sense to be able to > configure a feature without having it exposed to users immediately. But > that's complicated and should probably be a general decision (maybe > something like having 3 options for the prefs? (Enable config|Enable > feature|Disable) With the new table (circulation_rules) and the granularity it provides, I think we should move article requests-specific rules to their own section, and probably collapse them by default if the feature is disabled, but editable. For the same reasons you mentioned.
Article request is only one column - so I think we should not start the discussion here. Having separate for all suspension/fine, holds, recalls etc. would certainly make sense, but this is a small enhancement that fits the current pattern well. The other is reopening the discussion on a new editor for circulation rules, which is needed, but out of scope.
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.06
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Maybe something to document.
Thanks! Pushed to 21.11 for 21.11.12