Bug fix in log table view: sort order only applied in search, not presentation

Author: georgweiss

app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/AdvancedSearchViewController.java

@@ -19,7 +19,7 @@
 package org.phoebus.logbook.olog.ui;
 
 import javafx.application.Platform;
-import javafx.beans.property.SimpleBooleanProperty;
+import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.property.SimpleStringProperty;
 import javafx.beans.value.ChangeListener;
 import javafx.beans.value.ObservableValue;
@@ -36,15 +36,16 @@
 import javafx.scene.control.RadioButton;
 import javafx.scene.control.TextField;
 import javafx.scene.control.ToggleButton;
+import javafx.scene.control.ToggleGroup;
 import javafx.scene.image.ImageView;
 import javafx.scene.input.KeyCode;
 import javafx.scene.input.KeyEvent;
 import javafx.scene.layout.AnchorPane;
 import javafx.scene.layout.GridPane;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.VBox;
-import org.phoebus.logbook.LogEntryLevel;
 import org.phoebus.logbook.LogClient;
+import org.phoebus.logbook.LogEntryLevel;
 import org.phoebus.logbook.Logbook;
 import org.phoebus.logbook.Tag;
 import org.phoebus.ui.dialog.ListSelectionPopOver;
@@ -132,7 +133,8 @@ public class AdvancedSearchViewController {
 
     private final SearchParameters searchParameters;
 
-    private final SimpleBooleanProperty sortAscending = new SimpleBooleanProperty(false);
+    private final SimpleObjectProperty<SortOrder> sortOrderProperty =
+            new SimpleObjectProperty<>(SortOrder.DESCENDING);
     private final ObservableList<LevelSelection> levelSelections = FXCollections.observableArrayList();
     private final SimpleStringProperty selectedLevelsString = new SimpleStringProperty();
     private final List<String> levelsList = new ArrayList<>();
@@ -181,7 +183,6 @@ public void initialize() {
         endTime.textProperty().bindBidirectional(this.searchParameters.endTimeProperty());
         endTime.setOnKeyReleased(this::searchOnEnter);
         searchParameters.addListener((observable, oldValue, newValue) -> updateControls(newValue));
-        sortAscending.addListener(searchOnSortChange);
 
         attachmentTypes.textProperty().bindBidirectional(this.searchParameters.attachmentsProperty());
         attachmentTypes.setOnKeyReleased(this::searchOnEnter);
@@ -330,14 +331,16 @@ public void initialize() {
             levelsContextMenu.getItems().add(levelSelectionMenuItem);
         });
 
-        sortAscending.addListener((observable, oldValue, newValue) -> {
-            sortDescRadioButton.selectedProperty().set(!newValue);
-            sortAscRadioButton.selectedProperty().set(newValue);
-        });
+        sortOrderProperty.addListener(searchOnSortChange);
 
-        sortDescRadioButton.setOnAction(ae -> sortAscending.set(false));
+        sortDescRadioButton.setUserData(SortOrder.DESCENDING);
+        sortAscRadioButton.setUserData(SortOrder.ASCENDING);
 
-        sortAscRadioButton.setOnAction(ae -> sortAscending.set(true));
+        ToggleGroup toggleGroup = new ToggleGroup();
+        toggleGroup.getToggles().addAll(sortDescRadioButton, sortAscRadioButton);
+        toggleGroup.selectToggle(sortDescRadioButton);
+        toggleGroup.selectedToggleProperty().addListener((obs, o, n) ->
+                sortOrderProperty.set((SortOrder) n.getUserData()));
 
         gridPane.setOnMouseClicked(e -> levelsToggleButton.setSelected(false));
 
@@ -428,8 +431,8 @@ protected List<String> getValidatedLevelsSelection(String levels) {
         return levelsFromQueryString.stream().filter(levelsList::contains).collect(Collectors.toList());
     }
 
-    public SimpleBooleanProperty getSortAscending() {
-        return sortAscending;
+    public boolean getSortAscending() {
+        return sortOrderProperty.get().equals(SortOrder.ASCENDING);
     }
 
     private void searchOnEnter(KeyEvent e) {
@@ -440,7 +443,8 @@ private void searchOnEnter(KeyEvent e) {
 
     private final ChangeListener<? super String> searchOnTextChange = (options, oldValue, newValue) -> searchCallback.run();
 
-    private final ChangeListener<? super Boolean> searchOnSortChange = (options, oldValue, newValue) -> searchCallback.run();
+    private final ChangeListener<? super SortOrder> searchOnSortChange =
+            (options, oldValue, newValue) -> searchCallback.run();
 
     private void setSelectedLevelsString() {
         selectedLevelsString.set(levelSelections.stream().filter(LevelSelection::isSelected)
@@ -498,4 +502,8 @@ public CheckBox getCheckBox() {
         }
     }
 
+    private enum SortOrder {
+        DESCENDING,
+        ASCENDING
+    }
 }

app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/LogEntryTableViewController.java

@@ -378,7 +378,7 @@ public void search() {
 
         Map<String, String> params =
                 LogbookQueryUtil.parseHumanReadableQueryString(ologQueryManager.getOrAddQuery(queryString).getQuery());
-        params.put("sort", advancedSearchViewController.getSortAscending().get() ? "up" : "down");
+        params.put("sort", advancedSearchViewController.getSortAscending() ? "up" : "down");
         params.put("from", Integer.toString(pagination.getCurrentPageIndex() * pageSizeProperty.get()));
         params.put("size", Integer.toString(pageSizeProperty.get()));
 
@@ -444,7 +444,8 @@ private void refresh() {
                 List<TableViewListItem> selectedLogEntries = new ArrayList<>(tableView.getSelectionModel().getSelectedItems());
 
                 List<LogEntry> logEntries = searchResult.getLogs();
-                logEntries.sort((o1, o2) -> -(o1.getCreatedDate().compareTo(o2.getCreatedDate())));
+                logEntries.sort((o1, o2) -> advancedSearchViewController.getSortAscending() ? o1.getCreatedDate().compareTo(o2.getCreatedDate()) :
+                        -(o1.getCreatedDate().compareTo(o2.getCreatedDate())));
 
                 boolean showDetailsBoolean = showDetails.get();
                 var logs = logEntries.stream().map(le -> new TableViewListItem(le, showDetailsBoolean)).toList();