Bug 36418 - Set response's content-type to application/json when needed - svc scripts
Summary: Set response's content-type to application/json when needed - svc scripts
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on: 36351
Blocks: 36060
  Show dependency treegraph
 
Reported: 2024-03-25 16:27 UTC by Lucas Gass
Modified: 2024-04-17 15:15 UTC (History)
6 users (show)

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


Attachments
Bug 36418: Explicitly set response to json (1.69 KB, patch)
2024-03-25 16:35 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 36418: Set header response content type to application/json - svc (5.27 KB, patch)
2024-03-26 07:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36418: Explicitly set response to json (1.74 KB, patch)
2024-03-26 13:36 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 36418: Set header response content type to application/json - svc (5.30 KB, patch)
2024-03-26 13:36 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 36418: Catch more cases where header respose should be application/json (1.29 KB, patch)
2024-03-28 15:15 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 36418: Explicitly set response to json (1.93 KB, patch)
2024-03-28 17:58 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36418: Set header response content type to application/json - svc (5.30 KB, patch)
2024-03-28 17:58 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36418: Catch more cases where header response should be application/json (1.34 KB, patch)
2024-03-28 17:58 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36418: opac/svc/club: switch content-type to application/json (1.31 KB, patch)
2024-03-28 17:58 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36418: Explicitly set response to json (2.00 KB, patch)
2024-03-29 07:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36418: Set header response content type to application/json - svc (5.39 KB, patch)
2024-03-29 07:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36418: Catch more cases where header response should be application/json (1.42 KB, patch)
2024-03-29 07:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36418: opac/svc/club: switch content-type to application/json (1.39 KB, patch)
2024-03-29 07:28 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 Lucas Gass 2024-03-25 16:27:54 UTC
To recreate:

1. Add some checkouts and load the issues-table 
2. Select 'Check in' checkbox for that item and click "Check in selected items"
3. The row becomes read and reads "Unable to check in"
4. Reload the page, the item has actually checked in 

5. Try the 'Renew' checkbox and click "Renew selected items"
6. It just spins and spins.
7. Reload the page, the item has actually renewed. 

Seems related to Bug 36351? 

svc/renew and svc/checkin stopped assuming the response is JSON.
Comment 1 Lucas Gass 2024-03-25 16:35:18 UTC
Created attachment 163837 [details] [review]
Bug 36418: Explicitly set response to json

To test:
1. Add some checkouts and load the issues-table
2. Select 'Check in' checkbox for that item and click "Check in selected items"
3. The row becomes read and reads "Unable to check in"
4. Reload the page, the item has actually checked in

5. Try the 'Renew' checkbox and click "Renew selected items"
6. It just spins and spins.
7. Reload the page, the item has actually renewed.
8. APPLY PATCH and restart_all
9. Try the steps again and this time everything should work properly.
Comment 2 Lucas Gass 2024-03-25 16:36:30 UTC
Is this the correct fix and if so, do all the svc/* files need it?
Comment 3 Jonathan Druart 2024-03-26 07:42:47 UTC
Created attachment 163885 [details] [review]
Bug 36418: Set header response content type to application/json - svc

We need to set the content type to application/json for the svc scripts
returning json
Comment 4 Jonathan Druart 2024-03-26 07:43:37 UTC
(In reply to Lucas Gass from comment #2)
> Is this the correct fix and if so, do all the svc/* files need it?

Yes, I have also found other scripts that need the change. Thanks for catching that!
Comment 5 Brendan Lawlor 2024-03-26 13:36:50 UTC
Created attachment 163922 [details] [review]
Bug 36418: Explicitly set response to json

To test:
1. Add some checkouts and load the issues-table
2. Select 'Check in' checkbox for that item and click "Check in selected items"
3. The row becomes read and reads "Unable to check in"
4. Reload the page, the item has actually checked in

5. Try the 'Renew' checkbox and click "Renew selected items"
6. It just spins and spins.
7. Reload the page, the item has actually renewed.
8. APPLY PATCH and restart_all
9. Try the steps again and this time everything should work properly.

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 6 Brendan Lawlor 2024-03-26 13:36:52 UTC
Created attachment 163923 [details] [review]
Bug 36418: Set header response content type to application/json - svc

We need to set the content type to application/json for the svc scripts
returning json

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 7 Victor Grousset/tuxayo 2024-03-27 02:39:29 UTC
>> Is this the correct fix and if so, do all the svc/* files need it?
> Yes

Maybe these two then?
svc/recall
svc/mana/increment

What about the OPAC?
opac/svc/club/enroll
opac/svc/club/cancel_enrollment
Comment 8 Jonathan Druart 2024-03-27 07:39:37 UTC
(In reply to Victor Grousset/tuxayo from comment #7)
> >> Is this the correct fix and if so, do all the svc/* files need it?
> > Yes
> 
> Maybe these two then?
> svc/recall

I missed this one it seems.

> svc/mana/increment

I don't know, it's not obvious what is the return. Please investigate (test) and you will see if it returns JSON or something else.

> What about the OPAC?
> opac/svc/club/enroll
> opac/svc/club/cancel_enrollment

If they output JSON, yes they should.
Comment 9 Victor Grousset/tuxayo 2024-03-28 14:14:01 UTC
>> svc/recall

>I missed this one it seems.

This one indeed outputs JSON

>> svc/mana/increment

> I don't know, it's not obvious what is the return.
> Please investigate (test) and you will see if it returns JSON or something else.

It seems the return is nothing. From the code, and from testing.


>> opac/svc/club/enroll
>> opac/svc/club/cancel_enrollment

>If they output JSON, yes they should.

enroll: after testing, yes it does

cancel_enrollment: last two lines
> print $cgi->header( -type => 'text/plain', -charset => 'UTF-8' );
> print to_json( { success => $enrollment ? 1 : 0 } );

lol, yes also
A `type => 'text/plain'` just above a `print to_json`

So work needed on 3 of the 4 suspected files.
Comment 10 Victor Grousset/tuxayo 2024-03-28 14:16:31 UTC
> A `type => 'text/plain'` just above a `print to_json`

That's not the only case >_<

randomly opened serials/subscription-frequency.pl and there is:

> print $input->header(-type => 'text/plain', -charset => 'UTF-8');
> print to_json( $frequencyrecord );


Is that still relevant for this ticket? Leaning towards no.
Comment 11 Jonathan Druart 2024-03-28 14:20:00 UTC
Can you provide a follow-up maybe?
Comment 12 Lucas Gass 2024-03-28 15:15:22 UTC
Created attachment 164066 [details] [review]
Bug 36418: Catch  more cases where header respose should be application/json
Comment 13 Victor Grousset/tuxayo 2024-03-28 17:58:26 UTC
Created attachment 164094 [details] [review]
Bug 36418: Explicitly set response to json

To test:
1. Add some checkouts and load the issues-table
2. Select 'Check in' checkbox for that item and click "Check in selected items"
3. The row becomes read and reads "Unable to check in"
4. Reload the page, the item has actually checked in

5. Try the 'Renew' checkbox and click "Renew selected items"
6. It just spins and spins.
7. Reload the page, the item has actually renewed.
8. APPLY PATCH and restart_all
9. Try the steps again and this time everything should work properly.
10. tests these URLs
http://localhost:8081/cgi-bin/koha/svc/recall
http://localhost:8080/cgi-bin/koha/svc/club/enroll
http://localhost:8081/cgi-bin/koha/svc/mana/increment : empty response

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 14 Victor Grousset/tuxayo 2024-03-28 17:58:28 UTC
Created attachment 164095 [details] [review]
Bug 36418: Set header response content type to application/json - svc

We need to set the content type to application/json for the svc scripts
returning json

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 15 Victor Grousset/tuxayo 2024-03-28 17:58:31 UTC
Created attachment 164096 [details] [review]
Bug 36418: Catch more cases where header response should be application/json

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 16 Victor Grousset/tuxayo 2024-03-28 17:58:33 UTC
Created attachment 164097 [details] [review]
Bug 36418: opac/svc/club: switch content-type to application/json
Comment 17 Victor Grousset/tuxayo 2024-03-28 18:05:44 UTC
Tested & SOed Lucas' follow-up, and added another follow-up for the OPAC cases.
Amended test plan to add the URLs used to test Lucas' follow-up and mine partially.
Comment 18 Jonathan Druart 2024-03-29 07:28:30 UTC
Created attachment 164102 [details] [review]
Bug 36418: Explicitly set response to json

To test:
1. Add some checkouts and load the issues-table
2. Select 'Check in' checkbox for that item and click "Check in selected items"
3. The row becomes read and reads "Unable to check in"
4. Reload the page, the item has actually checked in

5. Try the 'Renew' checkbox and click "Renew selected items"
6. It just spins and spins.
7. Reload the page, the item has actually renewed.
8. APPLY PATCH and restart_all
9. Try the steps again and this time everything should work properly.
10. tests these URLs
http://localhost:8081/cgi-bin/koha/svc/recall
http://localhost:8080/cgi-bin/koha/svc/club/enroll
http://localhost:8081/cgi-bin/koha/svc/mana/increment : empty response

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2024-03-29 07:28:33 UTC
Created attachment 164103 [details] [review]
Bug 36418: Set header response content type to application/json - svc

We need to set the content type to application/json for the svc scripts
returning json

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Jonathan Druart 2024-03-29 07:28:36 UTC
Created attachment 164104 [details] [review]
Bug 36418: Catch more cases where header response should be application/json

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2024-03-29 07:28:39 UTC
Created attachment 164105 [details] [review]
Bug 36418: opac/svc/club: switch content-type to application/json

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Katrin Fischer 2024-04-02 16:01:01 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 23 Fridolin Somers 2024-04-17 15:15:23 UTC
Depends on bug 36351 not in 23.11.x