Skip to content

Commit 56577b5

Browse files
authored
Fix attach file action from contextmenu (#3084)
* Fix attach file action from contextmenu Return new ArrayList instead of unmodiefable emptyList() * Add unit test * always return ArrayList * Add javadoc comment * change to early return * fix checkstyle
1 parent 3acd1f8 commit 56577b5

File tree

6 files changed

+27
-13
lines changed

6 files changed

+27
-13
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
2525
- We fixed an issue where the preferences could not be imported without a restart of JabRef [#3064](https://github.com/JabRef/jabref/issues/3064)
2626
- We fixed an issue where <kbd>DEL</kbd>, <kbd>Ctrl</kbd>+<kbd>C</kbd>, <kbd>Ctrl</kbd>+<kbd>V</kbd> and <kbd>Ctrl</kbd>+<kbd>A</kbd> in the search field triggered corresponding actions in the main table [#3067](https://github.com/JabRef/jabref/issues/3067)
2727
- We fixed an issue where JabRef freezed when editing an assigned file in the `General`-Tab [#2930, comment](https://github.com/JabRef/jabref/issues/2930#issuecomment-311050976)
28+
- We fixed an issue where a file could not be assigned to an existing entry via the entry context menu action `Attach file` [#3080](https://github.com/JabRef/jabref/issues/3080)
2829
### Removed
2930

3031

src/main/java/org/jabref/gui/filelist/AttachFileAction.java

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ public class AttachFileAction implements BaseAction {
1515

1616
private final BasePanel panel;
1717

18-
1918
public AttachFileAction(BasePanel panel) {
2019
this.panel = panel;
2120
}

src/main/java/org/jabref/gui/filelist/FileListEntryEditor.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,12 @@ public class FileListEntryEditor {
123123

124124
private boolean dontOpenBrowseUntilDisposed;
125125

126-
public FileListEntryEditor(LinkedFile entry, boolean showProgressBar, boolean showOpenButton,
127-
BibDatabaseContext databaseContext, boolean showSaveDialog) {
126+
public FileListEntryEditor(LinkedFile entry, boolean showProgressBar, boolean showOpenButton, BibDatabaseContext databaseContext, boolean showSaveDialog) {
128127
this(entry, showProgressBar, showOpenButton, databaseContext);
129128
this.showSaveDialog = showSaveDialog;
130129
}
131130

132-
public FileListEntryEditor(LinkedFile entry, boolean showProgressBar, boolean showOpenButton,
133-
BibDatabaseContext databaseContext) {
131+
public FileListEntryEditor(LinkedFile entry, boolean showProgressBar, boolean showOpenButton, BibDatabaseContext databaseContext) {
134132
this.entry = entry;
135133
this.databaseContext = databaseContext;
136134

src/main/java/org/jabref/model/entry/BibEntry.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.jabref.model.entry;
22

3+
import java.util.ArrayList;
34
import java.util.Collection;
45
import java.util.Collections;
56
import java.util.HashMap;
@@ -802,13 +803,14 @@ public Optional<FieldChange> setFiles(List<LinkedFile> files) {
802803
/**
803804
* Gets a list of linked files.
804805
*
805-
* @return the list of linked files, is never null but can be empty
806+
* @return the list of linked files, is never null but can be empty.
807+
* Changes to the underlying list will have no effect on the entry itself. Use {@link #addFile(LinkedFile)}
806808
*/
807809
public List<LinkedFile> getFiles() {
808810
//Extract the path
809811
Optional<String> oldValue = getField(FieldName.FILE);
810812
if (!oldValue.isPresent()) {
811-
return Collections.emptyList();
813+
return new ArrayList<>(); //Return new ArrayList because emptyList is immutable
812814
}
813815

814816
return FileFieldParser.parse(oldValue.get());

src/main/java/org/jabref/model/entry/FileFieldParser.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
package org.jabref.model.entry;
22

33
import java.util.ArrayList;
4-
import java.util.Collections;
54
import java.util.List;
65

76
public class FileFieldParser {
7+
88
public static List<LinkedFile> parse(String value) {
9+
List<LinkedFile> files = new ArrayList<>();
10+
911
if ((value == null) || value.trim().isEmpty()) {
10-
return Collections.emptyList();
12+
return files;
1113
}
1214

13-
List<LinkedFile> files = new ArrayList<>();
1415
List<String> entry = new ArrayList<>();
1516
StringBuilder sb = new StringBuilder();
1617
boolean inXmlChar = false;

src/test/java/org/jabref/model/entry/BibEntryTest.java

+16-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
package org.jabref.model.entry;
22

3+
import java.util.Arrays;
4+
import java.util.List;
35
import java.util.Optional;
46

57
import org.junit.After;
6-
import org.junit.Assert;
78
import org.junit.Before;
89
import org.junit.Test;
910

11+
import static org.junit.Assert.assertEquals;
12+
import static org.junit.Assert.assertNotEquals;
13+
1014
public class BibEntryTest {
15+
1116
private BibEntry entry;
1217

1318
@Before
@@ -34,14 +39,22 @@ public void notClearReservedFields() {
3439
public void getFieldIsCaseInsensitive() throws Exception {
3540
entry.setField("TeSt", "value");
3641

37-
Assert.assertEquals(Optional.of("value"), entry.getField("tEsT"));
42+
assertEquals(Optional.of("value"), entry.getField("tEsT"));
3843
}
3944

4045
@Test
4146
public void clonedBibentryHasUniqueID() throws Exception {
4247
BibEntry entry = new BibEntry();
4348
BibEntry entryClone = (BibEntry) entry.clone();
4449

45-
Assert.assertNotEquals(entry.getId(), entryClone.getId());
50+
assertNotEquals(entry.getId(), entryClone.getId());
51+
}
52+
53+
@Test
54+
public void testGetAndAddToLinkedFileList() {
55+
List<LinkedFile> files = entry.getFiles();
56+
files.add(new LinkedFile("", "", ""));
57+
entry.setFiles(files);
58+
assertEquals(Arrays.asList(new LinkedFile("", "", "")), entry.getFiles());
4659
}
4760
}

0 commit comments

Comments
 (0)