Fix for issue #10418 - Added the ability to focus on the most viewed entries #12077

ExrosZ · 2024-10-24T14:11:19Z

Describe the changes you have made here:

By enabling view tracking in the entry tab, JabRef will track how many times a user looks an an entry every hour.
This information is stored on the user's machine in an MVStore.

This can then be used to create a group that shows the X most popular entries in Y timeframe, so long as each entry has atleast one view. If there are less entries with views than entries requested, it will just show the entries with 1 or more views.

Closes #10418

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Issues with my implementation:

Fails checkstyle due to using some static variables in EntryTabViewModel
Fails respectLayeredArchitecture test because popularityGroup is being used to get an MVStore, this is the only way I could find to get it to work without causing locking issues.
Likely needs some reworking to fix the above issues, but I'm genuinely lost.

# Conflicts: # src/main/java/org/jabref/gui/LibraryTab.java

koppor · 2024-10-24T15:55:59Z

src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java

+        try {
+            Files.createDirectories(userDataDir);
+        } catch (IOException e) {
+            // Update this for more elegant error handling


AI generated. Pleasemwork on this

koppor · 2024-10-24T15:56:13Z

src/main/java/org/jabref/gui/LibraryTab.java

+                int currentCount = viewCounts.getOrDefault(uniqueKey, 0);
+                viewCounts.put(uniqueKey, currentCount + 1);
+                lastViewTimestamps.put(uniqueKey, currentTime);
+                mvStore.commit(); // Save changes


AI comment. Delete.

koppor · 2024-10-24T15:56:53Z

src/main/java/org/jabref/gui/LibraryTab.java

+            String uniqueKey = PopularityGroup.getUniqueKeyForEntry(entry);
+            long lastViewTime = lastViewTimestamps.getOrDefault(uniqueKey, 0L);
+
+            if (currentTime - lastViewTime >= 3600000 && EntryTabViewModel.trackViewsProperty().get()) { // 3600000 milliseconds in one hour


Introduce constant

You can also use java.time.Duration.

koppor · 2024-10-24T15:57:09Z

src/main/java/org/jabref/gui/LibraryTab.java

@@ -7,6 +7,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Random;
+import java.util.concurrent.locks.ReentrantLock;


what is this useful for?

Sorry about that, I was having issues with file locks earlier in development and forgot to remove the import and some variables I never used.

koppor

The architecture of the functionality should be changed: MVStore life cycle should be handled by the tab (and passed down to the group). See AIIndexer or SearchIndex how it could work. -- You also persist data across sessions: Thus, you will have a simlar functionality.

I do not know why ""%Group containing entries cited in a given TeX file" sneaked in. I did not find functionality about that.

koppor · 2024-10-26T08:23:52Z

src/main/java/org/jabref/gui/LibraryTab.java

+    /**
+     * Takes an entry and increments the number of times it has been viewed by 1.
+     */


It does more - there is a kind of if

koppor · 2024-10-26T08:24:42Z

src/main/java/org/jabref/gui/LibraryTab.java

+            MVMap<String, Integer> viewCounts = mvStore.openMap("entryViewCounts");
+            MVMap<String, Long> lastViewTimestamps = mvStore.openMap("lastViewTimestamps");


Nono - the map is a class variable - no need to open the map each time for writing.

You need to shut down the mvstore when the tab is closed.

The the lifecycle of the AiService or search for examples.

koppor · 2024-10-26T08:25:03Z

src/main/java/org/jabref/gui/LibraryTab.java

+            String uniqueKey = PopularityGroup.getUniqueKeyForEntry(entry);
+            long lastViewTime = lastViewTimestamps.getOrDefault(uniqueKey, 0L);
+
+            if (currentTime - lastViewTime >= ONE_HOUR_IN_MILLISECONDS && EntryTabViewModel.trackViewsProperty().get()) {


The check for the property is too late - everything doesn't need to be done if the property is false.

koppor · 2024-10-26T08:25:19Z

src/main/java/org/jabref/gui/LibraryTab.java

+                int currentCount = viewCounts.getOrDefault(uniqueKey, 0);
+                viewCounts.put(uniqueKey, currentCount + 1);
+                lastViewTimestamps.put(uniqueKey, currentTime);
+                mvStore.commit();


I think, the commit happens automatically.

koppor · 2024-10-26T08:25:41Z

src/main/java/org/jabref/gui/LibraryTab.java

+     */
+    private void incrementViewCount(BibEntry entry) {
+        MVStore mvStore = PopularityGroup.getMVStore();
+        synchronized (mvStore) {


if the maps are opened after load, there is no need for synchronized (see below)

koppor · 2024-10-26T08:34:45Z

src/main/java/org/jabref/model/groups/PopularityGroup.java

+     * @return The number of views for the BibEntry, or 0 if not found.
+     */
+    public static int getEntryViewCount(BibEntry entry) {
+        MVMap<String, Integer> viewCounts = mvStore.openMap("entryViewCounts");


Please try to make that a class variable. It is passed from the library tab on creation.

koppor · 2024-10-26T08:35:15Z

src/main/java/org/jabref/model/groups/PopularityGroup.java

+    public static String getUniqueKeyForEntry(BibEntry entry) {
+        return entry.getField(StandardField.DOI).orElse(
+                entry.getField(StandardField.KEY).orElse(
+                        String.valueOf(entry.hashCode())
+                )
+        );
+    }


Remove --> use org.jabref.model.entry.BibEntry#getId instead.

koppor · 2024-10-26T08:35:27Z

src/main/java/org/jabref/model/groups/PopularityGroup.java

+    public static synchronized MVStore getMVStore() {
+        if (mvStore == null) {
+            Path mvStorePath = getOtherDataDir().resolve("tracking.mv");
+            mvStore = new MVStore.Builder().fileName(mvStorePath.toString()).open();
+        }
+        return mvStore;
+    }


MVStore should be passed by caller.

koppor · 2024-10-26T08:36:48Z

src/main/java/org/jabref/model/groups/PopularityGroup.java

+    private long getTimeThreshold(long currentTime, String selectedTimePeriod) {
+        return switch (selectedTimePeriod) {
+            case "Last week" ->
+                    currentTime - 7 * 24 * 60 * 60 * 1000L;
+            case "Last month" ->
+                    currentTime - 30 * 24 * 60 * 60 * 1000L;
+            case "Last year" ->
+                    currentTime - 365 * 24 * 60 * 60 * 1000L;
+            default -> // "All time"
+                    0;
+        };
+    }


The factor should be stored upon group creation (I think on change of group properties, the group is re-created)

Always reading the property seems to slow down things.

koppor · 2024-10-26T08:38:24Z

src/main/java/org/jabref/gui/preferences/entry/EntryTab.fxml

+
+
+
+    <CheckBox fx:id="enableViewTracking" text="%Enable user behaviour analysis"/>


Enable user behaviour analysis --> wrong word - please try to use the same words "view tracking" below "popularity" is OK, but "bheavior analysis" is even more general. It should be consistent with src/main/java/org/jabref/gui/groups/GroupDialog.fxml.

ExrosZ · 2024-10-26T13:47:46Z

Thanks for your review @koppor, I have exams coming up so i'll need to take a break from working on this for the next two-ish weeks, but im committed to getting this done :)

Sorry for all the mess, it's my first time contributing to a project like this.

HoussemNasri · 2024-11-02T19:15:40Z

src/main/java/org/jabref/model/groups/PopularityGroup.java

+    }
+
+    public List<BibEntry> processPopularityGroup(List<BibEntry> allEntries, String timePeriodCombo, Integer maxEntriesCombo) {
+     List<BibEntry> filteredEntries = new ArrayList<>();


Let's call this popularEntries for consistency.

ExrosZ-Alt and others added 11 commits October 21, 2024 19:51

Added UI items

07ead47

Got the Popularity button to enable/disable based on preferences.

0fa8663

Logic for counting views is now working correctly

d7697a1

Basic functionality added

15547d3

Fixed issue

f215a82

Minor fixes

f72b644

Merge branch 'JabRef:main' into popularity

bce457e

Added the ability to sort entries by popularity.

cbf4b27

Merge branch 'main' of https://github.com/JabRef/jabref into popularity

1f14d65

# Conflicts: # src/main/java/org/jabref/gui/LibraryTab.java

Merged with JabRef main

8c2efa2

Updated JabRef_en.properties

8d96cb5

koppor requested changes Oct 24, 2024

View reviewed changes

Fixed issues identified by Koppor

31e22bd

ExrosZ requested a review from koppor October 25, 2024 13:31

koppor requested changes Oct 26, 2024

View reviewed changes

koppor marked this pull request as draft October 26, 2024 14:35

koppor added the status: changes required Pull requests that are not yet complete label Oct 26, 2024

HoussemNasri reviewed Nov 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for issue #10418 - Added the ability to focus on the most viewed entries #12077

Fix for issue #10418 - Added the ability to focus on the most viewed entries #12077

ExrosZ commented Oct 24, 2024

koppor Oct 24, 2024

koppor Oct 24, 2024

koppor Oct 24, 2024

HoussemNasri Nov 2, 2024 •

edited

Loading

koppor Oct 24, 2024

ExrosZ Oct 25, 2024

koppor left a comment

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

koppor Oct 26, 2024

ExrosZ commented Oct 26, 2024

HoussemNasri Nov 2, 2024

		MVMap<String, Integer> viewCounts = mvStore.openMap("entryViewCounts");
		MVMap<String, Long> lastViewTimestamps = mvStore.openMap("lastViewTimestamps");




		<CheckBox fx:id="enableViewTracking" text="%Enable user behaviour analysis"/>

Fix for issue #10418 - Added the ability to focus on the most viewed entries #12077

Are you sure you want to change the base?

Fix for issue #10418 - Added the ability to focus on the most viewed entries #12077

Conversation

ExrosZ commented Oct 24, 2024

Mandatory checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HoussemNasri Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ExrosZ commented Oct 26, 2024

Choose a reason for hiding this comment

HoussemNasri Nov 2, 2024 •

edited

Loading