Bug 30709 - 'Insert' button in notices editor not adding selected placeholders to notice
Summary: 'Insert' button in notices editor not adding selected placeholders to notice
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Martin Renvoize
URL:
Keywords: regression
Depends on: 29155
Blocks:
  Show dependency treegraph
 
Reported: 2022-05-06 09:21 UTC by Katrin Fischer
Modified: 2023-12-28 20:42 UTC (History)
2 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:
22.05.00


Attachments
Bug 30709: 'Insert' button in notices editor not adding selected placeholders to notice (1.54 KB, patch)
2022-05-06 12:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30709: 'Insert' button in notices editor not adding selected placeholders to notice (1.60 KB, patch)
2022-05-06 12:25 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-05-06 09:21:16 UTC
To test:
- Edit any of your existing notices
- Try to add any of the placeholders from the list shown on the left to your notice's content using the 'insert' button
--> It doesn't work.
Comment 1 Martin Renvoize 2022-05-06 09:36:35 UTC
Pretty sure this comes from bug 30545.

What I've discovered so far is that we add `content_` to the textarea id and appear to drop the 'name' for the input.. but it needs a bit more digging.. 

Any chance you can take a look Owen?
Comment 2 Owen Leonard 2022-05-06 12:01:10 UTC
Bug 29155 is to blame.
Comment 3 Owen Leonard 2022-05-06 12:08:13 UTC
Created attachment 134727 [details] [review]
Bug 30709: 'Insert' button in notices editor not adding selected placeholders to notice

This patch updates letter.js so that it uses .prop("selected") to look
for selected <option>s instead of .attr("selected"). This is necessary
because of the jQuery upgrade.

To test, apply the patch and edit any notice. Test that you can select
one or more database columns and insert them into the body of a notice
by clicking "Insert." Confirm that your selections are inserted where
you left the cursor in the message textarea.

Test with multiple message transports and with multiple languages
installed (with TranslateNotices enabled).
Comment 4 Martin Renvoize 2022-05-06 12:25:24 UTC
Created attachment 134728 [details] [review]
Bug 30709: 'Insert' button in notices editor not adding selected placeholders to notice

This patch updates letter.js so that it uses .prop("selected") to look
for selected <option>s instead of .attr("selected"). This is necessary
because of the jQuery upgrade.

To test, apply the patch and edit any notice. Test that you can select
one or more database columns and insert them into the body of a notice
by clicking "Insert." Confirm that your selections are inserted where
you left the cursor in the message textarea.

Test with multiple message transports and with multiple languages
installed (with TranslateNotices enabled).

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2022-05-06 12:25:51 UTC
Trivial fix, thanks Owen.

Straight PQA as it's all working now :)
Comment 6 Fridolin Somers 2022-05-06 20:35:31 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄