Bug 38751 - Creating ILL request through OPAC openURL explodes if same attribute defined twice
Summary: Creating ILL request through OPAC openURL explodes if same attribute defined ...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Pedro Amorim
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 38819
  Show dependency treegraph
 
Reported: 2024-12-19 12:03 UTC by Pedro Amorim
Modified: 2025-01-10 18:21 UTC (History)
3 users (show)

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


Attachments
Bug 38751: Add tests (2.20 KB, patch)
2024-12-19 12:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Update Standard ILL backend add_request (1.50 KB, patch)
2024-12-19 12:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Update ILL::Request->extended_attributes (2.85 KB, patch)
2024-12-19 12:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Update ILL::Request->extended_attributes (2.85 KB, patch)
2024-12-19 12:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Add tests (2.20 KB, patch)
2024-12-19 12:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Update Standard ILL backend add_request (1.50 KB, patch)
2024-12-19 12:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Update ILL::Request->extended_attributes (2.89 KB, patch)
2024-12-22 22:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 38751: Add tests (2.24 KB, patch)
2024-12-22 22:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 38751: Update Standard ILL backend add_request (1.54 KB, patch)
2024-12-22 22:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 38751: Update Standard ILL backend migrate (2.05 KB, patch)
2025-01-03 16:07 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38751: Update ILL::Request->extended_attributes (2.97 KB, patch)
2025-01-08 16:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38751: Add tests (2.34 KB, patch)
2025-01-08 16:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38751: Update Standard ILL backend add_request (1.61 KB, patch)
2025-01-08 16:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38751: Update Standard ILL backend migrate (2.51 KB, patch)
2025-01-08 16: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 Pedro Amorim 2024-12-19 12:03:11 UTC

    
Comment 1 Pedro Amorim 2024-12-19 12:04:10 UTC Comment hidden (obsolete)
Comment 2 Pedro Amorim 2024-12-19 12:04:12 UTC Comment hidden (obsolete)
Comment 3 Pedro Amorim 2024-12-19 12:04:14 UTC Comment hidden (obsolete)
Comment 4 Pedro Amorim 2024-12-19 12:04:53 UTC
Created attachment 175703 [details] [review]
Bug 38751: Update ILL::Request->extended_attributes

This updates the extended_attributes method to also act as setter.
Additionally, it'll skip adding any attribute that result in error, preventing an ugly error 500.

Test plan:
1) Enable ILLModule sys pref
2) Attempt to create the following openurl request through the OPAC:
http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?DOI=10.1001%2Fjama.2024.11146&atitle=Interventions%20for%20High%20Body%20Mass%20Index%20in%20Children%20and%20Adolescents%3A%20US%20Preventive%20Services%20Task%20Force%20Recommendation%20Statement.&aulast=Nicholson%2C%20Wanda%20K.&backend=FreeForm&cardnumber=53807898&date=20240716&genre=article&id=DOI%3A10.1001%2Fjama.2024.11146&issn=00987484&issue=3&method=create&opac=1&openurl=1
3) Notice it fails with a Duplicate ID error and the OPAC blows up. This is because Koha is trying to add the 'DOI' illrequestattribute twice and that is resulting in a duplicate ID
4) Apply patches, repeat test plan, notice the error no longer happens, the request is created correctly and all successful attribute have been added.
Comment 5 Pedro Amorim 2024-12-19 12:04:56 UTC
Created attachment 175704 [details] [review]
Bug 38751: Add tests

prove t/db_dependent/Koha/ILL/Request.t
Comment 6 Pedro Amorim 2024-12-19 12:04:58 UTC
Created attachment 175705 [details] [review]
Bug 38751: Update Standard ILL backend add_request

Make use of newly updated ILL::Request->extended_attributes method instead of creating the query manually
Comment 7 David Nind 2024-12-22 22:05:46 UTC
Created attachment 175903 [details] [review]
Bug 38751: Update ILL::Request->extended_attributes

This updates the extended_attributes method to also act as setter.
Additionally, it'll skip adding any attribute that result in error, preventing an ugly error 500.

Test plan:
1) Enable ILLModule sys pref
2) Attempt to create the following openurl request through the OPAC:
http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?DOI=10.1001%2Fjama.2024.11146&atitle=Interventions%20for%20High%20Body%20Mass%20Index%20in%20Children%20and%20Adolescents%3A%20US%20Preventive%20Services%20Task%20Force%20Recommendation%20Statement.&aulast=Nicholson%2C%20Wanda%20K.&backend=FreeForm&cardnumber=53807898&date=20240716&genre=article&id=DOI%3A10.1001%2Fjama.2024.11146&issn=00987484&issue=3&method=create&opac=1&openurl=1
3) Notice it fails with a Duplicate ID error and the OPAC blows up. This is because Koha is trying to add the 'DOI' illrequestattribute twice and that is resulting in a duplicate ID
4) Apply patches, repeat test plan, notice the error no longer happens, the request is created correctly and all successful attribute have been added.

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2024-12-22 22:05:50 UTC
Created attachment 175904 [details] [review]
Bug 38751: Add tests

prove t/db_dependent/Koha/ILL/Request.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2024-12-22 22:05:52 UTC
Created attachment 175905 [details] [review]
Bug 38751: Update Standard ILL backend add_request

Make use of newly updated ILL::Request->extended_attributes method instead of creating the query manually

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Pedro Amorim 2025-01-03 16:07:08 UTC
Created attachment 176124 [details] [review]
Bug 38751: Update Standard ILL backend migrate

Make use of newly updated ILL::Request->extended_attributes method instead of creating the query manually

The test plan in bug 38819 fails without this patch (page explodes with duplicate ID error).
It's the same cause as the one being fixed here in bug 38751 so I'm submitting this here.
Comment 11 Martin Renvoize (ashimema) 2025-01-08 16:13:25 UTC
Created attachment 176250 [details] [review]
Bug 38751: Update ILL::Request->extended_attributes

This updates the extended_attributes method to also act as setter.
Additionally, it'll skip adding any attribute that result in error,
preventing an ugly error 500.

Test plan:
1) Enable ILLModule sys pref
2) Attempt to create the following openurl request through the OPAC:
   http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?DOI=10.1001%2Fjama.2024.11146&atitle=Interventions%20for%20High%20Body%20Mass%20Index%20in%20Children%20and%20Adolescents%3A%20US%20Preventive%20Services%20Task%20Force%20Recommendation%20Statement.&aulast=Nicholson%2C%20Wanda%20K.&backend=FreeForm&cardnumber=53807898&date=20240716&genre=article&id=DOI%3A10.1001%2Fjama.2024.11146&issn=00987484&issue=3&method=create&opac=1&openurl=1
3) Notice it fails with a Duplicate ID error and the OPAC blows up.
   This is because Koha is trying to add the 'DOI' illrequestattribute
   twice and that is resulting in a duplicate ID
4) Apply patches, repeat test plan, notice the error no longer happens,
   the request is created correctly and all successful attribute have
   been added.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2025-01-08 16:13:27 UTC
Created attachment 176251 [details] [review]
Bug 38751: Add tests

prove t/db_dependent/Koha/ILL/Request.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize (ashimema) 2025-01-08 16:13:30 UTC
Created attachment 176252 [details] [review]
Bug 38751: Update Standard ILL backend add_request

Make use of newly updated ILL::Request->extended_attributes method instead of
creating the query manually

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2025-01-08 16:13:32 UTC
Created attachment 176253 [details] [review]
Bug 38751: Update Standard ILL backend migrate

Make use of newly updated ILL::Request->extended_attributes method instead of creating the query manually

The test plan in bug 38819 fails without this patch (page explodes with duplicate ID error).
It's the same cause as the one being fixed here in bug 38751 so I'm submitting this here.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize (ashimema) 2025-01-08 16:14:33 UTC
Clear bugfix and code improvement, well tested and clear.

QA scripts all happy,

Passing QA
Comment 16 Katrin Fischer 2025-01-10 18:21:16 UTC
Pushed for 25.05!

Well done everyone, thank you!