fix(delete-modal): close modal after successful delete
The success path of handleConfirm() never reset the isSubmitting flag, so hide()'s early-return guard (line 209: 'if (isSubmitting) return') kept the modal visible after a successful delete — leaving the user looking at a stuck 'Deleting...' dialog while the card had already been removed by the WebSocket SERIES_DELETED event. Reset isSubmitting and the confirm button text before calling hide(), and capture currentKey into a local before hide() nulls it so the follow-up removeSeries() call receives the right key.
This commit is contained in:
@@ -300,6 +300,9 @@ AniWorld.DeleteModal = (function() {
|
|||||||
var result = await response.json();
|
var result = await response.json();
|
||||||
console.info('[DeleteModal] Delete succeeded:', result);
|
console.info('[DeleteModal] Delete succeeded:', result);
|
||||||
|
|
||||||
|
// Capture key before hide() nulls currentKey
|
||||||
|
var deletedKey = currentKey;
|
||||||
|
|
||||||
// Show success message based on what was deleted
|
// Show success message based on what was deleted
|
||||||
var msgParts = [];
|
var msgParts = [];
|
||||||
if (result.deleted_from_database) msgParts.push('removed from database');
|
if (result.deleted_from_database) msgParts.push('removed from database');
|
||||||
@@ -311,11 +314,17 @@ AniWorld.DeleteModal = (function() {
|
|||||||
: 'Delete completed.';
|
: 'Delete completed.';
|
||||||
|
|
||||||
AniWorld.UI.showToast(successMsg, result.success ? 'success' : 'warning');
|
AniWorld.UI.showToast(successMsg, result.success ? 'success' : 'warning');
|
||||||
|
|
||||||
|
// Close modal and reset submission state together — isSubmitting must
|
||||||
|
// be cleared before hide(), otherwise hide() bails out (early return
|
||||||
|
// on the !isSubmitting guard) and the modal stays visible.
|
||||||
|
isSubmitting = false;
|
||||||
|
if (confirmBtn) confirmBtn.textContent = 'Delete';
|
||||||
hide();
|
hide();
|
||||||
|
|
||||||
// Remove the card from the grid directly
|
// Remove the card from the grid directly
|
||||||
if (AniWorld.SeriesManager && AniWorld.SeriesManager.removeSeries) {
|
if (AniWorld.SeriesManager && AniWorld.SeriesManager.removeSeries) {
|
||||||
AniWorld.SeriesManager.removeSeries(currentKey);
|
AniWorld.SeriesManager.removeSeries(deletedKey);
|
||||||
}
|
}
|
||||||
|
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|||||||
@@ -318,6 +318,121 @@ class TestDeleteModalSeriesManagerIntegration:
|
|||||||
assert remove_called_with == [key]
|
assert remove_called_with == [key]
|
||||||
|
|
||||||
|
|
||||||
|
class TestDeleteModalModalCloseAfterSuccess:
|
||||||
|
"""Regression tests for the bug where the modal stayed visible after a
|
||||||
|
successful delete because isSubmitting was never reset on the success path.
|
||||||
|
|
||||||
|
Source-level tests (the JS is not run in pytest): they assert that the
|
||||||
|
handleConfirm success branch (a) clears isSubmitting before hide(), and
|
||||||
|
(b) removes the card via a key captured before hide() nulls currentKey.
|
||||||
|
"""
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _read_source():
|
||||||
|
import os
|
||||||
|
path = os.path.join(
|
||||||
|
os.path.dirname(__file__),
|
||||||
|
'..', '..',
|
||||||
|
'src', 'server', 'web', 'static', 'js', 'index', 'delete-modal.js'
|
||||||
|
)
|
||||||
|
with open(path, 'r') as f:
|
||||||
|
return f.read()
|
||||||
|
|
||||||
|
def test_isSubmitting_reset_on_success_path(self):
|
||||||
|
"""isSubmitting must be reset to false after a successful delete,
|
||||||
|
otherwise hide()'s early-return guard keeps the modal visible."""
|
||||||
|
src = self._read_source()
|
||||||
|
|
||||||
|
# Locate the success branch: it begins with "Delete succeeded:" log
|
||||||
|
success_idx = src.find("Delete succeeded:")
|
||||||
|
assert success_idx > 0, "Could not find Delete succeeded log line"
|
||||||
|
|
||||||
|
# Everything between the success log and the catch block belongs to
|
||||||
|
# the success path.
|
||||||
|
catch_idx = src.find("} catch (err)", success_idx)
|
||||||
|
assert catch_idx > 0, "Could not find catch block after success path"
|
||||||
|
|
||||||
|
success_branch = src[success_idx:catch_idx]
|
||||||
|
|
||||||
|
# The flag must be reset in this branch...
|
||||||
|
assert "isSubmitting = false" in success_branch, (
|
||||||
|
"isSubmitting is never reset on the success path — "
|
||||||
|
"this is the bug that left the modal visible with 'Deleting...'"
|
||||||
|
)
|
||||||
|
|
||||||
|
# ...BEFORE hide() is called.
|
||||||
|
reset_pos = success_branch.find("isSubmitting = false")
|
||||||
|
hide_pos = success_branch.find("hide();")
|
||||||
|
assert reset_pos > 0 and hide_pos > 0, (
|
||||||
|
"Could not locate isSubmitting reset or hide() call"
|
||||||
|
)
|
||||||
|
assert reset_pos < hide_pos, (
|
||||||
|
"isSubmitting must be cleared BEFORE hide() — otherwise hide()'s "
|
||||||
|
"guard (`if (isSubmitting) return`) bails out and the modal stays"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_modal_hidden_class_added_on_success(self):
|
||||||
|
"""After the success-path cleanup, hide() must run and apply the
|
||||||
|
'hidden' class. We verify by checking hide() is reached after the
|
||||||
|
isSubmitting reset (covered above) and that the reset precedes the
|
||||||
|
removal of the card from the grid."""
|
||||||
|
src = self._read_source()
|
||||||
|
|
||||||
|
success_idx = src.find("Delete succeeded:")
|
||||||
|
catch_idx = src.find("} catch (err)", success_idx)
|
||||||
|
success_branch = src[success_idx:catch_idx]
|
||||||
|
|
||||||
|
reset_pos = success_branch.find("isSubmitting = false")
|
||||||
|
hide_pos = success_branch.find("hide();")
|
||||||
|
remove_pos = success_branch.find("removeSeries(")
|
||||||
|
|
||||||
|
assert 0 < reset_pos < hide_pos < remove_pos, (
|
||||||
|
"Order on success path must be: "
|
||||||
|
"isSubmitting reset -> hide() -> removeSeries()"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_removeSeries_uses_captured_key_not_live_currentKey(self):
|
||||||
|
"""hide() nulls currentKey on line ~211. If removeSeries reads the
|
||||||
|
live currentKey AFTER hide(), it gets null and silently no-ops. The
|
||||||
|
fix captures the key into a local before hide() runs."""
|
||||||
|
src = self._read_source()
|
||||||
|
|
||||||
|
success_idx = src.find("Delete succeeded:")
|
||||||
|
catch_idx = src.find("} catch (err)", success_idx)
|
||||||
|
success_branch = src[success_idx:catch_idx]
|
||||||
|
|
||||||
|
# There must be a local capture of the key before hide().
|
||||||
|
assert "var deletedKey = currentKey;" in success_branch, (
|
||||||
|
"Success path must capture currentKey into a local before "
|
||||||
|
"hide() nulls it — otherwise removeSeries(currentKey) would be "
|
||||||
|
"a silent no-op."
|
||||||
|
)
|
||||||
|
|
||||||
|
# The removeSeries call must reference the captured local, not
|
||||||
|
# currentKey directly.
|
||||||
|
capture_pos = success_branch.find("var deletedKey = currentKey;")
|
||||||
|
remove_pos = success_branch.find("removeSeries(deletedKey)")
|
||||||
|
assert capture_pos > 0 and remove_pos > 0, (
|
||||||
|
"removeSeries must be called with the captured deletedKey"
|
||||||
|
)
|
||||||
|
assert capture_pos < remove_pos, (
|
||||||
|
"Capture must happen BEFORE removeSeries reads it"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_confirm_button_text_reset_on_success(self):
|
||||||
|
"""The button text is changed to 'Deleting...' during submit and must
|
||||||
|
be reverted to 'Delete' so the modal is in a clean state if reopened."""
|
||||||
|
src = self._read_source()
|
||||||
|
|
||||||
|
success_idx = src.find("Delete succeeded:")
|
||||||
|
catch_idx = src.find("} catch (err)", success_idx)
|
||||||
|
success_branch = src[success_idx:catch_idx]
|
||||||
|
|
||||||
|
assert "confirmBtn.textContent = 'Delete'" in success_branch, (
|
||||||
|
"Confirm button text must be reset to 'Delete' on the success path"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TestDeleteModalConstants:
|
class TestDeleteModalConstants:
|
||||||
"""Tests for SERIES_DELETED WebSocket event constant."""
|
"""Tests for SERIES_DELETED WebSocket event constant."""
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user