Fix order of string in editor (#10083)

* Fix order of string in editor

The string defined in a BibTeX file were shown in random order in the
respective GUI editor. While the code already sorts them, by storing the
items into a set, it destroys the order again. Fix this behaviour by
storing them as a list, which retains the order.

* Apply suggestions from code review

* Resort the strings table for new entry

Adding a new entry should also preserve the sorting of the strings
table, thus we need to resort the entries and take care that the focus
in the table is still on the newly-created entry.

* Add a test for the resorting

* Cleanup code

* Further cleanup for better reuse

CHANGELOG.md

@@ -118,6 +118,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where empty grey containers would remain in the groups panel, if displaying of group item count is turned off. [#9972](https://github.com/JabRef/jabref/issues/9972)
 - We fixed an issue where fetching an ISBN could lead to application freezing when the fetcher did not return any results. [#9979](https://github.com/JabRef/jabref/issues/9979)
 - We fixed an issue where closing a library containing groups and entries caused an exception [#9997](https://github.com/JabRef/jabref/issues/9997)
+- We fixed a bug where the editor for strings in a bibliography file did not sort the entries by their keys [#10083](https://github.com/JabRef/jabref/pull/10083)
 
 ### Removed
 

src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesView.java

@@ -62,8 +62,8 @@ public void initialize() {
                 .install(labelColumn, new DefaultStringConverter());
         labelColumn.setOnEditCommit((TableColumn.CellEditEvent<ConstantsItemModel, String> cellEvent) -> {
 
-            ConstantsItemModel cellItem = cellEvent.getTableView()
-                                                   .getItems()
+            var tableView = cellEvent.getTableView();
+            ConstantsItemModel cellItem = tableView.getItems()
                                                    .get(cellEvent.getTablePosition().getRow());
 
             Optional<ConstantsItemModel> existingItem = viewModel.labelAlreadyExists(cellEvent.getNewValue());
@@ -77,7 +77,13 @@ public void initialize() {
                 cellItem.setLabel(cellEvent.getNewValue());
             }
 
-            cellEvent.getTableView().refresh();
+            // Resort the entries based on the keys and set the focus to the newly-created entry
+            viewModel.resortStrings();
+            var selectionModel = tableView.getSelectionModel();
+            selectionModel.select(cellItem);
+            selectionModel.focus(selectionModel.getSelectedIndex());
+            tableView.refresh();
+            tableView.scrollTo(cellItem);
         });
 
         contentColumn.setSortable(true);

src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesViewModel.java

@@ -1,5 +1,7 @@
 package org.jabref.gui.libraryproperties.constants;
 
+import java.util.Comparator;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -48,7 +50,7 @@ public void setValues() {
         stringsListProperty.addAll(databaseContext.getDatabase().getStringValues().stream()
                                                   .sorted(new BibtexStringComparator(false))
                                                   .map(this::convertFromBibTexString)
-                                                  .collect(Collectors.toSet()));
+                                                  .toList());
     }
 
     public void addNewString() {
@@ -70,6 +72,11 @@ public void removeString(ConstantsItemModel item) {
         stringsListProperty.remove(item);
     }
 
+    public void resortStrings() {
+        // Resort the strings list in the same order as setValues() does
+        stringsListProperty.sort(Comparator.comparing(c -> c.labelProperty().get().toLowerCase(Locale.ROOT)));
+    }
+
     private ConstantsItemModel convertFromBibTexString(BibtexString bibtexString) {
         return new ConstantsItemModel(bibtexString.getName(), bibtexString.getContent());
     }

src/test/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesViewModelTest.java

@@ -0,0 +1,64 @@
+package org.jabref.gui.libraryproperties.constants;
+
+import java.util.List;
+
+import javafx.beans.property.StringProperty;
+
+import org.jabref.gui.DialogService;
+import org.jabref.model.database.BibDatabase;
+import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.entry.BibtexString;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+
+class ConstantsPropertiesViewModelTest {
+
+    @DisplayName("Check that the list of strings is sorted according to their keys")
+    @Test
+    void testStringsListPropertySorting() {
+        BibtexString string1 = new BibtexString("TSE", "Transactions on Software Engineering");
+        BibtexString string2 = new BibtexString("ICSE", "International Conference on Software Engineering");
+        BibDatabase db = new BibDatabase();
+        db.setStrings(List.of(string1, string2));
+        BibDatabaseContext context = new BibDatabaseContext(db);
+        DialogService service = mock(DialogService.class);
+        List<String> expected = List.of(string2.getName(), string1.getName()); // ICSE before TSE
+
+        ConstantsPropertiesViewModel model = new ConstantsPropertiesViewModel(context, service);
+        model.setValues();
+
+        List<String> actual = model.stringsListProperty().stream()
+                .map(ConstantsItemModel::labelProperty)
+                .map(StringProperty::getValue)
+                .toList();
+
+        assertEquals(expected, actual);
+    }
+
+    @DisplayName("Check that the list of strings is sorted after resorting it")
+    @Test
+    void testStringsListPropertyResorting() {
+        BibDatabase db = new BibDatabase();
+        BibDatabaseContext context = new BibDatabaseContext(db);
+        DialogService service = mock(DialogService.class);
+        List<String> expected = List.of("ICSE", "TSE");
+
+        ConstantsPropertiesViewModel model = new ConstantsPropertiesViewModel(context, service);
+        var stringsList = model.stringsListProperty();
+        stringsList.add(new ConstantsItemModel("TSE", "Transactions on Software Engineering"));
+        stringsList.add(new ConstantsItemModel("ICSE", "International Conference on Software Engineering"));
+
+        model.resortStrings();
+
+        List<String> actual = model.stringsListProperty().stream()
+                .map(ConstantsItemModel::labelProperty)
+                .map(StringProperty::getValue)
+                .toList();
+
+        assertEquals(expected, actual);
+    }
+}