Fixed Grobid Preference Dialog Logic, Removed Checkbox

[arshchawla21]

This issue revolves around the Grobid preference dialog logic. More specifically, it was found the "Do not ask again" button would not function as expected in certain cases, when asking the user for permission to use Grobid when uploading a PDF. Below demonstrates a logic diagram for the previous (broken) functionality. For context, the previous implentation (in GrobidPreferences.java), contained two boolean key variables, grobidEnabled and grobidOptOut.

From the diagram it is clear that in certain cases (highlighted red), the logic does not work as expected. A potential fix revolves around removing the checkbox altogether, only displaying the dialog once, and saving the users' preference for the future. This approach has been suggested by the following comments: #9801 (comment) and #9802 (comment). The new logic would operate as per the following diagram:

My implementation follows the above logic, with grobidOptOut being redefined to grobidPreference. Below is the dialog a user will receive if they have not used Grobid before:

While this will work as expected for new users, one potential flaw is that some users (who have used Grobid in the past) will be also receive this dialog. Specifically users who had Grobid enabled and users who had Grobid disabled, but did not specifically opt-out (did not press "Do not ask again").

I believe the implementation is robust, but I'm always open to suggestions and would greatly appreciate any feedback on potential improvements!

Fixes JabRef#566.

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.

[Siedlerchr]

Yes no only dialogs should be avoided, as the buttons are not self explanatory, better wording:

• Send to Grobid
• Do not send

[koppor]

We have this requirement at https://devdocs.jabref.org/code-howtos/ui-recommendations.html#name-your-buttons-for-the-actions, but need more text for explanation.

„yes“ is not an action.

[koppor]

Note this removes the possibility to ask everytime. #9291 (comment) – This is OK for me now!

[koppor]

The new logic would operate as per the following diagram:

I like diagrams. Thank you!

I do not get what „enabled“ means.

The diagram does not show what happens after restart of JabRef. The dialog should only appear lf user did Not decide yet.

[koppor]

We should use another word. This is NOT a JabRef setting (AKA preference), but a kind of "to prefer something". Moreover, this is asked at the first time only. Thus, I would rename it to isGrobidUseAsked. This is to strengthen that this is a one-time thing (in contrast to being asked at every run).

[arshchawla21]

Thank you for the observation, this would certainly make the logic more clear. I have made the changes as required.

[arshchawla21]

Yes no only dialogs should be avoided, as the buttons are not self explanatory, better wording:

• Send to Grobid
• Do not send

I have updated the dialog as per this suggestion, improving clarity.

The new logic would operate as per the following diagram:

I like diagrams. Thank you!

I do not get what „enabled“ means.

The diagram does not show what happens after restart of JabRef. The dialog should only appear lf user did Not decide yet.

Enabled is referring to the grobidEnabled variable. To confirm, the dialog only appears if the user has not decided. This is due to the groidPreference variable being updated and stored after the user has made their decision for the first time.

[koppor]

@arshchawla21 I review using https://github.com/JabRef/jabref/pull/12034/files. Maybe, you should open this, too. - You will see that Christoph made a suggestion to the CHANGELOG.md

[koppor]

I resolved the merge conflicts in CHANGELOG.md to have the CI working.

[koppor]

I added it to the merge queue. Should be in main in 30 minutes. Thank you for your work.

[arshchawla21]

Thank you so much for your guidance @koppor, is there anything else I need to do? The unit test check appears to be failing.

[koppor]

Thank you so much for your guidance @koppor, is there anything else I need to do? The unit test check appears to be failing.

Fortunately, we have the merge queue doing the final checks properly.

Please check and fix:

 org.jabref.logic.l10n.LocalizationConsistencyTest

  Test findMissingLocalizationKeys() FAILED

  org.opentest4j.AssertionFailedError: 
  DETECTED LANGUAGE KEYS WHICH ARE NOT IN THE ENGLISH LANGUAGE FILE
  PASTE THESE INTO THE ENGLISH LANGUAGE FILE "JabRef_en.properties"
  
  Do not send=Do not send
  Send to Grobid=Send to Grobid
  
   ==> expected: <[]> but was: <[Do not send (src/main/java/org/jabref/gui/importer/GrobidUseDialogHelper.java LANG), Send to Grobid (src/main/java/org/jabref/gui/importer/GrobidUseDialogHelper.java LANG)]>

[arshchawla21]

Hi @koppor, I've made the required changes to "JabRef_en.properties", and the checks appear to be passing. I believe this PR is ready to merge whenever you are ready!

[LoayGhreeb]

The dialog should not be displayed if preferences are enabled without it. Here's an example:

1. Reset preferences.
2. Restart.
3. Open preferences, then go to Web Search and allow Grobid.
4. Import a file.
5. The dialog will prompt to enable Grobid, even though it is already enabled.

[arshchawla21]

Hey @LoayGhreeb, thanks for the suggestion. I have made changes such that if a user has manually enabled Grobid, they will not be prompted in any case (even if they manually disable Grobid in the future) as required.

[ThiloteE]

@arshchawla21 If I may ask, what app did you use to make those diagrams?

[arshchawla21]

@arshchawla21 If I may ask, what app did you use to make those diagrams?

Of course, I used draw.io (available at https://app.diagrams.net/) to make the diagrams!