Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix selection of table sort order #10250

Merged
merged 40 commits into from
Sep 2, 2023
Merged

Fix selection of table sort order #10250

merged 40 commits into from
Sep 2, 2023

Conversation

koppor
Copy link
Member

@koppor koppor commented Aug 30, 2023

Fixes #9869

We also have meta data changes upon save. I added a quick output of these changes. This is far from end user friendly, but better than outputting nothing.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@koppor koppor marked this pull request as draft August 30, 2023 22:57
@github-actions
Copy link
Contributor

Your code currently does not meet JabRef's code guidelines. The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR. Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues.

More information on code quality in JabRef is available at https://devdocs.jabref.org/getting-into-the-code/development-strategy.html.

@koppor koppor added this to the v5.10 milestone Aug 30, 2023
@calixtus
Copy link
Member

There are a lot of architectural changes and especially the call to getTableSaveOrder in JabrefPreferences was on purpose private, since it is a low level call to the backing store as far as I remember. Also the changes to persistentVisualState are suspicious to me. I will take a look at it tomorrow and will try to make suggestions to improve it.

@koppor
Copy link
Member Author

koppor commented Sep 1, 2023

There are a lot of architectural changes and especially the call to getTableSaveOrder in JabrefPreferences was on purpose private, since it is a low level call to the backing store as far as I remember.

I am aware of that. At some place, there needs to be a mapping from the table sort order (UI) to SaveOrder for the logic (which must not access the UI). I interpreted that persistentVisualState together with the prefernces are the necessary bridges to route the UI state to a SaveOrder.

Also the changes to persistentVisualState are suspicious to me.

There should be no "doing" in a constructor, because a constructor is for initializing the class. "Doings" should be at methods. This was done at with that change. -- I know that the class at hand is a kind of helper class, because it's constructed once only and has not any further usage. Maybe, the whole thing should be refactored to a static method. Will do.

koppor and others added 3 commits September 1, 2023 19:31
…fix-table-sort-order

# Conflicts:
#	src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
#	src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java
#	src/main/java/org/jabref/preferences/PreferencesService.java
@koppor koppor marked this pull request as ready for review September 2, 2023 00:00
@koppor koppor modified the milestones: v5.10, 5.11 Sep 2, 2023
@Siedlerchr Siedlerchr added this pull request to the merge queue Sep 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2023

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@koppor koppor removed this pull request from the merge queue due to a manual request Sep 2, 2023
@koppor koppor added this pull request to the merge queue Sep 2, 2023
@koppor koppor mentioned this pull request Sep 2, 2023
6 tasks
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 2, 2023
@koppor koppor added this pull request to the merge queue Sep 2, 2023
@koppor koppor removed this pull request from the merge queue due to a manual request Sep 2, 2023
@koppor koppor added this pull request to the merge queue Sep 2, 2023
Merged via the queue into main with commit d743f52 Sep 2, 2023
14 checks passed
@koppor koppor deleted the fix-table-sort-order branch September 2, 2023 20:53
Siedlerchr added a commit that referenced this pull request Sep 3, 2023
* upstream/main: (737 commits)
  Use OpenRewrite UseStringReplace (#10299)
  Add ReplaceStringBuilderWithString (#10302)
  Apply ShortenFullyQualifiedTypeReferences (#10301)
  Apply StaticMethodNotFinal (#10300)
  Apply exit early pattern (#10297)
  Avoid GH_TOKEN_UPDATE_GRADLE_WRAPPER at refresh-csl-subtrees
  Update snapcraft.yaml
  Fix exception when opening preferences (#10294)
  gnome-42-2204
  use gnome 4.2
  Use gnome
  Update base 22
  Add method for detecting journal abbrev delimiter (#10281)
  Update bug_report.yml
  Fix selection of table sort order (#10250)
  Fix mixup with YAML tags (#10287)
  More guards for upload (#10282)
  Reduced calls to Globals.prefs and Globals.entryTypesManager (#10177)
  Update jmh plugin (#10279)
  Add computer science to protected terms (#10222)
  ...

# Conflicts:
#	build.gradle
#	src/main/java/module-info.java
#	src/main/java/org/jabref/cli/Launcher.java
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
#	src/main/resources/tinylog.properties
Siedlerchr added a commit that referenced this pull request Sep 4, 2023
…ustStore

* upstream/main: (295 commits)
  Use OpenRewrite UseStringReplace (#10299)
  Add ReplaceStringBuilderWithString (#10302)
  Apply ShortenFullyQualifiedTypeReferences (#10301)
  Apply StaticMethodNotFinal (#10300)
  Apply exit early pattern (#10297)
  Avoid GH_TOKEN_UPDATE_GRADLE_WRAPPER at refresh-csl-subtrees
  Update snapcraft.yaml
  Fix exception when opening preferences (#10294)
  gnome-42-2204
  use gnome 4.2
  Use gnome
  Update base 22
  Add method for detecting journal abbrev delimiter (#10281)
  Update bug_report.yml
  Fix selection of table sort order (#10250)
  Fix mixup with YAML tags (#10287)
  More guards for upload (#10282)
  Reduced calls to Globals.prefs and Globals.entryTypesManager (#10177)
  Update jmh plugin (#10279)
  Add computer science to protected terms (#10222)
  ...
@Siedlerchr Siedlerchr mentioned this pull request Jul 10, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entries not reordered after save
3 participants