Bug 13254 - Delete record don't wait for confirmation
Summary: Delete record don't wait for confirmation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P3 major (vote)
Assignee: Rafal Kopaczka
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-14 13:04 UTC by Rafal Kopaczka
Modified: 2015-12-03 22:02 UTC (History)
5 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:


Attachments
Bug 13254 - Delete record don't wait for confirmation (2.03 KB, patch)
2014-11-14 13:17 UTC, Rafal Kopaczka
Details | Diff | Splinter Review
Bug 13254 - Delete record don't wait for confirmation (2.21 KB, patch)
2014-11-14 18:30 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 13254 - Delete record don't wait for confirmation (2.33 KB, patch)
2014-11-15 08:28 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Rafal Kopaczka 2014-11-14 13:04:36 UTC
In some cases (eg. when Staf Client Search is active), when user choose Edit->Delete record on record tool bar, browser don't wait for confirmation and goes immediately to delete record.

To reproduce:
1. Search for some biblio records and choose one without items attached. 
2. Note that there, must be "Return to search results" box on left side, bug works in that case, when in normal view everything work fine.
3. Click Edit->Delete record, watch that confirmation box shows, but don't wait for OK and runs immediately. If you are fast enough to click OK, then you get error as below, because record was deleted earlier.
"Software error:
Can't call method "title" on an undefined value at /home/koha/kohaclon/cataloguing/addbiblio.pl line 826."
Comment 1 Owen Leonard 2014-11-14 13:11:30 UTC
This sounds like Bug 11817 which I thought was fixed. Are you testing in master?
Comment 2 Rafal Kopaczka 2014-11-14 13:17:00 UTC
(In reply to Owen Leonard from comment #1)
> This sounds like Bug 11817 which I thought was fixed. Are you testing in
> master?

Yes, today's master. It's maybe similar but removing (unnecessary) href from delete link in cat-toolbar.inc fixes problem.
I'll attach patch for a while.
Comment 3 Rafal Kopaczka 2014-11-14 13:17:30 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2014-11-14 18:25:30 UTC
I can reproduce the bug. 

After step 3 the confirmation dialog disappears after some seconds and advanced search page is displayed. After that, the biblio is gone.

Setting to P3 since the Bug leads to data loss.
Comment 5 Marc Véron 2014-11-14 18:30:03 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2014-11-15 08:28:21 UTC
Created attachment 33586 [details] [review]
[PASSED QA] Bug 13254 - Delete record don't wait for confirmation

In some cases (eg. when Staf Client Search is active), when user choose
Edit->Delete record on record tool bar, browser don't wait for
confirmation and goes immediately to delete record.

To reproduce:
1. Search for some biblio records and choose one without items attached.
2. Note that there, must be "Return to search results" box on left side,
bug works in that case, when in normal view everything work fine.
3. Click Edit->Delete record, watch that confirmation box shows, but
don't wait for OK and runs immediately. If you are fast enough to
click OK, then you get error as below, because record was deleted
earlier.

To test:
1. Apply patch.
2. Follow reproduce steps.
3. Check if waits for confirmation in all cases.
4. Check if deletes record after confirm.

Followed test plan. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Confirmed the problem and that the patch fixes it.
Good catch!
Comment 7 Tomás Cohen Arazi 2014-11-17 00:22:25 UTC
Patch pushed to master.

Thanks Rafal!
Comment 8 Fridolin Somers 2015-03-13 10:13:33 UTC
Dirty bug indeed.
Can this be backported to 3.16.x ?
Comment 9 Fridolin Somers 2015-03-13 10:16:00 UTC
This bug shows that the behavior of the browser is sometimes dangerous :
                    $('a[href*="biblionumber="]').click(function (ev) {
                        ev.preventDefault();
                        window.location = $(this).attr('href') + '&searchid=' + me.searchid;
                    });
In my opinion the selector 'a[href*="biblionumber="]' is too large.
It may impact future code.
Comment 10 Katrin Fischer 2015-03-13 10:29:34 UTC
Please see also bug 13644 - this piece of code can also break links under some circumstances.
Comment 11 Mason James 2015-03-26 00:38:39 UTC
(In reply to Fridolin SOMERS from comment #8)
> Dirty bug indeed.
> Can this be backported to 3.16.x ?

Pushed to 3.16.x, will be in 3.16.9
Comment 12 Fridolin Somers 2015-03-31 11:53:32 UTC
(In reply to Mason James from comment #11)
> (In reply to Fridolin SOMERS from comment #8)
> > Dirty bug indeed.
> > Can this be backported to 3.16.x ?
> 
> Pushed to 3.16.x, will be in 3.16.9

Thanks a lot ;)
Comment 13 Fridolin Somers 2015-04-02 07:36:20 UTC
Pushed to 3.14, will be in 3.14.15.