Fix: Usage of "Copy to" option preference

[priyanshu16095]

Fixes #12500

This fix ensures that the "Copy to" option follows the chosen preference correctly.

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)
• 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.

Recording.2025-02-15.mp4

[github-actions]

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.

[subhramit]

I didn't go through this thoroughly before so trying to gather context:

jabref/src/main/java/org/jabref/gui/edit/CopyTo.java

Lines 48 to 64 in 19df622

	public void execute() {
	// we need to operate on a copy otherwise we might get ConcurrentModification issues
	List<BibEntry> selectedEntries = stateManager.getSelectedEntries().stream().toList();
	boolean includeCrossReferences = false;
	boolean showDialogBox = copyToPreferences.getShouldAskForIncludingCrossReferences();
	for (BibEntry bibEntry : selectedEntries) {
	if (bibEntry.hasField(StandardField.CROSSREF) && showDialogBox) {
	includeCrossReferences = askForCrossReferencedEntries();
	copyToPreferences.setShouldIncludeCrossReferences(includeCrossReferences);
	}
	}
	if (includeCrossReferences) {
	copyEntriesWithCrossRef(selectedEntries, targetDatabaseContext);
	} else {

Correct me if I'm wrong, the preference is independent of entries, and if the preference is enabled, we should check for cross references in entries. Why then are we setting the preference for each entry with a cross reference (with each iteration)?

[subhramit]

I'm having a bit of trouble understanding this due to the naming.
What does the field includeCrossReferences do?
And why is it's state different from copyToPreferences.getShouldIncludeCrossReferences()?

[priyanshu16095]

The state of includeCrossReferences is different because every time the execute() method is called, it initializes to false.
It updates only when this code executes.

jabref/src/main/java/org/jabref/gui/edit/CopyTo.java

Lines 55 to 60 in 19df622

	for (BibEntry bibEntry : selectedEntries) {
	if (bibEntry.hasField(StandardField.CROSSREF) && showDialogBox) {
	includeCrossReferences = askForCrossReferencedEntries();
	copyToPreferences.setShouldIncludeCrossReferences(includeCrossReferences);
	}
	}

If showDialogBox is false, it will never update and will remain false, which is the cause of the bug.

[subhramit]

The failing unit tests would also need to be looked at.

[priyanshu16095]

@subhramit At that time, I tried it in a way that if the user had selected multiple entries, it would individually ask for confirmation for each entry that had cross-referenced entries.

I've seen this behavior multiple times in Windows.
I will try to improve this, in the same PR.

[subhramit]

@subhramit At that time, I tried it in a way that if the user had selected multiple entries, it would individually ask for confirmation for each entry that had cross-referenced entries.

I would not say that's a good way to do it - especially if a user is working with a large number of entries.

I will try to improve this, in the same PR.

🚀

Change the title as well to "Improve Copy to option + fix usage of preference" or something (and mark draft till done) if you plan to do that. A separate PR is also fine, up to you.

[Siedlerchr]

Please chekc the failing unit tests, otherwise it looks good

[Siedlerchr]

Is this ready now?

[priyanshu16095]

Yes, Please review.

[koppor]

Looked at it. OK. Next time, add some Java Comment -> the "break" took me 30 seconds to understand without any comment.