[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

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.

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.

SihasA · 2025-03-20T13:56:35Z

Just add a new heading below "Saving"

Thank you for the clarification- to ensure I understood correctly, is this what you meant @koppor?

…al preferences

koppor · 2025-03-20T14:33:56Z

Just add a new heading below "Saving"
Thank you for the clarification- to ensure I understood correctly, is this what you meant @koppor?

Yes 😅

…into Happy-Days-Git-Integration-WP3

Refactor postSaveDatabaseAction to properly handle exceptions Add dialog messages to push and pull method

koppor · 2025-03-21T15:05:23Z

I changed branch from gitintegration to main since this seems to be unrelated to the other features (yet). You fan do integration on one of your forks.

koppor · 2025-03-24T09:01:57Z

I merged main with the hope that trag-bot will look into this.

koppor

On a good way IMHO

koppor · 2025-03-24T09:59:47Z

src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java

+                new GitClientHandler(targetPath.getParent(),
+                        dialogService,
+                        preferences.getGitPreferences())
+                        .postSaveDatabaseAction();


Lifecycle is strange - the GitClientHandler should be bound to the BibdatabaseContext (or libraryTab), shouldn't it?

There is also an instance in src/main/java/org/jabref/gui/importer/actions/CheckForVersionControlAction.java. I think, the instance should be shared?

koppor · 2025-03-24T10:01:34Z

src/main/java/org/jabref/gui/importer/actions/CheckForVersionControlAction.java

+        this.gitClientHandler = new GitClientHandler(path.get(),
+                dialogService,
+                preferences.getGitPreferences());


This is a side effect of a simple boolean check. Can't it be done differently?

Maybe bind the GitClienthandler to the BibDatbaseContext?

koppor · 2025-03-24T10:06:36Z

src/main/java/org/jabref/gui/preferences/general/GeneralTab.fxml

+
+    <Label styleClass="sectionHeader" text="%Git"/>
+    <HBox spacing="10">
+        <Label text="Git Username:"/>


Please see two lines above how localiaztion works.

Moreover, no : at the end

koppor · 2025-03-24T10:07:19Z

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

+     * If the directory is a Git repository, performs a pull operation on the current branch.
+     * Will log any errors that occur during the pull operation.
+     */
+


Please remove empty line - JavaDoc is connected to the method directly.

koppor · 2025-03-24T10:07:27Z

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

+        this.dialogService = dialogService;
+        this.stateManager = stateManager;
+    }
+    /**


Please add a space above

koppor · 2025-03-24T10:15:21Z

src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java

+        String gitHubUsername = getString("gitHubUsername", "");
+        String gitHubPasskey = getString("gitHubPasskey", "");


Please read on at org.jabref.logic.preferences.JabRefCliPreferences#getProxyPreferences how JabRef stores passwords.

--> We use com.github.javakeyring.Keyring - and DO NOT STORE PASSWORDS IN PLAIN TEXT

koppor · 2025-03-24T10:17:30Z

src/test/java/org/jabref/logic/git/GitPreferencesTest.java

+
+    @Test
+    void constructorInitialisesValues() {
+        assertThat(gitPreferences.getAutoPushEnabled()).isTrue();


JabRef more uses plain JUnit5.

Please use assertTrue instead of (........isTrue())

koppor · 2025-03-24T10:18:23Z

src/test/java/org/jabref/logic/git/GitClientHandlerTest.java

+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+class GitClientHandlerTest {


This is currently just a skeletton of a test.

Either test with a real git repository (which could be created locally durent the setup phase of this test) - or just remove this test.

--> Work Package 4 could build on this infrastructure

koppor · 2025-03-24T10:20:00Z

src/test/java/org/jabref/gui/importer/actions/CheckForVersionControlActionEndToEndTest.java

+        } catch (IOException e) {
+            throw new AssertionError("Failed to set up test directory", e);
+        }


Do not catch exceptions - jsut add throws clause

koppor · 2025-03-24T10:20:33Z

src/test/java/org/jabref/gui/importer/actions/CheckForVersionControlActionEndToEndTest.java

+    void performAction_WhenGitPullSucceeds_ShouldNotThrowException() {
+        when(databaseContext.getDatabasePath()).thenReturn(Optional.of(gitRepo.resolve("test.bib")));
+
+        action.isActionNecessary(parserResult, dialogService, cliPreferences);


THis code reads strange - it is because of the side effect

At least, you should do assertTrue(....).

…into Happy-Days-Git-Integration-WP3

koppor · 2025-03-25T15:33:42Z

I merged main with the hope that our bots will comment with the most important things to adress - it are JUnit tests failing, isn't it?

trag-bot · 2025-03-25T15:33:58Z

@trag-bot didn't find any issues in the code! ✅✨

jabref-machine · 2025-03-25T15:36:42Z

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.

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

trag-bot bot reviewed Mar 19, 2025

View reviewed changes

Removed line in build.gradle

86edd20

Added a new heading under saving for git related preferences in gener…

14e6d52

…al preferences

arp-23 and others added 4 commits March 20, 2025 14:53

Fix bot comments

0b7db48

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

6ad7f25

…into Happy-Days-Git-Integration-WP3

Merge branch 'JabRef:main' into Happy-Days-Git-Integration-WP3

74398a8

Update GitClientHandler

ee71915

Refactor postSaveDatabaseAction to properly handle exceptions Add dialog messages to push and pull method

koppor changed the base branch from gitintegration to main March 21, 2025 15:04

Add basic GitClientHandlerTest and attempt to fix EndToEndTest

9bf27ad

JabRef deleted a comment from github-actions bot Mar 24, 2025

Merge branch 'main' into Happy-Days-Git-Integration-WP3

7cfb3e4

koppor mentioned this pull request Mar 24, 2025

Display the git synchronisation state on the .bib file tab title #12678

Draft

7 tasks

JabRef deleted a comment from jabref-machine Mar 24, 2025

Merge branch 'main' into Happy-Days-Git-Integration-WP3

6775c8b

koppor requested changes Mar 24, 2025

View reviewed changes

koppor and others added 4 commits March 24, 2025 12:28

Merge branch 'main' into Happy-Days-Git-Integration-WP3

a696800

Merge branch 'main' into Happy-Days-Git-Integration-WP3

f166255

removed combo box from gitpreferences

1b46b89

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

1c47c25

…into Happy-Days-Git-Integration-WP3

koppor mentioned this pull request Mar 25, 2025

Add simple git support #12817

Closed

4 tasks

Merge branch 'main' into Happy-Days-Git-Integration-WP3

8d8273e

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 19, 2025

koppor commented Mar 19, 2025

koppor commented Mar 19, 2025

SihasA commented Mar 20, 2025

koppor commented Mar 20, 2025

koppor commented Mar 21, 2025

koppor commented Mar 24, 2025

koppor left a comment

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor Mar 24, 2025

koppor commented Mar 25, 2025

trag-bot bot commented Mar 25, 2025

jabref-machine commented Mar 25, 2025

		String gitHubUsername = getString("gitHubUsername", "");
		String gitHubPasskey = getString("gitHubPasskey", "");

[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 19, 2025

Choose a reason for hiding this comment

koppor commented Mar 19, 2025

koppor commented Mar 19, 2025

SihasA commented Mar 20, 2025

koppor commented Mar 20, 2025

koppor commented Mar 21, 2025

koppor commented Mar 24, 2025

koppor left a comment

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Mar 25, 2025

trag-bot bot commented Mar 25, 2025

jabref-machine commented Mar 25, 2025