Skip to content

GUI for consistency check#12433

Merged
Siedlerchr merged 34 commits intoJabRef:mainfrom
priyanshu16095:consistencyGuiCheck
Feb 9, 2025
Merged

GUI for consistency check#12433
Siedlerchr merged 34 commits intoJabRef:mainfrom
priyanshu16095:consistencyGuiCheck

Conversation

@priyanshu16095
Copy link
Copy Markdown
Contributor

@priyanshu16095 priyanshu16095 commented Jan 30, 2025

Fixes #11950

This PR introduces a GUI for a bibliography consistency check to ensure consistency among BibTeX entries.

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 change is visible to the user)
  • 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.

Screenshot (26)
Screenshot (54)
Screenshot (59)

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

@priyanshu16095
Copy link
Copy Markdown
Contributor Author

priyanshu16095 commented Jan 30, 2025

Recording.2025-01-31.mp4

Added a table, but figuring out how to proceed with inserting data into the table as shown in the UI.

Please review.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Copy link
Copy Markdown
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

Just by a quick look two remarks
Also about the picture in the issue: The user needs some hints what o and ? and so on mean. It must be understandable somehow right away.

Comment thread src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialog.java Outdated
Comment thread src/main/resources/l10n/JabRef_en.properties Outdated
Comment thread src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialogViewModel.java Outdated
Comment thread src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialogViewModel.java Outdated
Comment thread src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialogViewModel.java Outdated
Comment thread src/main/java/org/jabref/gui/consistency/ConsistencyCheckDialog.java Outdated
Comment thread src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

Comment thread CHANGELOG.md Outdated
@priyanshu16095
Copy link
Copy Markdown
Contributor Author

I have taken a lot of code as it is from the BibliographyConsistencyCheckResultWriter class in the ConsistencyCheckDialogViewModel class. Is there a better way to do this?

@priyanshu16095
Copy link
Copy Markdown
Contributor Author

priyanshu16095 commented Feb 3, 2025

Recording.2025-02-03.mp4

The table is displaying data correctly.
The tasks are running in background.

What remains to be done:

  • Providing information of the symbols.
  • Filtering data based on the selected value of the combo box
  • Implementing the jump to the selected row feature

Siedlerchr
Siedlerchr previously approved these changes Feb 9, 2025
@Siedlerchr Siedlerchr enabled auto-merge February 9, 2025 19:59
Comment thread src/main/java/org/jabref/logic/importer/WebFetchers.java Outdated
@calixtus
Copy link
Copy Markdown
Member

calixtus commented Feb 9, 2025

Either i dont understand one comment or its an artifact from another pr?

@calixtus
Copy link
Copy Markdown
Member

calixtus commented Feb 9, 2025

Everything else looks good to go. Thanks for your great addition!

auto-merge was automatically disabled February 9, 2025 22:04

Head branch was pushed to by a user without write access

@Siedlerchr Siedlerchr added this pull request to the merge queue Feb 9, 2025
Merged via the queue into JabRef:main with commit 2178bc7 Feb 9, 2025
Comment on lines +7 to +10
@Override
public String toString() {
return "[" + message() + "]";
}
Copy link
Copy Markdown
Contributor Author

@priyanshu16095 priyanshu16095 Feb 11, 2025

Choose a reason for hiding this comment

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

This is redundant here, as records provide a built-in toString method.

The purpose of this method elsewhere in the codebase is when multiple fields are required, like "[ " + bibEntry.getCitationKey() + ", " + message.toString() + " ]". However, the message field itself already includes the citationKey.

@Override
public String toString() {
return "[" + entry().getCitationKey().orElse(entry().getAuthorTitleYear(50)) + "] in " + field.getDisplayName() + ": " + message();
}

In the follow up PR, should I also remove this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes

@koppor
Copy link
Copy Markdown
Member

koppor commented Feb 17, 2025

@priyanshu16095 Thank you for the work on this - would you feel comfortable to write a blog entry for this? (JabRef/blog.jabref.org#108) - I think, you have enough domain knowledge to explain?

In your screenshot, I saw that there is the column "Pages" filled with "O". That should not happen - if everything is the same (presence), then there should be no output - or is the logic different.

Finally, the logic of the consistency check maybe should ignore JabRef's fields (ranking in your example). That can be a follow-up. Maybe even configurable during the check itself?

@priyanshu16095
Copy link
Copy Markdown
Contributor Author

priyanshu16095 commented Feb 17, 2025

Sure! I'd be happy to write a blog entry for this.

I'll check why the "Pages" column is showing "o" and adjust the logic if needed.

Ignoring JabRef-specific fields in the consistency check sounds like a good idea. I'll definitely try it in a follow-up.

@koppor
Copy link
Copy Markdown
Member

koppor commented Feb 17, 2025

Sure! I'd be happy to write a blog entry for this.

Nice! In parallel, I would ask for a text for https://github.com/JabRef/user-documentation. I think, there is an overlap of 80% 😅 (Which is OK!). The blog won't evolve later, but the user documention 😅

@priyanshu16095
Copy link
Copy Markdown
Contributor Author

Understood! I’ll ensure the core content fits both needs.

@koppor
Copy link
Copy Markdown
Member

koppor commented Mar 11, 2025

Follow-up issue: #12695

@koppor
Copy link
Copy Markdown
Member

koppor commented Mar 11, 2025

Follow-up issue #12696

@koppor
Copy link
Copy Markdown
Member

koppor commented Mar 11, 2025

Follow-up issue: #12697

Sorry for the many follow-up isuses. I was so happy to see this in, but did not thouroughly test it.

@koppor
Copy link
Copy Markdown
Member

koppor commented Mar 12, 2025

Follow up: #12487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: consistency-check status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GUI for bibliography consistency check

6 participants