From 4631038407766139636967750705f453780dceaf Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 26 Mar 2021 20:46:31 +0100 Subject: [PATCH 1/8] Fix column sort order gets overwritten Fixes #7524 --- .../gui/maintable/ColumnPreferences.java | 18 +++++++++-- .../gui/maintable/MainTableColumnModel.java | 7 ++++ .../PersistenceVisualStateTable.java | 32 ++++++++++++------- .../jabref/preferences/JabRefPreferences.java | 9 +++--- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java b/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java index d79b3a6857b..849d01bf684 100644 --- a/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java +++ b/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java @@ -1,5 +1,6 @@ package org.jabref.gui.maintable; +import java.util.ArrayList; import java.util.List; public class ColumnPreferences { @@ -7,8 +8,8 @@ public class ColumnPreferences { public static final double DEFAULT_COLUMN_WIDTH = 100; public static final double ICON_COLUMN_WIDTH = 16 + 12; // add some additional space to improve appearance - private final List columns; - private final List columnSortOrder; + private List columns; + private List columnSortOrder = new ArrayList<>(); public ColumnPreferences(List columns, List columnSortOrder) { this.columns = columns; @@ -22,4 +23,17 @@ public List getColumns() { public List getColumnSortOrder() { return columnSortOrder; } + + public void setColumns(List columns) { + this.columns = columns; + } + + public void setColumnSorOrder(List columnSortOrder) { + this.columnSortOrder = columnSortOrder; + } + + @Override + public String toString() { + return "ColumnPreferences [columns=" + columns + ", columnSortOrder=" + columnSortOrder + "]"; + } } diff --git a/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java b/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java index 0edf1dbde17..f8345048a7c 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java +++ b/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java @@ -213,4 +213,11 @@ public static MainTableColumnModel parse(String rawColumnName) { return new MainTableColumnModel(type, qualifier); } + + @Override + public String toString() { + return "MainTableColumnModel [typeProperty=" + typeProperty + ", qualifierProperty=" + qualifierProperty + ", widthProperty=" + widthProperty + ", sortTypeProperty=" + sortTypeProperty + "]"; + } + + } diff --git a/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java b/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java index 6b695be291d..c409814d4a1 100644 --- a/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java +++ b/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java @@ -20,27 +20,35 @@ public PersistenceVisualStateTable(final MainTable mainTable, PreferencesService this.preferences = preferences; mainTable.getColumns().addListener((InvalidationListener) obs -> updateColumnPreferences()); - mainTable.getSortOrder().addListener((InvalidationListener) obs -> updateColumnPreferences()); + mainTable.getSortOrder().addListener((InvalidationListener) obs -> updateColSortOrder()); // As we store the ColumnModels of the MainTable, we need to add the listener to the ColumnModel properties, // since the value is bound to the model after the listener to the column itself is called. - mainTable.getColumns().forEach(col -> - ((MainTableColumn) col).getModel().widthProperty().addListener(obs -> updateColumnPreferences())); - mainTable.getColumns().forEach(col -> - ((MainTableColumn) col).getModel().sortTypeProperty().addListener(obs -> updateColumnPreferences())); + mainTable.getColumns().forEach(col -> ((MainTableColumn) col).getModel().widthProperty().addListener(obs -> updateColumnPreferences())); + mainTable.getColumns().forEach(col -> ((MainTableColumn) col).getModel().sortTypeProperty().addListener(obs -> updateColumnPreferences())); + } + + private void updateColSortOrder() { + ColumnPreferences prefs = preferences.getColumnPreferences(); + + var sortOrder = mainTable.getSortOrder().stream() + .map(column -> ((MainTableColumn) column).getModel()) + .collect(Collectors.toList()); + + prefs.setColumnSorOrder(sortOrder); + preferences.storeColumnPreferences(prefs); } /** * Store shown columns, their width and their sortType in preferences. */ private void updateColumnPreferences() { - preferences.storeColumnPreferences(new ColumnPreferences( - mainTable.getColumns().stream() - .map(column -> ((MainTableColumn) column).getModel()) - .collect(Collectors.toList()), - mainTable.getSortOrder().stream() + + ColumnPreferences prefs = preferences.getColumnPreferences(); + prefs.setColumns(mainTable.getColumns().stream() .map(column -> ((MainTableColumn) column).getModel()) - .collect(Collectors.toList()) - )); + .collect(Collectors.toList())); + preferences.storeColumnPreferences(prefs); + } } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index cec0984686a..217b147869c 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1934,10 +1934,11 @@ public void storeColumnPreferences(ColumnPreferences columnPreferences) { columnPreferences.getColumns().forEach(column -> columnSortTypesInOrder.add(column.sortTypeProperty().getValue().toString())); putStringList(COLUMN_SORT_TYPES, columnSortTypesInOrder); - putStringList(COLUMN_SORT_ORDER, columnPreferences - .getColumnSortOrder().stream() - .map(MainTableColumnModel::getName) - .collect(Collectors.toList())); + var columnSortOrder = columnPreferences + .getColumnSortOrder().stream() + .map(MainTableColumnModel::getName) + .collect(Collectors.toList()); + putStringList(COLUMN_SORT_ORDER, columnSortOrder); // Update cache mainTableColumns = columnPreferences.getColumns(); From 1ed1b98d2a0c30e54b62190cd22589c796ac6919 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 26 Mar 2021 20:52:51 +0100 Subject: [PATCH 2/8] checkstyle and changelog --- CHANGELOG.md | 1 + .../java/org/jabref/gui/maintable/MainTableColumnModel.java | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd67967d3c1..79c908c3771 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve ### Fixed +- We fixed an issue where the table column sort order was not properly stored and resulted in unsorted eports [#7524](https://github.com/JabRef/jabref/issues/7524) - We fixed an issue preventing to connect to a shared database. [#7570](https://github.com/JabRef/jabref/pull/7570) - We fixed an issue preventing files from being dragged & dropped into an empty library. [#6851](https://github.com/JabRef/jabref/issues/6851) - We fixed an issue where double-click onto PDF in file list under the 'General' tab section should just open the file. [#7465](https://github.com/JabRef/jabref/issues/7465) diff --git a/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java b/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java index f8345048a7c..9407e436f64 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java +++ b/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java @@ -218,6 +218,4 @@ public static MainTableColumnModel parse(String rawColumnName) { public String toString() { return "MainTableColumnModel [typeProperty=" + typeProperty + ", qualifierProperty=" + qualifierProperty + ", widthProperty=" + widthProperty + ", sortTypeProperty=" + sortTypeProperty + "]"; } - - } From 5eb56b9548d9473b029aeb6245fc0497a90681c4 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Sun, 28 Mar 2021 23:54:45 +0200 Subject: [PATCH 3/8] Clean up --- .../gui/maintable/ColumnPreferences.java | 18 +------- .../PersistenceVisualStateTable.java | 43 ++++++++++--------- 2 files changed, 24 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java b/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java index 849d01bf684..d79b3a6857b 100644 --- a/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java +++ b/src/main/java/org/jabref/gui/maintable/ColumnPreferences.java @@ -1,6 +1,5 @@ package org.jabref.gui.maintable; -import java.util.ArrayList; import java.util.List; public class ColumnPreferences { @@ -8,8 +7,8 @@ public class ColumnPreferences { public static final double DEFAULT_COLUMN_WIDTH = 100; public static final double ICON_COLUMN_WIDTH = 16 + 12; // add some additional space to improve appearance - private List columns; - private List columnSortOrder = new ArrayList<>(); + private final List columns; + private final List columnSortOrder; public ColumnPreferences(List columns, List columnSortOrder) { this.columns = columns; @@ -23,17 +22,4 @@ public List getColumns() { public List getColumnSortOrder() { return columnSortOrder; } - - public void setColumns(List columns) { - this.columns = columns; - } - - public void setColumnSorOrder(List columnSortOrder) { - this.columnSortOrder = columnSortOrder; - } - - @Override - public String toString() { - return "ColumnPreferences [columns=" + columns + ", columnSortOrder=" + columnSortOrder + "]"; - } } diff --git a/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java b/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java index c409814d4a1..092f5c02820 100644 --- a/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java +++ b/src/main/java/org/jabref/gui/maintable/PersistenceVisualStateTable.java @@ -19,36 +19,37 @@ public PersistenceVisualStateTable(final MainTable mainTable, PreferencesService this.mainTable = mainTable; this.preferences = preferences; - mainTable.getColumns().addListener((InvalidationListener) obs -> updateColumnPreferences()); - mainTable.getSortOrder().addListener((InvalidationListener) obs -> updateColSortOrder()); + mainTable.getColumns().addListener((InvalidationListener) obs -> updateColumns()); + mainTable.getSortOrder().addListener((InvalidationListener) obs -> updateSortOrder()); // As we store the ColumnModels of the MainTable, we need to add the listener to the ColumnModel properties, // since the value is bound to the model after the listener to the column itself is called. - mainTable.getColumns().forEach(col -> ((MainTableColumn) col).getModel().widthProperty().addListener(obs -> updateColumnPreferences())); - mainTable.getColumns().forEach(col -> ((MainTableColumn) col).getModel().sortTypeProperty().addListener(obs -> updateColumnPreferences())); + mainTable.getColumns().forEach(col -> + ((MainTableColumn) col).getModel().widthProperty().addListener(obs -> updateColumns())); + mainTable.getColumns().forEach(col -> + ((MainTableColumn) col).getModel().sortTypeProperty().addListener(obs -> updateColumns())); } - private void updateColSortOrder() { - ColumnPreferences prefs = preferences.getColumnPreferences(); - - var sortOrder = mainTable.getSortOrder().stream() - .map(column -> ((MainTableColumn) column).getModel()) - .collect(Collectors.toList()); - - prefs.setColumnSorOrder(sortOrder); - preferences.storeColumnPreferences(prefs); + /** + * Stores shown columns, their width and their sortType in preferences. + */ + private void updateColumns() { + preferences.storeColumnPreferences(new ColumnPreferences( + mainTable.getColumns().stream() + .map(column -> ((MainTableColumn) column).getModel()) + .collect(Collectors.toList()), + preferences.getColumnPreferences().getColumnSortOrder())); } /** - * Store shown columns, their width and their sortType in preferences. + * Stores the SortOrder of the the Table in the preferences. Cannot be combined with updateColumns, because JavaFX + * would provide just an empty list for the sort order on other changes. */ - private void updateColumnPreferences() { - - ColumnPreferences prefs = preferences.getColumnPreferences(); - prefs.setColumns(mainTable.getColumns().stream() + private void updateSortOrder() { + preferences.storeColumnPreferences(new ColumnPreferences( + preferences.getColumnPreferences().getColumns(), + mainTable.getSortOrder().stream() .map(column -> ((MainTableColumn) column).getModel()) - .collect(Collectors.toList())); - preferences.storeColumnPreferences(prefs); - + .collect(Collectors.toList()))); } } From e02f2497e26ed62dfa58642ebd29dc353257c754 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Mon, 29 Mar 2021 00:03:46 +0200 Subject: [PATCH 4/8] Clean up --- .../org/jabref/gui/maintable/MainTableColumnModel.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java b/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java index 9407e436f64..72890141023 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java +++ b/src/main/java/org/jabref/gui/maintable/MainTableColumnModel.java @@ -40,8 +40,8 @@ public enum Type { public static final EnumSet ICON_COLUMNS = EnumSet.of(EXTRAFILE, FILES, GROUPS, LINKED_IDENTIFIER); - private String name; - private String displayName; + private final String name; + private final String displayName; Type(String name) { this.name = name; @@ -213,9 +213,4 @@ public static MainTableColumnModel parse(String rawColumnName) { return new MainTableColumnModel(type, qualifier); } - - @Override - public String toString() { - return "MainTableColumnModel [typeProperty=" + typeProperty + ", qualifierProperty=" + qualifierProperty + ", widthProperty=" + widthProperty + ", sortTypeProperty=" + sortTypeProperty + "]"; - } } From 5dd33db97844a086cd19e9a6fe6fc92252c63c69 Mon Sep 17 00:00:00 2001 From: Carl Christian Snethlage Date: Mon, 29 Mar 2021 00:08:52 +0200 Subject: [PATCH 5/8] Clean up --- .../java/org/jabref/preferences/JabRefPreferences.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 217b147869c..cec0984686a 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1934,11 +1934,10 @@ public void storeColumnPreferences(ColumnPreferences columnPreferences) { columnPreferences.getColumns().forEach(column -> columnSortTypesInOrder.add(column.sortTypeProperty().getValue().toString())); putStringList(COLUMN_SORT_TYPES, columnSortTypesInOrder); - var columnSortOrder = columnPreferences - .getColumnSortOrder().stream() - .map(MainTableColumnModel::getName) - .collect(Collectors.toList()); - putStringList(COLUMN_SORT_ORDER, columnSortOrder); + putStringList(COLUMN_SORT_ORDER, columnPreferences + .getColumnSortOrder().stream() + .map(MainTableColumnModel::getName) + .collect(Collectors.toList())); // Update cache mainTableColumns = columnPreferences.getColumns(); From 3b0e0ac343242a69e69dc6b9b88944420497d5cc Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Mon, 29 Mar 2021 18:01:50 +0200 Subject: [PATCH 6/8] Convert table column sort type to boolean to align with metadata --- .../java/org/jabref/model/metadata/SaveOrderConfig.java | 6 ++++++ .../java/org/jabref/preferences/JabRefPreferences.java | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java b/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java index c2590e398e2..912592bf5e5 100644 --- a/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java +++ b/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java @@ -135,7 +135,13 @@ public static class SortCriterion { public boolean descending; + /** + * + * @param field The field + * @param descending Must be a boolean value as string, e.g. "true", "false" + */ public SortCriterion(Field field, String descending) { + this.field = field; this.descending = Boolean.parseBoolean(descending); } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index cec0984686a..cf427c697ac 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1152,9 +1152,12 @@ private SaveOrderConfig loadTableSaveOrder() { updateMainTableColumns(); List sortOrder = createMainTableColumnSortOrder(); - sortOrder.forEach(column -> config.getSortCriteria().add(new SaveOrderConfig.SortCriterion( - FieldFactory.parseField(column.getQualifier()), - column.getSortType().toString()))); + for (var column : sortOrder) { + boolean descending = column.getSortType() == SortType.DESCENDING ? true : false; + config.getSortCriteria().add(new SaveOrderConfig.SortCriterion( + FieldFactory.parseField(column.getQualifier()), + descending)); + } return config; } From f1c253e94094e68fece2a1f922c6b670cc89abeb Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Mon, 29 Mar 2021 18:04:45 +0200 Subject: [PATCH 7/8] remove empty line --- src/main/java/org/jabref/model/metadata/SaveOrderConfig.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java b/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java index 912592bf5e5..a444e12a5c0 100644 --- a/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java +++ b/src/main/java/org/jabref/model/metadata/SaveOrderConfig.java @@ -141,7 +141,6 @@ public static class SortCriterion { * @param descending Must be a boolean value as string, e.g. "true", "false" */ public SortCriterion(Field field, String descending) { - this.field = field; this.descending = Boolean.parseBoolean(descending); } From 3704771646d7d56e1dc1a269d110f1c4a0383f7a Mon Sep 17 00:00:00 2001 From: Christoph Date: Mon, 29 Mar 2021 20:48:18 +0200 Subject: [PATCH 8/8] Update src/main/java/org/jabref/preferences/JabRefPreferences.java Co-authored-by: Oliver Kopp --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index cf427c697ac..c146effc262 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1153,7 +1153,7 @@ private SaveOrderConfig loadTableSaveOrder() { List sortOrder = createMainTableColumnSortOrder(); for (var column : sortOrder) { - boolean descending = column.getSortType() == SortType.DESCENDING ? true : false; + boolean descending = (column.getSortType() == SortType.DESCENDING); config.getSortCriteria().add(new SaveOrderConfig.SortCriterion( FieldFactory.parseField(column.getQualifier()), descending));