Description
Melissa Lefebvre
2013-12-20 18:47:19 UTC
I think we can do this in a fairly elegant way: 1) Identify all triggers of existing sounds 2) Strip out existing html for playing sounds 3) Make those existing sound-triggering dialogs with a class "audio-success", "audio-error" 4) Add a new "warning" sound 5) Add the class "audio-alert" to those dialogs that should trigger the new audio sound 6) Modify the header include to trigger the respective sound if an element with one of these classes is present on the page via jquery ( if soundon is enabled of course ) 7) Rename soundon to something more sensible ( eg UseSounds, UseAudioAlerts, etc ) 8) Add three new system preferences to define urls to override these sounds with. If left empty, the default sounds will be used In your idea, will sounds be activable independently ? For example, I would like to have a sound at check-out, but NOT at check-in. Or I would like to have a sound at check-in only if there's a hold. If everything is OK, I don't want any sound. Will it be possible ? I have just been asked to make only the "Not Checked Out" sound different from the other check in message sounds. Other libraries may want a different sound for "Item was lost, now found". Will your solution accommodate that? Created attachment 29030 [details] [review] Bug 11431 [1] - Replace existing system with no change in functionality This patch set adds the ability to select which sounds are used for Koha's audio alerts, and which elements trigger those audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Set the various OverrideAudioAlert preferences to different sounds 6) Retest, verify new sounds play 7) Add/remove selectors from the AudioAlertSelector system preferences 8) Observe the difference in audio alert behavior based on your selections Created attachment 29031 [details] [review] Bug 11431 [2] - Rename soundon to AudioAlerts, move to it's own preference section Created attachment 29032 [details] [review] Bug 11431 [3] - Add ability to customize the warning, action, and success sounds Created attachment 29033 [details] [review] Bug 11431 [4] - Add the ability to choose the elements that trigger an audio alert Created attachment 29034 [details] [review] Bug 11431 [5] - Update sysprefs.sql Created attachment 29035 [details] [review] Bug 11431 [1] - Replace existing system with no change in functionality This patch set adds the ability to select which sounds are used for Koha's audio alerts, and which elements trigger those audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Set the various OverrideAudioAlert preferences to different sounds 6) Retest, verify new sounds play 7) Add/remove selectors from the AudioAlertSelector system preferences 8) Observe the difference in audio alert behavior based on your selections Created attachment 29036 [details] [review] Bug 11431 [2] - Rename soundon to AudioAlerts, move to it's own preference section Created attachment 29037 [details] [review] Bug 11431 [3] - Add ability to customize the warning, action, and success sounds Created attachment 29038 [details] [review] Bug 11431 [4] - Add the ability to choose the elements that trigger an audio alert Created attachment 29039 [details] [review] Bug 11431 [5] - Update sysprefs.sql Created attachment 29119 [details] [review] Bug 11431 [6] - Add the ability to specify arbitarary audio alerts This patch further enhances our control over Koha audio alerts by allowing a library to specify custom audio events. A new system preference is added the combines the functions of the Selector and Override systems preferences in a list of selector:sound combinations. These definitions override the default sounds and thus gives the library a way to completely redefine what events trigger and audio alert, and what sounds are played! Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Edit the new AudioAlertsCustom system preference and enter the following: "body:contains('Check in message')": "IM_notification.ogg" 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Created attachment 29120 [details] [review] Bug 11431 [6] - Add the ability to specify arbitrary audio alerts This patch further enhances our control over Koha audio alerts by allowing a library to specify custom audio events. A new system preference is added the combines the functions of the Selector and Override systems preferences in a list of selector:sound combinations. These definitions override the default sounds and thus gives the library a way to completely redefine what events trigger and audio alert, and what sounds are played! Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Edit the new AudioAlertsCustom system preference and enter the following: "body:contains('Check in message')": "IM_notification.ogg" 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! (In reply to Paul Poulain from comment #3) > In your idea, will sounds be activable independently ? For example, I would > like to have a sound at check-out, but NOT at check-in. Or I would like to > have a sound at check-in only if there's a hold. If everything is OK, I > don't want any sound. > Will it be possible ? Yes! The last patch in this series give you full control of what events trigger an audio alert! (In reply to Peggy Thrasher from comment #4) > I have just been asked to make only the "Not Checked Out" sound different > from the other check in message sounds. Other libraries may want a > different sound for "Item was lost, now found". Will your solution > accommodate that? Yep! See the comment above this one! Kyle Created attachment 29404 [details] [review] Bug 11431 [6] - Add the ability to specify arbitrary audio alerts This patch further enhances our control over Koha audio alerts by allowing a library to specify custom audio events. A new system preference is added the combines the functions of the Selector and Override systems preferences in a list of selector:sound combinations. These definitions override the default sounds and thus gives the library a way to completely redefine what events trigger and audio alert, and what sounds are played! Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Edit the new AudioAlertsCustom system preference and enter the following: "body:contains('Check in message')": "IM_notification.ogg" 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Created attachment 29764 [details] [review] Bug 11431 [1] - Replace existing system with no change in functionality This patch set adds the ability to select which sounds are used for Koha's audio alerts, and which elements trigger those audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Set the various OverrideAudioAlert preferences to different sounds 6) Retest, verify new sounds play 7) Add/remove selectors from the AudioAlertSelector system preferences 8) Observe the difference in audio alert behavior based on your selections Created attachment 29765 [details] [review] Bug 11431 [2] - Rename soundon to AudioAlerts, move to it's own preference section Created attachment 29766 [details] [review] Bug 11431 [3] - Add ability to customize the warning, action, and success sounds Created attachment 29767 [details] [review] Bug 11431 [4] - Add the ability to choose the elements that trigger an audio alert Created attachment 29768 [details] [review] Bug 11431 [5] - Update sysprefs.sql Created attachment 29769 [details] [review] Bug 11431 [6] - Add the ability to specify arbitrary audio alerts This patch further enhances our control over Koha audio alerts by allowing a library to specify custom audio events. A new system preference is added the combines the functions of the Selector and Override systems preferences in a list of selector:sound combinations. These definitions override the default sounds and thus gives the library a way to completely redefine what events trigger and audio alert, and what sounds are played! Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Edit the new AudioAlertsCustom system preference and enter the following: "body:contains('Check in message')": "IM_notification.ogg" 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Created attachment 31145 [details] [review] [SIGNED OFF] Bug 11431 [1] - Replace existing system with no change in functionality This patch set adds the ability to select which sounds are used for Koha's audio alerts, and which elements trigger those audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Set the various OverrideAudioAlert preferences to different sounds 6) Retest, verify new sounds play 7) Add/remove selectors from the AudioAlertSelector system preferences 8) Observe the difference in audio alert behavior based on your selections Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31146 [details] [review] [SIGNED OFF] Bug 11431 [2] - Rename soundon to AudioAlerts, move to it's own preference section Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31147 [details] [review] [SIGNED OFF] Bug 11431 [3] - Add ability to customize the warning, action, and success sounds Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31148 [details] [review] [SIGNED OFF] Bug 11431 [4] - Add the ability to choose the elements that trigger an audio alert Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31149 [details] [review] [SIGNED OFF] Bug 11431 [5] - Update sysprefs.sql Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31150 [details] [review] [SIGNED OFF] Bug 11431 [6] - Add the ability to specify arbitrary audio alerts This patch further enhances our control over Koha audio alerts by allowing a library to specify custom audio events. A new system preference is added the combines the functions of the Selector and Override systems preferences in a list of selector:sound combinations. These definitions override the default sounds and thus gives the library a way to completely redefine what events trigger and audio alert, and what sounds are played! Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Edit the new AudioAlertsCustom system preference and enter the following: "body:contains('Check in message')": "IM_notification.ogg" 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> All sounds seemed to work with different .ogg .mp3 .wav sound types in firefox and chrome. I like the idea of this feature very much, but I have to be honest I find the configuration system for this really confusing. I don't think it's very user-friendly to require that you enter data in a very specific format. What about offering a set of form fields where an arbitrary number of lines could be added: [ jquery selector ] [ dropdown of existing media files ] - or - [ path to remote file] You might even be able to roll all of the preferences (except the "enable" one) into such a system if you included the defaults as choices under [ jquery selector]. I'm also curious about the possible performance hit the staff client takes by processing these DOM checks on every page, especially in slower systems. However, I'm not sure what is the best way to test that. Sorry I know that's a lot to ask after so much time and effort was put into this patch, but I would really like to see a feature as easy to use as the options for choosing an item type image. The other thing I noticed when testing this was that it didn't seem that any defaults were honored. After applying the patch and running the update I didn't have any circulation alert sounds anymore. Does this require that I re-enter them? I didn't think so based on my examination of the changes. (In reply to Owen Leonard from comment #33) > The other thing I noticed when testing this was that it didn't seem that any > defaults were honored. After applying the patch and running the update I > didn't have any circulation alert sounds anymore. Does this require that I > re-enter them? I didn't think so based on my examination of the changes. That is correct, you should have had the same default sound behavior as before. I'll have to re-test. Are you saying that you had sound alerts at all after patching and updating the db, or was it just a specific sound that was missing? Created attachment 34637 [details] [review] Bug 11431 [1] - Replace existing system with no change in functionality This patch set adds the ability to select which sounds are used for Koha's audio alerts, and which elements trigger those audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Set the various OverrideAudioAlert preferences to different sounds 6) Retest, verify new sounds play 7) Add/remove selectors from the AudioAlertSelector system preferences 8) Observe the difference in audio alert behavior based on your selections Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34638 [details] [review] Bug 11431 [2] - Rename soundon to AudioAlerts, move to it's own preference section Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34639 [details] [review] Bug 11431 [3] - Add ability to customize the warning, action, and success sounds Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34640 [details] [review] Bug 11431 [4] - Add the ability to choose the elements that trigger an audio alert Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34641 [details] [review] Bug 11431 [5] - Update sysprefs.sql Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34642 [details] [review] Bug 11431 [6] - Add the ability to specify arbitrary audio alerts This patch further enhances our control over Koha audio alerts by allowing a library to specify custom audio events. A new system preference is added the combines the functions of the Selector and Override systems preferences in a list of selector:sound combinations. These definitions override the default sounds and thus gives the library a way to completely redefine what events trigger and audio alert, and what sounds are played! Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Edit the new AudioAlertsCustom system preference and enter the following: "body:contains('Check in message')": "IM_notification.ogg" 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> All sounds seemed to work with different .ogg .mp3 .wav sound types in firefox and chrome. Created attachment 34643 [details] [review] Bug 11431 [7] - Add new table for audio alerts Created attachment 34644 [details] [review] Bug 11431 [8] - Add audio alerts editor Created attachment 34645 [details] [review] Bug 11431 [9] - Remove new system preferences that are no longer needed Created attachment 34646 [details] [review] Bug 11431 - Update database Created attachment 34647 [details] [review] Add audio alerts to installer Created attachment 34648 [details] [review] Bug 11431 - Update schema Created attachment 34649 [details] [review] Bug 11431 - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Created attachment 34650 [details] [review] Bug 11431 [1] - Update database Created attachment 34651 [details] [review] Bug 11431 [2] - Add audio alerts to installer Created attachment 34652 [details] [review] Bug 11431 [3] - Update schema Created attachment 34653 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Applying: Bug 11431 [4] - Add additional sound options fatal: sha1 information is lacking or useless (Koha/Object.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 11431 [4] - Add additional sound options When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-11431-4---Add-additional-sound-options-7v_z2F.patch forgot the depends Still can't test, after applying 13019 and 11431 OPAC functions but I cannot access any pages in staff client, I only get the error below: Software error: Template process failed: undef error - hash- or arrayref expected (not a simple scalar, use allow_nonref to allow this) at /home/nick/kohaclone/Koha/Template/Plugin/Koha.pm line 52. For help, please send mail to the webmaster ([no address given]), giving this error message and the time and date of the error. Was an issue with updatedatabase, cleared and tested on two different machines and worked in both Chrome and firefox with ogg, wav and mp3. Created attachment 34717 [details] [review] [SIGNED OFF]Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34718 [details] [review] [SIGNED OFF]Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34719 [details] [review] [SIGNED OFF]Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34720 [details] [review] [SIGNED OFF]Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34864 [details] [review] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34865 [details] [review] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34866 [details] [review] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 34867 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 35843 [details] [review] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 35844 [details] [review] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 35845 [details] [review] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 35846 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> (In reply to Kyle M Hall from comment #67) > Created attachment 35846 [details] [review] [review] Just wondering if http:// vs https:// matters for this: + // This is way faster than substring + if ( ! ( sound.charAt(4) == ':' && sound.charAt(5) == '/' && sound.charAt(6) == '/' ) ) { (In reply to paxed from comment #68) > (In reply to Kyle M Hall from comment #67) > > Created attachment 35846 [details] [review] [review] [review] > > Just wondering if http:// vs https:// matters for this: > > + // This is way faster than substring > + if ( ! ( sound.charAt(4) == ':' && sound.charAt(5) == '/' && > sound.charAt(6) == '/' ) ) { Good catch, maybe a regex is much more appropriate here. Our libraries workflow would be greatly enhanced by getting this enhancement wrapped up and pushed for release. Can someone take some time to look at this again? Created attachment 38547 [details] [review] Bug 11431 [QA Followup] - Allow https in sound URLs Created attachment 38548 [details] [review] Bug 11431 [QA Followup] - Fix POD This does not apply. You should change the collate of the new table, remove the "soundon" occurrence in C4/UsageStats.pm. I am a bit concerned by the Koha::Template::Plugin::Koha::AudioAlerts subroutine, not sure it's the best place to put it. Created attachment 40726 [details] [review] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 40727 [details] [review] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 40728 [details] [review] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 40729 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 40730 [details] [review] Bug 11431 [QA Followup] - Allow https in sound URLs Created attachment 40731 [details] [review] Bug 11431 [QA Followup] - Fix POD Created attachment 40732 [details] [review] Bug 11431 - Update UsageStats.pm QA comments: 1/ There are no tests for new modules 2/ I really would like to see Koha::Template::Plugin::Koha::AudioAlerts moved somewhere else 3/ SyntaxError: missing ) after condition ndexOf('http://') == 0 || sound.indexOf('https://') == 0 ) { Marked as Failed QA. Created attachment 41634 [details] [review] Bug 11431 [QA Followup] - Fix js error Created attachment 41661 [details] [review] Bug 11431 [QA Followup] - Unit Tests Created attachment 41662 [details] [review] Bug 11431 [QA Followup] - Simplify audio_alert_id to just id (In reply to Kyle M Hall from comment #84) > Created attachment 41662 [details] [review] [review] > Bug 11431 [QA Followup] - Simplify audio_alert_id to just id audio_alert_id is very wordy and it's a better convention to just give the table id the name 'id'. No changes in the code were needed since I was already using the id() method. Unit tests pass pre and post patch. CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_client.pref CONFLICT (content): Merge conflict in Koha/Template/Plugin/Koha.pm (In reply to Jonathan Druart from comment #81) > 2/ I really would like to see Koha::Template::Plugin::Koha::AudioAlerts > moved somewhere else Still needs an answer. Are not you afraid that this plugin become a holdall? Created attachment 42678 [details] [review] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 42679 [details] [review] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 42680 [details] [review] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 42681 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 42682 [details] [review] Bug 11431 [QA Followup] - Allow https in sound URLs Created attachment 42683 [details] [review] Bug 11431 [QA Followup] - Fix POD Created attachment 42684 [details] [review] Bug 11431 - Update UsageStats.pm Created attachment 42685 [details] [review] Bug 11431 [QA Followup] - Fix js error Created attachment 42686 [details] [review] Bug 11431 [QA Followup] - Unit Tests Created attachment 42687 [details] [review] Bug 11431 [QA Followup] - Simplify audio_alert_id to just id Created attachment 42688 [details] [review] Bug 11431 [QA Followup] - Move AudioAlerts to a separate plugin Created attachment 42689 [details] [review] Bug 11431 [QA Followup] - Hard code list of sounds for security The sounds directory is no longer available for listing for security. It is better and safer to hard code the list of sounds. (In reply to Jonathan Druart from comment #86) > CONFLICT (content): Merge conflict in > koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_client.pref > CONFLICT (content): Merge conflict in Koha/Template/Plugin/Koha.pm Fixed! > (In reply to Jonathan Druart from comment #81) > > 2/ I really would like to see Koha::Template::Plugin::Koha::AudioAlerts > > moved somewhere else > > Still needs an answer. > Are not you afraid that this plugin become a holdall? I understand what you mean. Fixed! Tested just to confirm my sign off. Test plan works as expected. Minor bug: If you delete a sound alert, the preferences are not automatically renumbered and you get an error if you try to move another alert into the empty slot. To recreate: 1 - Add an alert (example in test plan works fine) 2 - Move it to priority 2 3 - Delete it 4 - Remaining alerts are preference 1,3,4 5 - Try to move #3 up one slot 6 - Receive error: Can't call method "precedence" on an undefined value at /home/nick/kohaclone/Koha/AudioAlerts.pm line 96. 7 - Back up, move alert number 3 to top (second arrow) 8 - Priorities are renumbered correctly Created attachment 42696 [details] [review] Bug 11431 [QA Followup] - Fix alter precedences on delete Thanks for catching that! Followup posted! (In reply to Nick Clemens from comment #100) > Tested just to confirm my sign off. Test plan works as expected. > > Minor bug: If you delete a sound alert, the preferences are not > automatically renumbered and you get an error if you try to move another > alert into the empty slot. > > To recreate: > 1 - Add an alert (example in test plan works fine) > 2 - Move it to priority 2 > 3 - Delete it > 4 - Remaining alerts are preference 1,3,4 > 5 - Try to move #3 up one slot > 6 - Receive error: Can't call method "precedence" on an undefined value at > /home/nick/kohaclone/Koha/AudioAlerts.pm line 96. > 7 - Back up, move alert number 3 to top (second arrow) > 8 - Priorities are renumbered correctly Patch doesn't apply - please fix! Created attachment 43753 [details] [review] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 43754 [details] [review] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 43755 [details] [review] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 43756 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 43757 [details] [review] Bug 11431 [QA Followup] - Allow https in sound URLs Created attachment 43758 [details] [review] Bug 11431 [QA Followup] - Fix POD Created attachment 43759 [details] [review] Bug 11431 - Update UsageStats.pm Created attachment 43760 [details] [review] Bug 11431 [QA Followup] - Fix js error Created attachment 43761 [details] [review] Bug 11431 [QA Followup] - Unit Tests Created attachment 43762 [details] [review] Bug 11431 [QA Followup] - Simplify audio_alert_id to just id Created attachment 43763 [details] [review] Bug 11431 [QA Followup] - Move AudioAlerts to a separate plugin Created attachment 43764 [details] [review] Bug 11431 [QA Followup] - Hard code list of sounds for security The sounds directory is no longer available for listing for security. It is better and safer to hard code the list of sounds. Created attachment 43765 [details] [review] Bug 11431 [QA Followup] - Fix alter precedences on delete Hi Kyle, one small thing: If AudioAlerts is turned off the 'play' button on the administration page won't work and save the entry instead. That was quite confusing. I think independent of the AudioAlerts setting, you should be able to 'preview' the sounds on the admin page. The second best solution would be to not display the play button in this case. Enhancement ideas: - I'd really really like an option to play the sounds of saved entries for easier testing and also the option of editing the entries - if you get your selector wrong, it can be quite annoying to delete and start over. - Adding an example to the help page would be great. The example on the bug report is not quite so good, as it relies on the existence of an English string in a certain place and would be hard to translate, but maybe another example that is based on a class or id? Can you please take a look? Created attachment 43817 [details] [review] Bug 11431 [QA Followup] - Always allow audio alerts for audio alerts editor page Created attachment 43820 [details] [review] Bug 11431 [QA Followup] - Add ability to edit audio alerts Created attachment 43821 [details] [review] Bug 11431 [QA Followup] - Update help Created attachment 43822 [details] [review] Bug 11431 [QA Followup] - Add ability to edit audio alerts Created attachment 43823 [details] [review] Bug 11431 [QA Followup] - Update help Created attachment 43824 [details] [review] Bug 11431 [QA Followup] - Update help Created attachment 43825 [details] [review] Bug 11431 [QA Followup] - Add ability to edit audio alerts Created attachment 43887 [details] [review] [PASSED QA] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43888 [details] [review] [PASSED QA] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43889 [details] [review] [PASSED QA] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43890 [details] [review] [PASSED QA] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43891 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Allow https in sound URLs Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43892 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Fix POD Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43893 [details] [review] [PASSED QA] Bug 11431 - Update UsageStats.pm Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43894 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Fix js error Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43895 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Unit Tests Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43896 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Simplify audio_alert_id to just id Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43897 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Move AudioAlerts to a separate plugin Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43898 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Hard code list of sounds for security The sounds directory is no longer available for listing for security. It is better and safer to hard code the list of sounds. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43899 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Fix alter precedences on delete Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43900 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Always allow audio alerts for audio alerts editor page Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43901 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Update help Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43902 [details] [review] [PASSED QA] Bug 11431 [QA Followup] - Add ability to edit audio alerts Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43903 [details] [review] Bug 11431: QA follow-up: Typo in help file / variable name Fixes the variable name to avoid a double-up that made it impossible to turn off the sounds in general. If AudioAlerts are turned off, they will only work on the audio administration page now so you can set up and test the different sounds there. Also fixes a little typo in the help file. Created attachment 44391 [details] [review] Bug 11431 [QA Followup] - Make audio alerts sql top level Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 44392 [details] [review] Bug 11431 [1] - Update database Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44393 [details] [review] Bug 11431 [2] - Add audio alerts to installer http://bugs.koha-community.org/show_bug.cgi?id=11431 Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44394 [details] [review] Bug 11431 [3] - Update schema Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44395 [details] [review] Bug 11431 [4] - Add additional sound options This patch set replaces and extends Koha's current sound options. This is implemented be removing the existing sound system, and re-engineering using a table of selector/sound combinations such that the highest precedence selector that is found in the DOM will trigger and audio alert. The existing audio behaviors are implemented as a set of default audio alerts. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Enable the AudioAlerts system preference 4) Test existing sounds 5) Enter the new alerts editor in the admin section 6) Add a new audo alert with the following selector: "body:contains('Check in message')", choose any sound alert you wish, make sure it's not one of the 3 sounds already used! Make this selector precedence 1 4) Browse to the checkins page, you should hear the default sound 5) Attempt to return an invalid barcode, you should hear your custom sound! Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44396 [details] [review] Bug 11431 [QA Followup] - Allow https in sound URLs Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44397 [details] [review] Bug 11431 [QA Followup] - Fix POD Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44398 [details] [review] Bug 11431 - Update UsageStats.pm Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44399 [details] [review] Bug 11431 [QA Followup] - Fix js error Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44400 [details] [review] Bug 11431 [QA Followup] - Unit Tests Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44401 [details] [review] Bug 11431 [QA Followup] - Simplify audio_alert_id to just id Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44402 [details] [review] Bug 11431 [QA Followup] - Move AudioAlerts to a separate plugin Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44404 [details] [review] Bug 11431 [QA Followup] - Hard code list of sounds for security The sounds directory is no longer available for listing for security. It is better and safer to hard code the list of sounds. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44405 [details] [review] Bug 11431 [QA Followup] - Fix alter precedences on delete Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44407 [details] [review] Bug 11431 [QA Followup] - Always allow audio alerts for audio alerts editor page Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44408 [details] [review] Bug 11431 [QA Followup] - Update help Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44409 [details] [review] Bug 11431 [QA Followup] - Add ability to edit audio alerts Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 44411 [details] [review] Bug 11431: QA follow-up: Typo in help file / variable name Fixes the variable name to avoid a double-up that made it impossible to turn off the sounds in general. If AudioAlerts are turned off, they will only work on the audio administration page now so you can set up and test the different sounds there. Also fixes a little typo in the help file. Created attachment 44412 [details] [review] Bug 11431 [QA Followup] - Make audio alerts sql top level Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Patches pushed to master. Great job Kyle! Created attachment 44599 [details] [review] Bug 11431: (QA followup) remove diags Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 45135 [details] [review] Bug 11431: (follow-up) Update database This patch specifies the collate for the new audio_alerts table to the updatedb entry. *** Bug 18416 has been marked as a duplicate of this bug. *** |