Bug 5087 - Option to not show CSV profiles in OPAC
Summary: Option to not show CSV profiles in OPAC
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Aleisha Amohia
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on: 25402
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-04 11:27 UTC by Katrin Fischer
Modified: 2021-12-13 21:08 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This patch adds an option to show or not show a CSV profile in the OPAC cart and lists download formats list.
Version(s) released in:
20.11.00, 20.05.03
Circulation function:


Attachments
download option in opac's cart (16.77 KB, image/png)
2010-08-04 11:27 UTC, Katrin Fischer
Details
Bug 5087: Ability to define whether CSV profile shows in OPAC (4.79 KB, patch)
2018-12-20 22:59 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (4.85 KB, patch)
2019-01-04 21:28 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (5.56 KB, patch)
2020-02-12 21:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (6.79 KB, patch)
2020-04-22 02:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (6.79 KB, patch)
2020-04-22 21:13 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (6.83 KB, patch)
2020-04-29 14:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (6.80 KB, patch)
2020-07-20 14:59 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: (QA follow-up) Rename the attribute to fit later API usage (7.75 KB, patch)
2020-07-20 14:59 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: (QA follow-up) used_for deserves an index (1.85 KB, patch)
2020-07-20 14:59 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: (QA follow-up) Fix leakage in opac-downloadshelf.pl (1.29 KB, patch)
2020-07-20 15:02 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: Ability to define whether CSV profile shows in OPAC (6.80 KB, patch)
2020-07-21 17:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: (QA follow-up) Rename the attribute to fit later API usage (7.73 KB, patch)
2020-07-21 17:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: (QA follow-up) used_for deserves an index (1.85 KB, patch)
2020-07-21 17:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: (QA follow-up) Fix leakage in opac-downloadshelf.pl (1.29 KB, patch)
2020-07-21 17:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 5087: Add server-side check (1.87 KB, patch)
2020-07-23 09:17 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2010-08-04 11:27:03 UTC
Created attachment 2450 [details]
download option in opac's cart

Currently CSV profiles defined in staff are shown in the pull down of download options in opac's cart.

I would like an option to only show those CSV profiles in staff.

More complex but even better would be a system where I can define for each CSV profile wether it will be shown in OPAC or only in staff.
Comment 1 Aleisha Amohia 2018-12-20 22:59:25 UTC Comment hidden (obsolete)
Comment 2 Maryse Simard 2019-01-04 21:28:50 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2019-01-27 12:12:01 UTC
Hi Aleisha, 
please don't forget to change kohastructure.sql as well.
Comment 4 Aleisha Amohia 2020-02-12 21:34:52 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2020-02-14 18:08:10 UTC
I get this warning from the QA tool:

 FAIL   installer/data/mysql/kohastructure.sql
   OK     boolean_vs_tinyint
   OK     charset_collate
   OK     git manipulation
   FAIL   tinyint_has_boolean_flag
                The new column export_format.opac_option is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/ExportFormat.pm), see the SQL12 coding guideline
Comment 6 Aleisha Amohia 2020-02-17 01:47:02 UTC
(In reply to Owen Leonard from comment #5)
> I get this warning from the QA tool:
> 
>  FAIL   installer/data/mysql/kohastructure.sql
>    OK     boolean_vs_tinyint
>    OK     charset_collate
>    OK     git manipulation
>    FAIL   tinyint_has_boolean_flag
>                 The new column export_format.opac_option is using TINYINT(1)
> as type but is not defined as boolean in the schema file
> (Koha/Schema/Result/ExportFormat.pm), see the SQL12 coding guideline

Hi Owen, did you update the schema files? 

misc/devel/update_dbix_class_files.pl --db_name=koha_instance --db_user=koha_instance --db_passwd=thepassword
Comment 7 Aleisha Amohia 2020-04-22 02:16:36 UTC Comment hidden (obsolete)
Comment 8 David Nind 2020-04-22 09:42:35 UTC
Hi Aleisha.

I get this error when trying to do the database update:

root@eb7cfcd62ddf:koha(bz5087)$ perl installer/data/mysql/updatedatabase.pl
{UNKNOWN}: Can't locate object method "add_columns" via package "PACKAGE__" (perhaps you forgot to load "PACKAGE__"?) at /kohadevbox/koha/Koha/Schema/Result/ExportFormat.pm line 159.
Compilation failed in require at /usr/share/perl5/Class/C3/Componentised.pm line 150. at /usr/share/perl5/Class/C3/Componentised.pm line 155
Compilation failed in require at /kohadevbox/koha/Koha/Database.pm line 50.
Comment 9 Aleisha Amohia 2020-04-22 21:13:19 UTC
Created attachment 103506 [details] [review]
Bug 5087: Ability to define whether CSV profile shows in OPAC

To test:
1) Update database
2) Update schema
3) In staff side, go to Tools -> CSV profiles
4) Make a CSV profile with profile type = MARC and usage = Export
records
5) Leave 'Show option in OPAC' unchecked and save
6) Go to OPAC and add an item to your cart
7) Click Cart, click Download, confirm that your newly created CSV
profile does not show as an option
8) Go back and edit CSV profile on staff side
9) Check the 'Show option in OPAC' checkbox and save
10) Go back to download cart in OPAC
11) Confirm the CSV profile now shows in the dropdown

Sponsored-by: Catalyst IT
Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 10 David Nind 2020-04-23 11:46:50 UTC
Thanks Aleisha, the database update now works.

Everything now works as per the test plan.

However, when testing I tried to turn off the 'Show option in OPAC' setting and I get this error message:

  "An error occurred when updating this CSV profile. Perhaps it already exists."

This appears in red at the top of the CSV profiles page after pressing the submit button.

I can successfully make changes to any of the other form elements for the CSV profile.
Comment 11 Aleisha Amohia 2020-04-23 21:12:10 UTC
Thanks for testing David, but unfortunately I'm not getting that error! Perhaps a data problem?

Setting back to Needs signoff in case this is working as expected for someone else.
Comment 12 Owen Leonard 2020-04-29 14:53:33 UTC
Created attachment 103955 [details] [review]
Bug 5087: Ability to define whether CSV profile shows in OPAC

To test:
1) Update database
2) Update schema
3) In staff side, go to Tools -> CSV profiles
4) Make a CSV profile with profile type = MARC and usage = Export
records
5) Leave 'Show option in OPAC' unchecked and save
6) Go to OPAC and add an item to your cart
7) Click Cart, click Download, confirm that your newly created CSV
profile does not show as an option
8) Go back and edit CSV profile on staff side
9) Check the 'Show option in OPAC' checkbox and save
10) Go back to download cart in OPAC
11) Confirm the CSV profile now shows in the dropdown

Sponsored-by: Catalyst IT
Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 13 Katrin Fischer 2020-06-11 13:14:04 UTC
Hi Aleisha,

thanks a lot for taking these on!

1) With this patch we will hide any CSV profiles now shown in the OPAC. I think we should add an update for the existing CSV profiles to check the checkbox, so that we have no change of behaviour on update.

2) Can you please make the same change for the lists download? That should then cover all bases!
Comment 14 Tomás Cohen Arazi (tcohen) 2020-07-20 14:59:42 UTC Comment hidden (obsolete)
Comment 15 Tomás Cohen Arazi (tcohen) 2020-07-20 14:59:51 UTC Comment hidden (obsolete)
Comment 16 Tomás Cohen Arazi (tcohen) 2020-07-20 14:59:59 UTC Comment hidden (obsolete)
Comment 17 Tomás Cohen Arazi (tcohen) 2020-07-20 15:02:36 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2020-07-21 14:41:04 UTC
Applying: Bug 5087: (QA follow-up) Rename the attribute to fit later API usage
error: sha1 information is lacking or useless (installer/data/mysql/kohastructure.sql).
error: could not build fake ancestor
Comment 19 Tomás Cohen Arazi (tcohen) 2020-07-21 17:17:11 UTC
Created attachment 107166 [details] [review]
Bug 5087: Ability to define whether CSV profile shows in OPAC

To test:
1) Update database
2) Update schema
3) In staff side, go to Tools -> CSV profiles
4) Make a CSV profile with profile type = MARC and usage = Export
records
5) Leave 'Show option in OPAC' unchecked and save
6) Go to OPAC and add an item to your cart
7) Click Cart, click Download, confirm that your newly created CSV
profile does not show as an option
8) Go back and edit CSV profile on staff side
9) Check the 'Show option in OPAC' checkbox and save
10) Go back to download cart in OPAC
11) Confirm the CSV profile now shows in the dropdown

Sponsored-by: Catalyst IT
Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Tomás Cohen Arazi (tcohen) 2020-07-21 17:17:17 UTC
Created attachment 107167 [details] [review]
Bug 5087: (QA follow-up) Rename the attribute to fit later API usage

This patch renames the introduced attribute for later usage on the API,
and changes the logic to use it the other way around.

It also adds a KEY for the flag, as it will be used in WHERE statements.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Tomás Cohen Arazi (tcohen) 2020-07-21 17:17:22 UTC
Created attachment 107168 [details] [review]
Bug 5087: (QA follow-up) used_for deserves an index

This patch introduces an index for the used_for column

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 22 Tomás Cohen Arazi (tcohen) 2020-07-21 17:17:28 UTC
Created attachment 107169 [details] [review]
Bug 5087: (QA follow-up) Fix leakage in opac-downloadshelf.pl

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Tomás Cohen Arazi (tcohen) 2020-07-21 17:18:26 UTC
Rebased the follow-up, boss.
Comment 24 Jonathan Druart 2020-07-22 08:30:51 UTC
(In reply to Tomás Cohen Arazi from comment #20)
> Created attachment 107167 [details] [review] [review]
> Bug 5087: (QA follow-up) Rename the attribute to fit later API usage
> 
> This patch renames the introduced attribute for later usage on the API,
> and changes the logic to use it the other way around.
> 
> It also adds a KEY for the flag, as it will be used in WHERE statements.
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Can you explain a bit more Tomas?
Should not we usually provide "OPAC only" by default, and explicitly pass the "staff" flag?
Comment 25 Tomás Cohen Arazi (tcohen) 2020-07-22 11:24:40 UTC
(In reply to Jonathan Druart from comment #24)
> 
> Can you explain a bit more Tomas?
> Should not we usually provide "OPAC only" by default, and explicitly pass
> the "staff" flag?

This is not 'opac only', this is like the 'public' flag on reports, i.e. 'staff only', and the author named it 'opac_display'. The original naming also required to pass it on the query in the OPAC controllers, I just thought it was better to name it different, and so the flipped value also. The default behavior is kept with my follow-up and DB default values.
Comment 26 Katrin Fischer 2020-07-22 20:26:49 UTC
Tomas is right, it makes more sense this way. We don't want 'opac only' we want 'staff only'. Usually you want the option with "less exposed" to be the default :)
Comment 27 Jonathan Druart 2020-07-23 09:17:13 UTC
Created attachment 107224 [details] [review]
Bug 5087: Add server-side check

If a user to access a CSV profile that is not available at the OPAC we
should redirect to 404.

It will also handle undefined csv_profiles

For instance:
kohadev.mydnsname.org:8080/cgi-bin/koha/opac-downloadshelf.pl?shelfnumber=1&format=3
kohadev.mydnsname.org:8080/cgi-bin/koha/opac-downloadcart.pl?bib_list=1/2/&format=3
Comment 28 Jonathan Druart 2020-07-23 09:49:05 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 29 Lucas Gass (lukeg) 2020-07-30 17:16:59 UTC
backported to 20.05.x for 20.05.03
Comment 30 Aleisha Amohia 2020-08-03 23:36:20 UTC
enhancement, not backported to 19.11.x