Skip to content

fix(AddLessonConfigureComponent): Pressing Enter adds the same lesson twice - #2348

Merged
hirokiterashima merged 2 commits into
WISE-Community:developfrom
Isaries:fix/add-lesson-enter-double-submit
Oct 21, 2026
Merged

hirokiterashima merged 2 commits into
WISE-Community:developfrom
Isaries:fix/add-lesson-enter-double-submit

Conversation

@Isaries

@Isaries Isaries commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Pressing Enter in the lesson title field of the add lesson view calls submit() twice: once from (keydown.enter) on the <form> and once from (keyup)="$event.keyCode == 13 && submit()" on the <input>. The preventDefault() on the keydown does not stop the keyup. submitting disables the Submit button (the fix in #1833), but submit() itself never checks it, so the Enter path is not covered.

The second run does not produce a second lesson. It produces a broken one:

  1. getNextAvailableGroupId() reads this.groupNodes, which is only rebuilt when the project is parsed again after the save. The second run therefore gets the same id as the first.
  2. addNode() replaces the node with that id, so nodes holds one lesson, but insertNodeAfterInGroups() splices the id into the parent a second time, so group0.ids contains it twice.
  3. insertNodeAfterInTransitions() copies the transitions of the node it inserts after. That node already transitions to the new lesson, so the new lesson ends up transitioning to itself.

Both versions are saved, the second one last. The next time the unit is opened, parseProject() reaches the empty lesson in getAllPaths() and recurses until RangeError: Maximum call stack size exceeded. The authoring tool then falls back to the home page, and preview fails the same way, so the unit can no longer be opened. Copying the unit copies the broken JSON.

To reproduce: open a unit in the authoring tool, choose to add a lesson, type a title and press Enter. project.json now lists the new lesson twice in group0.ids with a transition to itself, and reopening the unit returns to the home page.

Changes

  • Remove the (keyup) handler on the title input. The form's (keydown.enter) already handles Enter.
  • submit() returns early while submitting is true or the form is invalid, the same conditions that disable the Submit button. Previously Enter on an empty title also added a lesson.

#2349 makes getLessonPaths() stop at such a loop, so units already saved in this state can be opened and repaired. This PR stops new ones from being created.

Test

  • New add-lesson-configure.component.spec.ts: one Enter adds one lesson, a second Enter while the save is in flight adds nothing, and Enter on an empty title adds nothing. Against the current develop, the first test reports createGroup called 2 times.

… twice

Pressing Enter in the lesson title field ran submit() from both the
form's keydown.enter and the input's keyup handler. The second run
computed the same group id, because the next id comes from the parsed
project that the in-flight save has not refreshed yet, and inserted the
lesson again: a duplicate id in the parent's ids and a transition from
the lesson to itself. Opening the unit afterwards overflows the stack
in getAllPaths() and the authoring tool falls back to the home page.

Remove the keyup handler and make submit() ignore calls while a save is
in flight or the title is empty, the same conditions that already
disable the Submit button.
@hirokiterashima hirokiterashima added the bug Something isn't working label Oct 8, 2026
@hirokiterashima hirokiterashima self-assigned this Oct 8, 2026

@Aaron-Detre Aaron-Detre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@hirokiterashima
hirokiterashima merged commit 3da84c7 into WISE-Community:develop Oct 21, 2026
3 of 4 checks passed
@hirokiterashima

Copy link
Copy Markdown
Member

Thanks for catching and fixing this issue @Isaries!

@hirokiterashima

Copy link
Copy Markdown
Member

🎉 This PR is included in version 5.248.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants