From 217f9024c77bf3e7a4d6e8747ecff721ac0f03ff Mon Sep 17 00:00:00 2001 From: "Anna (Ruoyu) Qian" Date: Mon, 29 Nov 2021 08:49:44 -0600 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../gui/fieldeditors/LinkedFileViewModel.java | 22 +++++++++++-- .../gui/fieldeditors/LinkedFilesEditor.java | 4 +-- .../fieldeditors/LinkedFileViewModelTest.java | 32 +++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f8e42e06c..e8d88cebc9e 100644 --- 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 index 6d3d3ac2988..50f27f6bb5e 100644 --- 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 newDir = databaseContext.getFirstExistingFileDir(preferences.getFilePreferences()); + FilePreferences filePreferences = preferences.getFilePreferences(); + Optional 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 targetDir = baseDir.map(dir -> dir.resolve(targetDirectoryName)); Optional currentDir = linkedFile.findIn(databaseContext, preferences.getFilePreferences()).map(Path::getParent); + if (currentDir.isEmpty()) { + // Could not find file + return false; + } BiPredicate 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 index 36aba630f54..ab8bac582aa 100644 --- 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 index 32967ed0d34..6c4ddf69385 100644 --- 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