Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only disable move to file dir when path equals #7269

Merged
merged 3 commits into from
Dec 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
### Fixed

- We fixed an issue with the style of highlighted check boxes while searching in preferences. [#7226](https://github.com/JabRef/jabref/issues/7226)
- We fixed an issue where the option "Move file to file directory" was disabled in the entry editor for all files [#7194](https://github.com/JabRef/jabref/issues/7194)

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,14 @@ public boolean isGeneratedNameSameAsOriginal() {
}

/**
* Compares suggested filepath of current linkedFile with existing filepath.
* Compares suggested directory of current linkedFile with existing filepath directory.
*
* @return true if suggested filepath is same as existing filepath.
*/
public boolean isGeneratedPathSameAsOriginal() {
Optional<Path> newDir = databaseContext.getFirstExistingFileDir(filePreferences);

Optional<Path> currentDir = linkedFile.findIn(databaseContext, filePreferences);
Optional<Path> currentDir = linkedFile.findIn(databaseContext, filePreferences).map(Path::getParent);

BiPredicate<Path, Path> equality = (fileA, fileB) -> {
try {
Expand Down Expand Up @@ -434,7 +434,7 @@ public void download() {
List<LinkedFile> linkedFiles = entry.getFiles();
int oldFileIndex = -1;
int i = 0;
while (i < linkedFiles.size() && oldFileIndex == -1) {
while ((i < linkedFiles.size()) && (oldFileIndex == -1)) {
LinkedFile file = linkedFiles.get(i);
// The file type changes as part of download process (see prepareDownloadTask), thus we only compare by link
if (file.getLink().equalsIgnoreCase(linkedFile.getLink())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,15 +267,15 @@ public ContextAction(StandardActions command, LinkedFileViewModel linkedFile, Pr

this.executable.bind(
switch (command) {
case RENAME_FILE_TO_PATTERN -> Bindings.createBooleanBinding(
case RENAME_FILE_TO_PATTERN, MOVE_FILE_TO_FOLDER_AND_RENAME -> Bindings.createBooleanBinding(
() -> !linkedFile.getFile().isOnlineLink()
&& linkedFile.getFile().findIn(databaseContext, preferencesService.getFilePreferences()).isPresent()
&& !linkedFile.isGeneratedNameSameAsOriginal(),
linkedFile.getFile().linkProperty());
case MOVE_FILE_TO_FOLDER_AND_RENAME, MOVE_FILE_TO_FOLDER -> Bindings.createBooleanBinding(
case MOVE_FILE_TO_FOLDER -> Bindings.createBooleanBinding(
() -> !linkedFile.getFile().isOnlineLink()
&& linkedFile.getFile().findIn(databaseContext, preferencesService.getFilePreferences()).isPresent()
&& linkedFile.isGeneratedPathSameAsOriginal(),
&& !linkedFile.isGeneratedPathSameAsOriginal(),
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
linkedFile.getFile().linkProperty());
case DOWNLOAD_FILE -> Bindings.createBooleanBinding(
() -> linkedFile.getFile().isOnlineLink(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.preferences.FilePreferences;
import org.jabref.testutils.category.FetcherTest;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -152,12 +151,11 @@ void deleteWhenDialogCancelledReturnsFalseAndDoesNotRemoveFile() {
assertTrue(Files.exists(tempFile));
}

@FetcherTest
void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), "");

databaseContext = mock(BibDatabaseContext.class);
when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]"); // use this variant, as we cannot mock the linkedFileHandler cause it's initialized inside the viewModel
when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");

LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, xmpPreferences, filePreferences, externalFileType);

Expand All @@ -170,4 +168,26 @@ void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
task.onFailure(Assertions::fail);
new CurrentThreadTaskExecutor().execute(task);
}

@Test
void isNotSamePath() {
linkedFile = new LinkedFile("desc", tempFile, "pdf");
databaseContext = mock(BibDatabaseContext.class);
when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
when(databaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(Path.of("/home")));

LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, xmpPreferences, filePreferences, externalFileType);
assertFalse(viewModel.isGeneratedPathSameAsOriginal());
}

@Test
void isSamePath() {
linkedFile = new LinkedFile("desc", tempFile, "pdf");
databaseContext = mock(BibDatabaseContext.class);
when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
when(databaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(tempFile.getParent()));

LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, xmpPreferences, filePreferences, externalFileType);
assertTrue(viewModel.isGeneratedPathSameAsOriginal());
}
}