Bug 25318 - Convert authorities_js.inc to JavaScript file with translatable strings
Summary: Convert authorities_js.inc to JavaScript file with translatable strings
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Owen Leonard
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 39874 21156
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-29 18:44 UTC by Owen Leonard
Modified: 2025-05-15 07:21 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 25318: Convert authorities_js.inc to JavaScript file (13.55 KB, patch)
2025-02-21 19:10 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 25318: Convert authorities_js.inc to JavaScript file (13.59 KB, patch)
2025-03-14 03:53 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 25318: (follow-up) Make searchType be defined in merge.tt (1.41 KB, patch)
2025-03-14 03:54 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 25318: Convert authorities_js.inc to JavaScript file (13.64 KB, patch)
2025-05-12 13:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 25318: (follow-up) Make searchType be defined in merge.tt (1.46 KB, patch)
2025-05-12 13:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 25318: Convert authorities_js.inc to JavaScript file (13.72 KB, patch)
2025-05-12 14:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 25318: (follow-up) Make searchType be defined in merge.tt (1.53 KB, patch)
2025-05-12 14:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2020-04-29 18:44:53 UTC
Now that we can translate strings in JavaScript we can convert authorities_js.inc into a JavaScript file with language strings updated with the new JS translation function.
Comment 1 Owen Leonard 2025-02-21 19:10:41 UTC
Created attachment 178537 [details] [review]
Bug 25318: Convert authorities_js.inc to JavaScript file

This patch creates a new JavaScript file, authorities.js, and includes
it on pages which previously used authorities_js.inc. Some minor changes
to the JavaScript have been made in order to pass JS variables instead
of template variables.

To test, apply the patch and go to Authorities.

- Perform a search for authorities and test various features on the
  search result page::
  - New from Z39.50/SRU
  - Actions -> Merge
  - Actions -> Delete
  - Action -> MARC preview
- View the detail page for an authority record and test:
  - New from Z39.50/SRU
  - Edit -> Replace record via Z39.50/SRU
  - Edit -> Delete record
- Go to Cataloging and create a new record.
  - Go to tab 1 and click the plugin trigger link on 100a.
  - Confirm that the authority search popup appears and works to add a
    selected author.

Sponsored-by: Athens County Public Libraries
Comment 2 Phil Ringnalda 2025-02-27 22:28:07 UTC
Merging throws "Uncaught ReferenceError: searchType is not defined." Since it's exactly what we were doing before (with the extra step of getting "" from [% marclist | html %] without a marclist), sticking a 'const searchType = "";' in merge.tt seems like the poetic fix.
Comment 6 Phil Ringnalda 2025-03-14 03:53:07 UTC
Created attachment 179299 [details] [review]
Bug 25318: Convert authorities_js.inc to JavaScript file

This patch creates a new JavaScript file, authorities.js, and includes
it on pages which previously used authorities_js.inc. Some minor changes
to the JavaScript have been made in order to pass JS variables instead
of template variables.

To test, apply the patch and go to Authorities.

- Perform a search for authorities and test various features on the
  search result page::
  - New from Z39.50/SRU
  - Actions -> Merge
  - Actions -> Delete
  - Action -> MARC preview
- View the detail page for an authority record and test:
  - New from Z39.50/SRU
  - Edit -> Replace record via Z39.50/SRU
  - Edit -> Delete record
- Go to Cataloging and create a new record.
  - Go to tab 1 and click the plugin trigger link on 100a.
  - Confirm that the authority search popup appears and works to add a
    selected author.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 7 Phil Ringnalda 2025-03-14 03:54:47 UTC
Created attachment 179300 [details] [review]
Bug 25318: (follow-up) Make searchType be defined in merge.tt

With authorities_js.inc doing "var searchType = '[% marclist | html %]';" in
merge.tt where marclist isn't defined we got "var searchType = '';". This
patch just adds 'const searchType = ""' directly to merge.tt to avoid a
JavaScript console error.

Test plan:
1. Open the devtools console
2. Do an authority search that returns at least two authorities
3. Select one, then another for merge
4. When the merge.pl page loads, observe an "Uncaught ReferenceError" in
   the console
5. Apply followup, repeat steps 1-3, without an "Uncaught ReferenceError"
Comment 8 David Nind 2025-05-11 23:39:09 UTC
For the life of me, I can't figure out how to merge two authority records, I'm sure I've done it before!

Here is what I did:
1. Go to Authorities.
2. Click the search button (arrow) in the staff interface header.
   Result: search results are displayed
3. For a record select Action > Merge.
   Result: Message is displayed in blue under the authority search results heading:
           Merging with authority: Ślez, Ts. (Tsevi), b. 1833. () Cancel merge
3. For another authority record of the same type (in this case Personal Names), go to Actions > Merge
4. I get a message saying: "You cannot merge a record with itself. Please select two different authorities."

I'm sure if merging was broken, someone would have noticed by now 8-)

Or I'm trying to merge incorrectly!

It may be moot anyway, as I think Phil has signed the bug off - the status needs updating if that is the case.
Comment 9 Owen Leonard 2025-05-12 11:36:57 UTC
(In reply to David Nind from comment #8)
> For the life of me, I can't figure out how to merge two authority records,
> I'm sure I've done it before!

You have discovered Bug 39874
Comment 10 David Nind 2025-05-12 13:57:40 UTC
Created attachment 182279 [details] [review]
Bug 25318: Convert authorities_js.inc to JavaScript file

This patch creates a new JavaScript file, authorities.js, and includes
it on pages which previously used authorities_js.inc. Some minor changes
to the JavaScript have been made in order to pass JS variables instead
of template variables.

To test, apply the patch and go to Authorities.

- Perform a search for authorities and test various features on the
  search result page::
  - New from Z39.50/SRU
  - Actions -> Merge
  - Actions -> Delete
  - Action -> MARC preview
- View the detail page for an authority record and test:
  - New from Z39.50/SRU
  - Edit -> Replace record via Z39.50/SRU
  - Edit -> Delete record
- Go to Cataloging and create a new record.
  - Go to tab 1 and click the plugin trigger link on 100a.
  - Confirm that the authority search popup appears and works to add a
    selected author.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2025-05-12 13:57:43 UTC
Created attachment 182280 [details] [review]
Bug 25318: (follow-up) Make searchType be defined in merge.tt

With authorities_js.inc doing "var searchType = '[% marclist | html %]';" in
merge.tt where marclist isn't defined we got "var searchType = '';". This
patch just adds 'const searchType = ""' directly to merge.tt to avoid a
JavaScript console error.

Test plan:
1. Open the devtools console
2. Do an authority search that returns at least two authorities
3. Select one, then another for merge
4. When the merge.pl page loads, observe an "Uncaught ReferenceError" in
   the console
5. Apply followup, repeat steps 1-3, without an "Uncaught ReferenceError"

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2025-05-12 13:58:40 UTC
(In reply to Owen Leonard from comment #9)
> (In reply to David Nind from comment #8)
> > For the life of me, I can't figure out how to merge two authority records,
> > I'm sure I've done it before!
> 
> You have discovered Bug 39874

8-)
Comment 13 Martin Renvoize (ashimema) 2025-05-12 14:13:30 UTC
Created attachment 182284 [details] [review]
Bug 25318: Convert authorities_js.inc to JavaScript file

This patch creates a new JavaScript file, authorities.js, and includes
it on pages which previously used authorities_js.inc. Some minor changes
to the JavaScript have been made in order to pass JS variables instead
of template variables.

To test, apply the patch and go to Authorities.

- Perform a search for authorities and test various features on the
  search result page::
  - New from Z39.50/SRU
  - Actions -> Merge
  - Actions -> Delete
  - Action -> MARC preview
- View the detail page for an authority record and test:
  - New from Z39.50/SRU
  - Edit -> Replace record via Z39.50/SRU
  - Edit -> Delete record
- Go to Cataloging and create a new record.
  - Go to tab 1 and click the plugin trigger link on 100a.
  - Confirm that the authority search popup appears and works to add a
    selected author.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 14 Martin Renvoize (ashimema) 2025-05-12 14:13:33 UTC
Created attachment 182285 [details] [review]
Bug 25318: (follow-up) Make searchType be defined in merge.tt

With authorities_js.inc doing "var searchType = '[% marclist | html %]';" in
merge.tt where marclist isn't defined we got "var searchType = '';". This
patch just adds 'const searchType = ""' directly to merge.tt to avoid a
JavaScript console error.

Test plan:
1. Open the devtools console
2. Do an authority search that returns at least two authorities
3. Select one, then another for merge
4. When the merge.pl page loads, observe an "Uncaught ReferenceError" in
   the console
5. Apply followup, repeat steps 1-3, without an "Uncaught ReferenceError"

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 15 Katrin Fischer 2025-05-15 07:21:30 UTC
Pushed for 25.05!

Well done everyone, thank you!