Fix most-recently-used-entry delete bug#1296
Merged
Merged
Conversation
This replaces the low-level setCurrentEntry() method with the higher-level editEntryAndScroll() method that we use to change entry state. The higher-level method handles URL updating and MRU cache updates among other things. This fixes a bug where deleting an entry did not update the URL or update the MRU cache, which could result in an error state under certain circumstances.
Add a check to ensure the MRU entry actually exists in the list before setting the entryId. The previous commit ensures that the MRU cache is updated upon deleting an entry, so I'm not 100% sure that this check is needed, but it shouldn't hurt, so I think it's safe to add as a precaution against an invalid state.
longrunningprocess
approved these changes
Feb 4, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is a follow up PR to #1252 where during QA testing @longrunningprocess discovered a bug whereby deleting an entry caused the MRU cache and URL to not be updated, thereby creating a situation where the app could go to an invalid state.
Addresses testing and comments called out in #1252
Also contains an unrelated tweak to a GH issue template 😁
Type of Change
Testing on your branch
Please describe how to test and/or verify your changes. Provide instructions so we can reproduce. Please also provide relevant test data as necessary. These instructions will be used for QA testing below.
Checklist
qa.languageforge.org testing
Reviewers: add/replace your name below and check the box to sign-off/attest the feature works as expected on qa.languageforge.org
'2022-02-04T17:42:24.670Z'