Bug 19882 - Add Novelist Select staff client profile
Summary: Add Novelist Select staff client profile
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 11606
Blocks:
  Show dependency treegraph
 
Reported: 2017-12-27 12:45 UTC by Nick Clemens
Modified: 2018-12-03 20:04 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This patch adds a new Staff Client profile for Novelist information. Previously we used the same value as the opac, this caused malformed links on the staff side. With this patch Novelist features will be disabled on the staff client until a correct profile is obtained from Novelist and entered into the system preference
Version(s) released in:


Attachments
Bug 19882 - Add Novelist Select staff client profile (8.93 KB, patch)
2017-12-27 13:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19882 - Add Novelist Select staff client profile (9.19 KB, patch)
2018-03-12 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19882 - Add Novelist Select staff client profile (8.95 KB, patch)
2018-03-14 15:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19882 - Add Novelist Select staff client profile (9.24 KB, patch)
2018-03-15 10:07 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19882: Add Novelist Select staff client profile (9.32 KB, patch)
2018-04-12 12:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19882: (follow-up) Add 'IGNORE' in DB update (1.32 KB, patch)
2018-04-12 13:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19882: Fix syntax issue in .pref file (1.20 KB, patch)
2018-04-16 14:16 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 Nick Clemens 2017-12-27 12:45:04 UTC
The novelist in the staff client doesn't form links correctly as the profile refers to the OPAC. Novelist has suggested a separate profile for the staff side would allow better styling and correct linking.
Comment 1 Nick Clemens 2017-12-27 13:22:39 UTC Comment hidden (obsolete)
Comment 2 Victor Grousset/tuxayo 2018-03-12 15:54:32 UTC
conflicts:

~/d/koha ❯❯❯ git bz apply 19882
Bug 19882 - Add Novelist Select staff client profile

70171 - Bug 19882 - Add Novelist Select staff client profile

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 19882 - Add Novelist Select staff client profile
Using index info to reconstruct a base tree...
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Auto-merging installer/data/mysql/sysprefs.sql
error: Failed to merge in the changes.
Patch failed at 0001 Bug 19882 - Add Novelist Select staff client profile
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-19882---Add-Novelist-Select-staff-client-profi-lR7BtR.patch
Comment 3 Nick Clemens 2018-03-12 16:08:51 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2018-03-14 14:44:09 UTC
I tested this with my library's Novelist credentials and it's working well. The only problem I see is a fail from the QA tools: [% KOHA_VERSION %] is removed by the patch.
Comment 5 Nick Clemens 2018-03-14 15:29:52 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2018-03-15 10:00:10 UTC
(In reply to Owen Leonard from comment #4)
> The only problem I see is a fail from the QA tools: [% KOHA_VERSION %] is
> removed by the patch.

I think this fail is a bug in the qa tools. We don't use [% KOHA_VERSION %] with third-party assets.
Comment 7 Owen Leonard 2018-03-15 10:07:29 UTC
Created attachment 72918 [details] [review]
Bug 19882 - Add Novelist Select staff client profile

This patchset allows for use of a separate Novelist profile for the
staff client and opac. This allows Novelist to determine links and
special styling for each. Testing is best with novelist credentials,
however, confirming that values are set and used correctly and nothing
is broken should suffice.

To test:
1 - Apply patches
2 - Update database
3 - View detail pages for some records on opac and staff client and ensure no changes have been
    made
4 - Enable NovelistSelectStaffEnabled (and enter or have info in the Profile and password)
5 - Set NovelistSelectStaffView to 'above'
6 - View detail pages for some records, verify that if novelist info is
    available it is displayed and nothing is displayed if not
7 - Repeat with all staff view options

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Katrin Fischer 2018-04-12 12:33:16 UTC
Comment on attachment 72918 [details] [review]
Bug 19882 - Add Novelist Select staff client profile

Review of attachment 72918 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
@@ +1025,5 @@
>              verify_images();
>          });[% END %]
>      </script>
> +    [% IF ( Koha.Preference('NovelistSelectStaffEnabled') && Koha.Preference('NovelistSelectStaffProfile') && ( normalized_isbn || normalized_upc ) ) %]
> +        <script type="text/javascript" src="https://imageserver.ebscohost.com/novelistselect/ns2init.js"></script>

External Javascript, so this is ok.

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   FAIL	  forbidden patterns
		forbidden pattern: Include [% KOHA_VERSION %] in js paths (bug 12904) - May be false positive (line 1049)
Comment 9 Katrin Fischer 2018-04-12 12:36:18 UTC
Created attachment 74084 [details] [review]
Bug 19882: Add Novelist Select staff client profile

This patchset allows for use of a separate Novelist profile for the
staff client and opac. This allows Novelist to determine links and
special styling for each. Testing is best with novelist credentials,
however, confirming that values are set and used correctly and nothing
is broken should suffice.

To test:
1 - Apply patches
2 - Update database
3 - View detail pages for some records on opac and staff client and ensure no changes have been
made
4 - Enable NovelistSelectStaffEnabled (and enter or have info in the Profile and password)
5 - Set NovelistSelectStaffView to 'above'
6 - View detail pages for some records, verify that if novelist info is
available it is displayed and nothing is displayed if not
7 - Repeat with all staff view options

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Jonathan Druart 2018-04-12 13:23:08 UTC
1. IGNORE is missing (I can do it when pushing, but...)

2. We should set NovelistSelectStaffProfile=NovelistSelectProfile, right?
Comment 11 Nick Clemens 2018-04-12 13:52:20 UTC
Created attachment 74103 [details] [review]
Bug 19882: (follow-up) Add 'IGNORE' in DB update
Comment 12 Nick Clemens 2018-04-12 13:53:34 UTC
1 - Added :-)
2 - That is the problem right now - using the regular profile makes all links point over to the opac and so they don't work properly. I can add to the release notes as I think disabling it until they add a profile is better
Comment 13 Jonathan Druart 2018-04-13 16:41:48 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 14 Jonathan Druart 2018-04-16 14:16:50 UTC
Created attachment 74239 [details] [review]
Bug 19882: Fix syntax issue in .pref file

The translate script raised an error:
Use of uninitialized value $pref_name in concatenation (.) or string at
LangInstaller.pm line 190.
Comment 15 Jonathan Druart 2018-04-16 14:18:38 UTC
Last patch pushed to master.
Comment 16 Nick Clemens 2018-05-21 17:38:13 UTC
Enhancement, not backported for 17.11.x