Bug 36047 - Apostrophe in suggestion status reason blocks order receipt
Summary: Apostrophe in suggestion status reason blocks order receipt
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Pedro Amorim
QA Contact: Julian Maurice
URL:
Keywords:
Depends on: 8179
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-08 13:31 UTC by Mirjam Vantieghem
Modified: 2024-03-19 19:33 UTC (History)
8 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,23.11.04,23.05.10


Attachments
Bug 36047: Workaround the apostrophe (1.41 KB, patch)
2024-02-16 16:59 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36047: Add apos and quot to escapeHtml core function (1008 bytes, patch)
2024-02-19 17:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36047: filter jQuery selector through escapeHtml (1.80 KB, patch)
2024-02-19 17:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36047: filter jQuery selector through escapeHtml (1.85 KB, patch)
2024-02-19 21:39 UTC, Phan Tung Bui
Details | Diff | Splinter Review
Bug 36047: Remove jQuery selector from conditional (3.60 KB, patch)
2024-02-22 15:46 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36047: Remove jQuery selector from conditional (3.69 KB, patch)
2024-02-23 07:46 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mirjam Vantieghem 2024-02-08 13:31:24 UTC
Since the new way of receiving orders in 23.11, the receiving of an order is impossible when this order was created from a purchase suggestion and the reason for accepting the suggestion contains an apostrophe. 

I didn't test all possible characters. Exclamation and question marks, comma's and semicolons all seem fine.

Temporary workaround is to edit the suggestion were the order came from and remove the apostrophe. However, by editing the suggestion, the patron who made the suggestion will get another automatic notice that the title has been ordered and the library will get another email that a suggestion was made, so not ideal. And it slows down the acquisition process.
Comment 1 Caroline Cyr La Rose 2024-02-16 14:56:29 UTC
I have had several complaints about this from clients since we upgraded them.

Here is the test plan to recreate the problem:
1. Create a suggestion
   1.1. Go to Acquisitions > Suggestions
   1.2. Click New purchase suggestion
   1.3. Enter a title
   1.4. Click Submit your suggestion

2. Update the suggestion to accepted and add a reason with an apostrophe
   2.1. In the suggestions list, check the box next to the suggestion
   2.2. At the bottom of the screen, choose Mark selected as: Accepted
   2.3. In With this reason, choose Others...
   2.4. Enter a reason with an apostrophe (my case was in French : Disponible en impression à la demande (POD). S'attendre à un long temps d'attente)
   2.5. Click Submit

3. Order the suggestion
   3.1. Go to Acquisitions
   3.2. Search for a vendor
   3.3. Click New > Basket
   3.4. Enter a basket name
   (Optional: for testing I like to change Create items when: cataloging the record)
   3.5. Click Save
   3.6. Click Add to basket
   3.7. Click From a suggestion
   3.8. Click Order next to the suggestion
   3.9. Enter order details
   3.10. Click Save
   3.11. Click Close basket
   3.12. Click Yes, close

4. Receive the order
   4.1. Click Receive shipments
   4.2. Enter a Vendor invoice number
   4.3. Click Next
   4.4. Click Receive next to the order
        --> Page stays In processing forever


I upped the severity to major as this causes a functionality to not work at all (acquisitions receive), but there is a workaround (remove the apostrophe), so not critical.
Comment 2 Pedro Amorim 2024-02-16 16:59:38 UTC
Created attachment 162243 [details] [review]
Bug 36047: Workaround the apostrophe

This is just a quick hack fix highlighting where the issue lies

the real fix should be escape HTML entities coming from biblio.suggestions
as this fix may also fail in case the data contains double quotes
Comment 3 Pedro Amorim 2024-02-16 17:01:07 UTC
I may come back to this but I'm out of time for now so just wanted to submit the result of my dig through.
Comment 4 Pedro Amorim 2024-02-19 17:09:20 UTC
Created attachment 162280 [details] [review]
Bug 36047: Add apos and quot to escapeHtml core function
Comment 5 Pedro Amorim 2024-02-19 17:09:23 UTC
Created attachment 162281 [details] [review]
Bug 36047: filter jQuery selector through escapeHtml
Comment 6 Phan Tung Bui 2024-02-19 21:39:25 UTC
Created attachment 162285 [details] [review]
Bug 36047: filter jQuery selector through escapeHtml

Signed-off-by: Phan Tung Bui <phan-tung.bui@inlibro.com>
Comment 7 Julian Maurice 2024-02-22 15:16:48 UTC
With the patch applied, if the reason selected is one from the SUGGEST authorised value category, and this reason has an apostrophe, on the receive modal the reason is displayed inside a text input, instead of the select.
I think it's due to the fact that the selector `option[value="l&apos;apostrophe"]` will not match <option value="l'apostrophe">. A properly escaped selector would be `option[value="l\'apostrophe"]`

We should do something like this instead:

-    if($("#reason option[value='"+row.biblio.suggestions[0].reason+"']").length) {
+    const options = Array.from(document.querySelectorAll('#reason option'))
+    if (options.some(option => option.value === row.biblio.suggestions[0].reason)) {

No need to escape anything
Comment 8 Pedro Amorim 2024-02-22 15:46:44 UTC
Created attachment 162337 [details] [review]
Bug 36047: Remove jQuery selector from conditional

This prevents the jQuery from breaking if the reason contains an apostrophe character.

1. Create a suggestion
   1.1. Go to Acquisitions > Suggestions
   1.2. Click New purchase suggestion
   1.3. Enter a title
   1.4. Click Submit your suggestion

2. Update the suggestion to accepted and add a reason with an apostrophe
   2.1. In the suggestions list, check the box next to the suggestion
   2.2. At the bottom of the screen, choose Mark selected as: Accepted
   2.3. In With this reason, choose Others...
   2.4. Enter a reason with an apostrophe (my case was in French : Disponible en impression à la demande (POD). S'attendre à un long temps d'attente)
   2.5. Click Submit

3. Order the suggestion
   3.1. Go to Acquisitions
   3.2. Search for a vendor
   3.3. Click New > Basket
   3.4. Enter a basket name
   (Optional: for testing I like to change Create items when: cataloging the record)
   3.5. Click Save
   3.6. Click Add to basket
   3.7. Click From a suggestion
   3.8. Click Order next to the suggestion
   3.9. Enter order details
   3.10. Click Save
   3.11. Click Close basket
   3.12. Click Yes, close

4. Receive the order
   4.1. Click Receive shipments
   4.2. Enter a Vendor invoice number
   4.3. Click Next
   4.4. Click Receive next to the order
        --> Page stays In processing forever

Apply patch. Repeat. Notice the page now renders correctly.
Extra: Add an entry containg an apostrophe to the SUGGEST authorized values category and repeat test plan. Confirm everything works as expected.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 9 Pedro Amorim 2024-02-22 15:48:22 UTC
(In reply to Julian Maurice from comment #7)
> With the patch applied, if the reason selected is one from the SUGGEST
> authorised value category, and this reason has an apostrophe, on the receive
> modal the reason is displayed inside a text input, instead of the select.
> I think it's due to the fact that the selector
> `option[value="l&apos;apostrophe"]` will not match <option
> value="l'apostrophe">. A properly escaped selector would be
> `option[value="l\'apostrophe"]`
> 
> We should do something like this instead:
> 
> -    if($("#reason
> option[value='"+row.biblio.suggestions[0].reason+"']").length) {
> +    const options = Array.from(document.querySelectorAll('#reason option'))
> +    if (options.some(option => option.value ===
> row.biblio.suggestions[0].reason)) {
> 
> No need to escape anything

Thank you Julian, my solution fixed the reported issue but broke functionality as you've described.
I was able to reproduce the issue my patch introduced, tested your suggestion and confirmed it fixes it.

I've submitted the patch authored by you with my SO.
Comment 10 Julian Maurice 2024-02-23 07:46:50 UTC
Created attachment 162361 [details] [review]
Bug 36047: Remove jQuery selector from conditional

This prevents the jQuery from breaking if the reason contains an
apostrophe character.

1. Create a suggestion
   1.1. Go to Acquisitions > Suggestions
   1.2. Click New purchase suggestion
   1.3. Enter a title
   1.4. Click Submit your suggestion

2. Update the suggestion to accepted and add a reason with an apostrophe
   2.1. In the suggestions list, check the box next to the suggestion
   2.2. At the bottom of the screen, choose Mark selected as: Accepted
   2.3. In With this reason, choose Others...
   2.4. Enter a reason with an apostrophe (my case was in French :
        Disponible en impression à la demande (POD). S'attendre à un
        long temps d'attente)
   2.5. Click Submit

3. Order the suggestion
   3.1. Go to Acquisitions
   3.2. Search for a vendor
   3.3. Click New > Basket
   3.4. Enter a basket name
        (Optional: for testing I like to change Create items when:
        cataloging the record)
   3.5. Click Save
   3.6. Click Add to basket
   3.7. Click From a suggestion
   3.8. Click Order next to the suggestion
   3.9. Enter order details
   3.10. Click Save
   3.11. Click Close basket
   3.12. Click Yes, close

4. Receive the order
   4.1. Click Receive shipments
   4.2. Enter a Vendor invoice number
   4.3. Click Next
   4.4. Click Receive next to the order
        --> Page stays In processing forever

Apply patch. Repeat. Notice the page now renders correctly.
Extra: Add an entry containg an apostrophe to the SUGGEST authorized
values category and repeat test plan. Confirm everything works as
expected.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 11 Katrin Fischer 2024-03-07 15:38:03 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 12 Fridolin Somers 2024-03-11 09:59:07 UTC
Pushed to 23.11.x for 23.11.04
Comment 13 Lucas Gass 2024-03-19 19:33:48 UTC
Backported to 23.05.x for upcoming 23.05.10