Consider directory pattern when checking if a file can be moved (#8244)

* Update LinkedFilesEditor.java Disable MOVE_FILE_TO_FOLDER_AND_RENAME using the same logic as MOVE_FILE_TO_FOLDER * Update FileHelper.java * Update LinkedFileViewModel.java * Update FileHelper.java Added Javadoc comment * Update LinkedFileViewModelTest.java * Update LinkedFileViewModelTest.java * Update LinkedFileViewModelTest.java Adds test to test if isGeneratedPathSameAsOriginal takes into consideration File directory pattern * Update FileUtilTest.java Adds unit tests for FileHelper.equals * Update FileUtilTest.java * Update LinkedFileViewModelTest.java * Update LinkedFileViewModel.java * Update LinkedFileViewModel.java * Update LinkedFileViewModel.java * Update LinkedFileViewModel.java * Update FileHelper.java removed newly added function, which is no longer needed * Update FileUtilTest.java removed newly added tests for the function no longer exist * Update CHANGELOG.md * Update CHANGELOG.md
JabRef · Nov 29, 2021 · 217f902 · 217f902
1 parent 58d7593
commit 217f902
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,6 +55,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 
 ### Fixed
 
+- We fixed an issue where the actions to move a file to a directory were incorrectly disabled. [#7908](https://github.com/JabRef/jabref/issues/7908)
 - We fixed an issue where an exception occurred when a linked online file was edited in the entry editor [#8008](https://github.com/JabRef/jabref/issues/8008)
 - We fixed an issue when checking for a new version when JabRef is used behind a corporate proxy. [#7884](https://github.com/JabRef/jabref/issues/7884)
 - We fixed some icons that were drawn in the wrong color when JabRef used a custom theme. [#7853](https://github.com/JabRef/jabref/issues/7853)

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -61,6 +61,7 @@
 import org.jabref.model.strings.StringUtil;
 import org.jabref.model.util.FileHelper;
 import org.jabref.model.util.OptionalUtil;
+import org.jabref.preferences.FilePreferences;
 import org.jabref.preferences.PreferencesService;
 
 import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
@@ -332,9 +333,26 @@ public boolean isGeneratedNameSameAsOriginal() {
      * @return true if suggested filepath is same as existing filepath.
      */
     public boolean isGeneratedPathSameAsOriginal() {
-        Optional<Path> newDir = databaseContext.getFirstExistingFileDir(preferences.getFilePreferences());
+        FilePreferences filePreferences = preferences.getFilePreferences();
+        Optional<Path> baseDir = databaseContext.getFirstExistingFileDir(filePreferences);
+        if (baseDir.isEmpty()) {
+            // could not find default path
+            return false;
+        }
+
+        // append File directory pattern if exits
+        String targetDirectoryName = FileUtil.createDirNameFromPattern(
+                databaseContext.getDatabase(),
+                entry,
+                filePreferences.getFileDirectoryPattern());
+
+        Optional<Path> targetDir = baseDir.map(dir -> dir.resolve(targetDirectoryName));
 
         Optional<Path> currentDir = linkedFile.findIn(databaseContext, preferences.getFilePreferences()).map(Path::getParent);
+        if (currentDir.isEmpty()) {
+            // Could not find file
+            return false;
+        }
 
         BiPredicate<Path, Path> equality = (fileA, fileB) -> {
             try {
@@ -343,7 +361,7 @@ public boolean isGeneratedPathSameAsOriginal() {
                 return false;
             }
         };
-        return OptionalUtil.equals(newDir, currentDir, equality);
+        return OptionalUtil.equals(targetDir, currentDir, equality);
     }
 
     public void moveToDefaultDirectoryAndRename() {

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
@@ -294,12 +294,12 @@ public ContextAction(StandardActions command, LinkedFileViewModel linkedFile, Pr
 
             this.executable.bind(
                     switch (command) {
-                        case RENAME_FILE_TO_PATTERN, MOVE_FILE_TO_FOLDER_AND_RENAME -> Bindings.createBooleanBinding(
+                        case RENAME_FILE_TO_PATTERN -> Bindings.createBooleanBinding(
                                 () -> !linkedFile.getFile().isOnlineLink()
                                         && linkedFile.getFile().findIn(databaseContext, preferencesService.getFilePreferences()).isPresent()
                                         && !linkedFile.isGeneratedNameSameAsOriginal(),
                                 linkedFile.getFile().linkProperty(), bibEntry.getValue().map(BibEntry::getFieldsObservable).orElse(null));
-                        case MOVE_FILE_TO_FOLDER -> Bindings.createBooleanBinding(
+                        case MOVE_FILE_TO_FOLDER, MOVE_FILE_TO_FOLDER_AND_RENAME -> Bindings.createBooleanBinding(
                                 () -> !linkedFile.getFile().isOnlineLink()
                                         && linkedFile.getFile().findIn(databaseContext, preferencesService.getFilePreferences()).isPresent()
                                         && !linkedFile.isGeneratedPathSameAsOriginal(),

diff --git a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java
@@ -1,5 +1,6 @@
 package org.jabref.gui.fieldeditors;
 
+import java.io.IOException;
 import java.net.CookieHandler;
 import java.net.CookieManager;
 import java.net.CookiePolicy;
@@ -22,6 +23,7 @@
 import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.CurrentThreadTaskExecutor;
 import org.jabref.gui.util.TaskExecutor;
+import org.jabref.logic.externalfiles.LinkedFileHandler;
 import org.jabref.logic.net.URLDownload;
 import org.jabref.logic.xmp.XmpPreferences;
 import org.jabref.model.database.BibDatabaseContext;
@@ -255,6 +257,7 @@ void isNotSamePath() {
         linkedFile = new LinkedFile("desc", tempFile, "pdf");
         databaseContext = mock(BibDatabaseContext.class);
         when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("");
         when(databaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(Path.of("/home")));
 
         LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType);
@@ -266,12 +269,41 @@ void isSamePath() {
         linkedFile = new LinkedFile("desc", tempFile, "pdf");
         databaseContext = mock(BibDatabaseContext.class);
         when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("");
         when(databaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(tempFile.getParent()));
 
         LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType);
         assertTrue(viewModel.isGeneratedPathSameAsOriginal());
     }
 
+    // Tests if isGeneratedPathSameAsOriginal takes into consideration File directory pattern
+    @Test
+    void isNotSamePathWithPattern() {
+        linkedFile = new LinkedFile("desc", tempFile, "pdf");
+        databaseContext = mock(BibDatabaseContext.class);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("[entrytype]");
+        when(databaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(tempFile.getParent()));
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType);
+        assertFalse(viewModel.isGeneratedPathSameAsOriginal());
+    }
+
+    // Tests if isGeneratedPathSameAsOriginal takes into consideration File directory pattern
+    @Test
+    void isSamePathWithPattern() throws IOException {
+        linkedFile = new LinkedFile("desc", tempFile, "pdf");
+        databaseContext = mock(BibDatabaseContext.class);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("[entrytype]");
+        when(databaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(tempFile.getParent()));
+
+        LinkedFileHandler fileHandler = new LinkedFileHandler(linkedFile, entry, databaseContext, filePreferences);
+        fileHandler.moveToDefaultDirectory();
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType);
+        assertTrue(viewModel.isGeneratedPathSameAsOriginal());
+    }
 
     // Tests if added parameters to mimeType gets parsed to correct format.
     @Test