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

Rename the Review Tab into Comments Tab #3658

Merged
merged 28 commits into from
Feb 17, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
289c2d2
Rename the Review Tab into Comments Tab
LinusDietz Jan 22, 2018
351b1db
Remove Localization Key for Review
LinusDietz Jan 22, 2018
ac341e4
Merge branch 'master' into comment-tab
LinusDietz Feb 1, 2018
07a8965
Use Comment field in the Comment tab
LinusDietz Feb 14, 2018
6a75c6a
Migration for the Review field
LinusDietz Feb 14, 2018
af9022a
Merge remote-tracking branch 'origin/comment-tab' into comment-tab
LinusDietz Feb 14, 2018
afe5476
Remove Comment Field from General Tab
LinusDietz Feb 14, 2018
995b965
Tests
LinusDietz Feb 14, 2018
056ef9d
Simplify tests
LinusDietz Feb 14, 2018
9d9c5a6
Fix Codacy
LinusDietz Feb 14, 2018
4a6886e
Package refactoring, streamline naming
LinusDietz Feb 14, 2018
a5a2994
Also move tests
LinusDietz Feb 14, 2018
37a247c
Merge branch 'master' of https://github.com/JabRef/jabref into commen…
LinusDietz Feb 14, 2018
1e0d56e
Add Confirmation Dialog for Merging Fields
LinusDietz Feb 15, 2018
e949025
Localize Tests
LinusDietz Feb 15, 2018
c93b490
Fix tests
LinusDietz Feb 15, 2018
e405f67
Fix Codacy
LinusDietz Feb 15, 2018
6cca4ab
fixed issue 298
grotepfn Feb 15, 2018
b7ca150
updated Changes
grotepfn Feb 15, 2018
38750e8
Update CHANGELOG.md
grotepfn Feb 15, 2018
2d2bb7e
User decides now once for all migrations.
LinusDietz Feb 15, 2018
dfeda8d
fix localization
LinusDietz Feb 16, 2018
4828955
Merge pull request #3726 from grotepfn/patch-3
lenhard Feb 16, 2018
5962710
Remove 'no' button
LinusDietz Feb 16, 2018
4c1905d
Merge branch 'master' of https://github.com/JabRef/jabref into commen…
LinusDietz Feb 16, 2018
38e6139
Merge branch 'comment-tab' of https://github.com/JabRef/jabref into c…
LinusDietz Feb 16, 2018
b246cfe
Mark Database as changed after the LoadDatabaseMigration
LinusDietz Feb 16, 2018
303ed4d
Getters and Setters
LinusDietz Feb 16, 2018
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 @@ -21,6 +21,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We improved the export of the `address` and `location` field to the MS-Office XML fields. If the address field does not contain a comma, it is treated as single value and exported to the field `city`. [#1750, comment](https://github.com/JabRef/jabref/issues/1750#issuecomment-357539167)
For more details refer to the [field mapping help page](http://help.jabref.org/en/MsOfficeBibFieldMapping)
- We added Facebook and Twitter icons in the toolbar to link to our [Facebook](https://www.facebook.com/JabRef/) and [Twitter](https://twitter.com/jabref_org) pages.
- Renamed the _Review_ Tab into _Comments_ Tab
- We no longer print empty lines when exporting an entry in RIS format [#3634](https://github.com/JabRef/jabref/issues/3634)
- We improved file saving so that hard links are now preserved when a save is performed [#2633](https://github.com/JabRef/jabref/issues/2633)

Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/jabref/logic/importer/OpenDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.jabref.logic.importer.fileformat.BibtexImporter;
import org.jabref.logic.importer.util.ConvertLegacyExplicitGroups;
import org.jabref.logic.importer.util.PostOpenAction;
import org.jabref.logic.importer.util.RenameReviewToComment;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.specialfields.SpecialFieldsUtils;
import org.jabref.logic.util.io.FileBasedLock;
Expand Down Expand Up @@ -87,7 +88,8 @@ public static ParserResult loadDatabase(File fileToOpen, ImportFormatPreferences
}

private static void applyPostActions(ParserResult parserResult) {
List<PostOpenAction> actions = Arrays.asList(new ConvertLegacyExplicitGroups());

List<PostOpenAction> actions = Arrays.asList(new ConvertLegacyExplicitGroups(), new RenameReviewToComment());

for (PostOpenAction action : actions) {
action.performAction(parserResult);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package org.jabref.logic.importer.util;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move the complete class into org.jabref.migrations? Because that's what it actually does, it migrates an opened bib file to a newer version of JabRef. I think it makes sense to bundle all this stuff in a dedicated package, because then it gets easier for us to see what sort of conversions are performed on open.

While you're doing that, could you also move the ConvertLegacyExplicitGroups?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I also renamed these actions to what they are: migrations


import java.util.Objects;

import org.jabref.logic.importer.ParserResult;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FieldName;

public class RenameReviewToComment implements PostOpenAction {

@Override
public void performAction(ParserResult parserResult) {
Objects.requireNonNull(parserResult);

for (BibEntry entry : parserResult.getDatabase().getEntries()) {
if (entry.getField(FieldName.REVIEW).isPresent()) {
String review = entry.getField(FieldName.REVIEW).get();
review = concatCommentFieldIfPresent(entry, review);
updateFields(entry, review);
}
}
}

private String concatCommentFieldIfPresent(BibEntry entry, String review) {
if (entry.getField(FieldName.COMMENT).isPresent()) {
String comment = entry.getField(FieldName.COMMENT).get().trim();
if (!comment.isEmpty()) {
review = String.format("%s\nReview:\n%s", comment, review.trim());
}
}

return review;
}

private void updateFields(BibEntry entry, String review) {
entry.setField(FieldName.COMMENT, review);
entry.clearField(FieldName.REVIEW);
}
}
11 changes: 5 additions & 6 deletions src/main/java/org/jabref/model/entry/InternalBibtexFields.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
* config files -> simple extension and definition of new fields
*
* TODO:
* - handling of identically fields with different names (https://github.com/JabRef/jabref/issues/521)
* e.g. LCCN = lib-congress, journaltitle = journal
* - group id for each fields, e.g. standard, jurabib, bio, ...
* - add a additional properties functionality into the BibtexSingleField class
* - handling of identically fields with different names (https://github.com/JabRef/jabref/issues/521)
* e.g. LCCN = lib-congress, journaltitle = journal
* - group id for each fields, e.g. standard, jurabib, bio, ...
* - add a additional properties functionality into the BibtexSingleField class
*/
public class InternalBibtexFields {

Expand All @@ -35,7 +35,7 @@ public class InternalBibtexFields {
*
* A user can change them. The change is currently stored in the preferences only and not explicitley exposed as separte preferences object
*/
public static final List<String> DEFAULT_GENERAL_FIELDS = Arrays.asList(FieldName.CROSSREF, FieldName.KEYWORDS, FieldName.FILE, FieldName.DOI, FieldName.URL, FieldName.GROUPS, FieldName.COMMENT, FieldName.OWNER, FieldName.TIMESTAMP);
public static final List<String> DEFAULT_GENERAL_FIELDS = Arrays.asList(FieldName.CROSSREF, FieldName.KEYWORDS, FieldName.FILE, FieldName.DOI, FieldName.URL, FieldName.GROUPS, FieldName.OWNER, FieldName.TIMESTAMP);

// Lists of fields with special properties
private static final List<String> INTEGER_FIELDS = Arrays.asList(FieldName.CTLMAX_NAMES_FORCED_ETAL,
Expand Down Expand Up @@ -382,7 +382,6 @@ public static void setNumericFields(List<String> numFields) {
field.setNumeric(true);
InternalBibtexFields.RUNTIME.fieldSet.put(fieldName, field);
}

}

public static Set<FieldProperty> getFieldProperties(String name) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -974,9 +974,9 @@ public void setLanguageDependentDefaultValues() {
defaults.put(CUSTOM_TAB_FIELDS + "_def1", FieldName.ABSTRACT);
defaults.put(CUSTOM_TAB_NAME + "_def1", Localization.lang("Abstract"));

// Entry editor tab 2: Review Field - used for research comments, etc.
defaults.put(CUSTOM_TAB_FIELDS + "_def2", FieldName.REVIEW);
defaults.put(CUSTOM_TAB_NAME + "_def2", Localization.lang("Review"));
// Entry editor tab 2: Comments Field - used for research comments, etc.
defaults.put(CUSTOM_TAB_FIELDS + "_def2", FieldName.COMMENT);
defaults.put(CUSTOM_TAB_NAME + "_def2", Localization.lang("Comments"));

defaults.put(EMAIL_SUBJECT, Localization.lang("References"));
}
Expand Down
4 changes: 1 addition & 3 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ Color\ for\ marking\ incomplete\ entries=Color for marking incomplete entries
Column\ width=Column width

Command\ line\ id=Command line id

Comments=Comments

Contained\ in=Contained in

Expand Down Expand Up @@ -1004,8 +1004,6 @@ Resolve\ strings\ for\ standard\ BibTeX\ fields\ only=Resolve strings for standa

resolved=resolved

Review=Review

Review\ changes=Review changes

Right=Right
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package org.jabref.logic.importer.util;

import java.util.Collections;

import org.jabref.logic.importer.ParserResult;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FieldName;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

class RenameReviewToCommentTest {

private RenameReviewToComment action;

@BeforeEach
void setUp() {
action = new RenameReviewToComment();
}

@Test
void noFields() {
BibEntry entry = createMinimalBibEntry();
ParserResult actualParserResult = new ParserResult(Collections.singletonList(entry));

action.performAction(actualParserResult);

assertEquals(new ParserResult(Collections.singletonList(entry)).getDatabase().getEntryByKey("Entry1"),
actualParserResult.getDatabase().getEntryByKey("Entry1"));
}

@Test
void reviewField() {
BibEntry actualEntry = createMinimalBibEntry();
actualEntry.setField(FieldName.REVIEW, "My Review");
ParserResult actualParserResult = new ParserResult(Collections.singletonList(actualEntry));

BibEntry expectedEntry = createMinimalBibEntry();
expectedEntry.setField(FieldName.COMMENT, "My Review");

action.performAction(actualParserResult);

assertEquals(new ParserResult(Collections.singletonList(expectedEntry)).getDatabase().getEntryByKey("Entry1"),
actualParserResult.getDatabase().getEntryByKey("Entry1"));
}

@Test
void commentField() {
BibEntry entry = createMinimalBibEntry();
entry.setField(FieldName.COMMENT, "My Comment");
ParserResult actualParserResult = new ParserResult(Collections.singletonList(entry));

action.performAction(actualParserResult);

assertEquals(new ParserResult(Collections.singletonList(entry)).getDatabase().getEntryByKey("Entry1"),
actualParserResult.getDatabase().getEntryByKey("Entry1"));
}


@Test
void reviewAndCommentField() {
BibEntry actualEntry = createMinimalBibEntry();
actualEntry.setField(FieldName.REVIEW, "My Review");
actualEntry.setField(FieldName.COMMENT, "My Comment");

ParserResult actualParserResult = new ParserResult(Collections.singletonList(actualEntry));

BibEntry expectedEntry = createMinimalBibEntry();
expectedEntry.setField(FieldName.COMMENT, "My Comment\nReview:\nMy Review");

action.performAction(actualParserResult);

assertEquals(new ParserResult(Collections.singletonList(expectedEntry)).getDatabase().getEntryByKey("Entry1"),
Copy link
Member

@tobiasdiez tobiasdiez Feb 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just assertEquals(expectedEntry, actualParserResult.getDatabase().getEntryByKey("Entry1"))?

Same remark applies also to some other tests here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good remark! (:

actualParserResult.getDatabase().getEntryByKey("Entry1"));
}

private BibEntry createMinimalBibEntry() {
BibEntry entry = new BibEntry();
entry.setCiteKey("Entry1");
entry.setField(FieldName.TITLE, "A random entry!");
entry.setField(FieldName.AUTHOR, "JabRef DEVELOPERS");
return entry;
}
}