Bug 27250 - DELETE calls are stacked on the SMTP servers admin page
Summary: DELETE calls are stacked on the SMTP servers admin page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 22343
Blocks:
  Show dependency treegraph
 
Reported: 2020-12-16 12:39 UTC by Jonathan Druart
Modified: 2022-06-06 20:25 UTC (History)
5 users (show)

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


Attachments
Bug 27250: Prevent DELETE to be called several times on SMTP servers admin page (1.54 KB, patch)
2020-12-16 12:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALTERNATE] Bug 27250: Fix stacked event listeners in SMTP delete modal (4.08 KB, patch)
2020-12-17 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27250: Fix stacked event listeners in SMTP delete modal (4.14 KB, patch)
2020-12-17 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27250: Fix stacked event listeners in SMTP delete modal (4.20 KB, patch)
2020-12-21 16:29 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-12-16 12:39:42 UTC
To recreate:
Create at least 2 SMTP servers
Click "Delete" for server 1
=> You get a successful message (OK)
Click "Delete" for server 2
=> You get a successful message (OK)
=> You get also an alert message about server 1 (KO)

The DELETE route is called twice (one per Delete button you clicked)
Comment 1 Jonathan Druart 2020-12-16 12:41:16 UTC
Created attachment 114434 [details] [review]
Bug 27250: Prevent DELETE to be called several times on SMTP servers admin page

The on click event is not replaced but added. We must unbind it before
adding the second one

To recreate:
Create at least 2 SMTP servers
Click "Delete" for server 1
=> You get a successful message (OK)
Click "Delete" for server 2
=> You get a successful message (OK)
=> You get also an alert message about server 1 (KO)

The DELETE route is called twice (one per Delete button you clicked)
Comment 2 Tomás Cohen Arazi 2020-12-17 14:59:26 UTC
Created attachment 114480 [details] [review]
[ALTERNATE] Bug 27250: Fix stacked event listeners in SMTP delete modal

There is a design issue on the click event listener that makes the modal
get added new event listeners on each row being deleted. This makes the
page call all the event listeners and generates an error.

This patch replaces the flawed logic (adding the event listener inside
the event listener). It makes the event listener be defined at startup
time, and make the .on('click') action just pass information around as
required.

To test:
1. Create at least 2 SMTP servers
2. Open the browser inspector, on the network tab
3. Click "Delete" for server 1
=> SUCCESS: You get a successful message (OK)
4. Click "Delete" for server 2
=> SUCCESS: You get a successful message (OK)
=> FAIL: You get also an alert message about server 1 (KO)
=> FAIL: The inspector shows more than one AJAX call has been made
5. Apply this patch and reload the page
6. Repeat 3-4
=> SUCCESS: Everything goes smooth this time
7. Sign off :-D
Comment 3 Jonathan Druart 2020-12-17 16:00:21 UTC
Created attachment 114484 [details] [review]
Bug 27250: Fix stacked event listeners in SMTP delete modal

There is a design issue on the click event listener that makes the modal
get added new event listeners on each row being deleted. This makes the
page call all the event listeners and generates an error.

This patch replaces the flawed logic (adding the event listener inside
the event listener). It makes the event listener be defined at startup
time, and make the .on('click') action just pass information around as
required.

To test:
1. Create at least 2 SMTP servers
2. Open the browser inspector, on the network tab
3. Click "Delete" for server 1
=> SUCCESS: You get a successful message (OK)
4. Click "Delete" for server 2
=> SUCCESS: You get a successful message (OK)
=> FAIL: You get also an alert message about server 1 (KO)
=> FAIL: The inspector shows more than one AJAX call has been made
5. Apply this patch and reload the page
6. Repeat 3-4
=> SUCCESS: Everything goes smooth this time
7. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Nick Clemens 2020-12-21 16:29:38 UTC
Created attachment 114569 [details] [review]
Bug 27250: Fix stacked event listeners in SMTP delete modal

There is a design issue on the click event listener that makes the modal
get added new event listeners on each row being deleted. This makes the
page call all the event listeners and generates an error.

This patch replaces the flawed logic (adding the event listener inside
the event listener). It makes the event listener be defined at startup
time, and make the .on('click') action just pass information around as
required.

To test:
1. Create at least 2 SMTP servers
2. Open the browser inspector, on the network tab
3. Click "Delete" for server 1
=> SUCCESS: You get a successful message (OK)
4. Click "Delete" for server 2
=> SUCCESS: You get a successful message (OK)
=> FAIL: You get also an alert message about server 1 (KO)
=> FAIL: The inspector shows more than one AJAX call has been made
5. Apply this patch and reload the page
6. Repeat 3-4
=> SUCCESS: Everything goes smooth this time
7. Sign off :-D

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Jonathan Druart 2020-12-26 17:15:43 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 6 Fridolin Somers 2020-12-30 14:47:41 UTC
Pushed to 20.11.x for 20.11.01
Comment 7 Andrew Fuerste-Henry 2021-01-04 22:25:47 UTC
Missing dependencies, not backported to 20.05