[WIP] Happy days git integration wp3 #12773

11raphael · 2025-03-17T19:32:21Z

This PR addresses Working Package 3 for #12350

We shall go through org.jabref.model.database.BibDatabaseContext to execute on-save functionality.

We have decided to instead execute on-save functionality as a single line method call in org/jabref/gui/exporter/SaveDatabaseAction.java

“With regards to implementing preferences- we shall proceed with adding preferences to the current 'Saving' under the "General" tab as suggested. If the segment gets cluttered we shall opt for a new "Saving" tab.”

We have currently added git preferences under “Saving” in “General”. Is this okay? And if not should we proceed to making a new segment for it in preferences?

Small architecture concern: We opted to pass preferences information into GitHandler through method updateCredentials but are unsure if this is good practice.

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[x ] Tests created for changes (if applicable)
[x ] Manually tested changed features in running JabRef (always required)
[ x] Screenshots added in PR description (if change is visible to the user)
[ x] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
[ x] 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.

…and to pull latest changes if so

Update GitHandler constructor to optionally not create a repository if none exists

…erence exists

…gui.importer.actions

…d-pull' into library-under-version-control-and-pull # Conflicts: # src/test/java/org/jabref/gui/importer/actions/CheckForVersionControlActionTest.java

removed the logic that required the download of additional libraries/dependencies

…into Happy-Days-Git-Integration-WP3

…ion-control-and-pull

…eneral for autopush - to be cleaned up

…d-pull' into library-under-version-control-and-pull # Conflicts: # src/main/java/org/jabref/logic/preferences/CliPreferences.java

…-Git-Integration-WP3

…into Happy-Days-Git-Integration-WP3

- Checkstyle issue in GitPreferences.java fixed. (Incorrect import order) - Broad catch blocks in GitPullAction and GitPushAction fixed - Also fixed incorrect logger usage.

trag-bot · 2025-03-18T15:45:29Z

src/main/java/org/jabref/gui/shared/GitPullAction.java

+        BibDatabaseContext databaseContext = stateManager.getActiveDatabase().get();
+        if (stateManager.getActiveDatabase().isEmpty()) {
+            return;
+        }


Potential NPE as get() is called before isEmpty() check. The code retrieves the database before checking if it exists, violating the fail-fast principle and risking exceptions.

Still valid comment

trag-bot · 2025-03-18T15:45:56Z

src/main/java/org/jabref/gui/shared/GitPushAction.java

+                dialogService.showErrorDialogAndWait(e);
+            }
+        } else {
+            LOGGER.info("Not a git repository at path: {}", path.get());


The log message directly calls path.get() without checking if the value is present, which could lead to NoSuchElementException. The Optional should be handled properly.

As per intelliJ's response when trying to set a condition, it can be said that there will always be a path and hence this comment is irrelevant?

Just log path, will appear properly. Try it out!

Thank you that works!

- fixed incorrect order of statements for GitPullAction - removed .get() for path in LOGGER statements - checkstyle error fixed - fixed used of a hardcoded path instead of leveraging JUnit's @tempdir annotation.

…into Happy-Days-Git-Integration-WP3

… logic inside a complex if condition. Should check conditions individually and return early if any fails."

trag-bot · 2025-03-19T13:18:58Z

src/main/java/org/jabref/logic/git/GitClientHandler.java

+                preferences.getAutoPushEnabled()) {
+            this.updateCredentials(preferences);
+            try {
+                this.createCommitOnCurrentBranch("Automatic update via JabRef", false);


Boolean parameter in public method should be avoided. Consider creating separate methods with meaningful names instead of using a boolean flag.

This function was produced for the previous SLR functionality, so we will avoid changing it.

koppor · 2025-03-19T14:08:12Z

build.gradle

@@ -160,6 +160,7 @@ dependencies {
    // Include all jar-files in the 'lib' folder as dependencies
    implementation fileTree(dir: 'lib', includes: ['*.jar'])

+


Does an AI bot create these empty lines?

I would recommend to review a commit using git gui!

My accident actually! A member of my team had accidentally added a few dependencies and removed some when trying to add the git functionality to the main menu (He wanted to add an icon next to "Git"). I was removing these dependencies manually and left a line. Will fix that and review the changes properly next time, thank you.

koppor

Quick comment; I am in the road currently

- Localisation tests were failing. Added keys yo JabRef_en.properties. - Bot mentioned issues with logger, these have been resolved.

…t fix. - Removed CheckForVersionControlActionTest.java and working on a replacement. - fixed properties file issue

…into Happy-Days-Git-Integration-WP3

trag-bot · 2025-03-19T15:51:39Z

src/main/java/org/jabref/logic/git/GitClientHandler.java

+    public void postSaveDatabaseAction(GitPreferences preferences) {
+        if (this.isGitRepository() &&
+                preferences.getAutoPushMode() == AutoPushMode.ON_SAVE &&
+                preferences.getAutoPushEnabled()) {


The method should validate the preferences parameter for null before accessing its methods. Current implementation may throw NullPointerException if preferences is null.

I am going to delete the enum in its entirety as the combo-box is no longer necessary for the scope of this PR. Will make the adjustments as necessary.

…into Happy-Days-Git-Integration-WP3

trag-bot · 2025-03-19T16:27:35Z

src/main/java/org/jabref/gui/shared/GitPullAction.java

+    }
+
+    @Override
+    public void execute() {


The method lacks JavaDoc despite being a public method that performs complex Git operations. Complex public methods should have comprehensive documentation.

trag-bot · 2025-03-19T16:29:04Z

src/main/java/org/jabref/logic/git/GitClientHandler.java

+                   .setRef(commit.toString())
+                   .call();
+            } catch (GitAPIException e) {
+                LOGGER.error("Failed to rever to commit");


Typo in error message ('rever' instead of 'revert') and the exception 'e' is not included in the log message, losing valuable debugging information

trag-bot · 2025-03-19T16:29:24Z

src/main/java/org/jabref/logic/git/GitClientHandler.java

+                   .setRebase(true)
+                   .call();
+            } catch (GitAPIException e) {
+                LOGGER.error("Failed to pull and rebase");


Exception 'e' is not included in the log message, which loses stack trace and error details needed for debugging

trag-bot · 2025-03-19T16:29:26Z

src/main/java/org/jabref/logic/git/GitClientHandler.java

+            try {
+                this.pullAndRebaseOnCurrentBranch();
+                dialogService.notify(Localization.lang("Successfully pulled"));
+            } catch (IOException e) {


Try block covers multiple statements including notification, violating the principle of minimal try block coverage

github-actions · 2025-03-19T16:32:22Z

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

koppor · 2025-03-19T20:13:26Z

We have currently added git preferences under “Saving” in “General”. Is this okay? And if not should we proceed to making a new segment for it in preferences?

I think, we "discussed" this for 10 secons in one of our meetings. This was too fast.

If possible Move it below "General" with "Saving" as heading.

(Hint to me and you for a next project: More time for discussions / issue refinement. Write and check minutes. Wirte minitues in a shared manner - e.g., Etherpad (https://pad.riseup.net/) or Microsoft Word online on OneDrive)

koppor · 2025-03-19T20:15:40Z

If possible Move it below "General" with "Saving" as heading.

Update. Seems to be hard. Just add a new heading below "Saving". - When it comes to integrate WP4, there surely will be a new tab "Git" or "Saving". Due to time constraints, I think, this integration is out of scope of your project.

RapidShotzz and others added 30 commits February 26, 2025 13:09

added methods to and boolean to see if file is under version control …

01dec74

…and to pull latest changes if so

Add CheckForVersionControlAction

eb1b9a8

Update GitHandler constructor to optionally not create a repository if none exists

Added unit tests for - CheckForVersionControlAction

c761db5

Added unit tests for - CheckForVersionControlAction

2c9dad4

added functionality for tagging as versioned and pulling only if pref…

ed6eb4a

…erence exists

moved test file and amended shouldAutoPull method to test.org.jabref.…

9cbdd76

…gui.importer.actions

Remove RuntimeException from CheckForVersionControlAction

62defc4

Remove redundant methods from GitHandler

bbc6b38

modified the test cases for CheckForVersionControlAction

69e0f9a

Merge remote-tracking branch 'origin/library-under-version-control-an…

d578bd5

…d-pull' into library-under-version-control-and-pull # Conflicts: # src/test/java/org/jabref/gui/importer/actions/CheckForVersionControlActionTest.java

modified the test cases for CheckForVersionControlAction

cf32088

modified the test cases for CheckForVersionControlAction

c670625

modified the test cases for CheckForVersionControlAction

9026eb9

modified the test cases for CheckForVersionControlAction

40cdd64

removed the logic that required the download of additional libraries/dependencies

Remove preference in CheckForVersionControlAction

8db799c

Add git preferences

af0c3c8

Add GitPreferences to CliPreferences

1f2de0a

added autoPushEnabled attributes and set methods

cfd46db

modified tab view model to handle auto push

6ecf1a8

Add getGitPreferences to JabRefCliPreferences

a0e2dc7

Merge remote-tracking branch 'origin/Happy-Days-Git-Integration-WP3' …

c8be361

…into Happy-Days-Git-Integration-WP3

Merge branch 'Happy-Days-Git-Integration-WP3' into library-under-vers…

5127bca

…ion-control-and-pull

Remove redundant shouldAutoPull from CliPreferences

2bf349d

Added a rudimentary first draft preference selector under Saving in g…

bff9857

…eneral for autopush - to be cleaned up

Merge remote-tracking branch 'origin/library-under-version-control-an…

c197264

…d-pull' into library-under-version-control-and-pull # Conflicts: # src/main/java/org/jabref/logic/preferences/CliPreferences.java

removed duplicate boolean

f7c21c4

Cleaned up UI - "Auto push" Combo Box no longer is in all Caps

980c748

Added "Git" submenu with "Git Pull" and "Git Push" options

caf405b

constructed get git preferences

4ed64cd

Merge branch 'library-under-version-control-and-pull' into Happy-Days…

45c453e

…-Git-Integration-WP3

11raphael and others added 2 commits March 18, 2025 15:22

Merge remote-tracking branch 'origin/Happy-Days-Git-Integration-WP3' …

dcc5a06

…into Happy-Days-Git-Integration-WP3

Fixed CheckStyleIssues and Bot Issue

b11f155

- Checkstyle issue in GitPreferences.java fixed. (Incorrect import order) - Broad catch blocks in GitPullAction and GitPushAction fixed - Also fixed incorrect logger usage.

trag-bot bot reviewed Mar 18, 2025

View reviewed changes

SihasA and others added 7 commits March 18, 2025 16:20

Fixing Bot Comments

854af5c

- fixed incorrect order of statements for GitPullAction - removed .get() for path in LOGGER statements - checkstyle error fixed - fixed used of a hardcoded path instead of leveraging JUnit's @tempdir annotation.

Reverted changes to module-info.java and build.gradle.

95197c2

Refactor git client functionality into GitClientHandler

51e6fd6

Merge remote-tracking branch 'origin/Happy-Days-Git-Integration-WP3' …

f1fe7f5

…into Happy-Days-Git-Integration-WP3

Corrected issue - "Method violates fail-fast principle by nesting all…

e3fc84e

… logic inside a complex if condition. Should check conditions individually and return early if any fails."

Corrected issue - "Method violates fail-fast principle by nesting all…

3070087

… logic inside a complex if condition. Should check conditions individually and return early if any fails."

Corrected issue - "Method violates fail-fast principle by nesting all…

4e1c577

… logic inside a complex if condition. Should check conditions individually and return early if any fails."

trag-bot bot reviewed Mar 19, 2025

View reviewed changes

Ran rewriteRun and fixed bot comments

5b278aa

koppor reviewed Mar 19, 2025

View reviewed changes

SihasA and others added 4 commits March 19, 2025 14:32

Added localisation and fixed logger issues.

c305e53

- Localisation tests were failing. Added keys yo JabRef_en.properties. - Bot mentioned issues with logger, these have been resolved.

Fixed one failing test case and removed problematic test + Bot commen…

9b0bf8f

…t fix. - Removed CheckForVersionControlActionTest.java and working on a replacement. - fixed properties file issue

Add test cases for Git repository detection

493775b

Merge remote-tracking branch 'origin/Happy-Days-Git-Integration-WP3' …

92df862

…into Happy-Days-Git-Integration-WP3

trag-bot bot reviewed Mar 19, 2025

View reviewed changes

11raphael added 3 commits March 19, 2025 16:23

Refactor postSaveDatabaseAction and GitClientHandler

0c0a094

Merge remote-tracking branch 'origin/Happy-Days-Git-Integration-WP3' …

6e731cf

…into Happy-Days-Git-Integration-WP3

Update postSaveDatabaseAction JavaDoc

355ff89

trag-bot bot reviewed Mar 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Happy days git integration wp3 #12773

[WIP] Happy days git integration wp3 #12773

11raphael commented Mar 17, 2025

trag-bot bot Mar 18, 2025

koppor Mar 18, 2025

trag-bot bot Mar 18, 2025

SihasA Mar 18, 2025

koppor Mar 18, 2025

SihasA Mar 18, 2025

trag-bot bot Mar 19, 2025

SihasA Mar 19, 2025

koppor Mar 19, 2025

SihasA Mar 19, 2025

koppor left a comment

trag-bot bot Mar 19, 2025

SihasA Mar 19, 2025

trag-bot bot Mar 19, 2025

trag-bot bot Mar 19, 2025

trag-bot bot Mar 19, 2025

trag-bot bot Mar 19, 2025

github-actions bot commented Mar 19, 2025

koppor commented Mar 19, 2025

koppor commented Mar 19, 2025

		@@ -160,6 +160,7 @@ dependencies {
		// Include all jar-files in the 'lib' folder as dependencies
		implementation fileTree(dir: 'lib', includes: ['*.jar'])

[WIP] Happy days git integration wp3 #12773

Are you sure you want to change the base?

[WIP] Happy days git integration wp3 #12773

Conversation

11raphael commented Mar 17, 2025

Mandatory checks

trag-bot bot Mar 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Mar 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Mar 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

trag-bot bot Mar 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trag-bot bot Mar 19, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 19, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 19, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 19, 2025

Choose a reason for hiding this comment

github-actions bot commented Mar 19, 2025

koppor commented Mar 19, 2025

koppor commented Mar 19, 2025