Fix duplicate external file type issue when change language and other corresponding issue of external file type preferences by papatekken · Pull Request #10496 · JabRef/jabref

papatekken · 2023-10-15T15:58:49Z

In Preferences->External file types,

fixes preferences: external file types duplicates #10271, which is about duplicate issue when changing the langauge
EditExternalFileTypeEntryDialog Cancel button close the dialog and wont take further action
Missing value in EditExternalFileTypeEntryDialog will block the closing dialog action when OK button clicked
New External File Type cannot saved if the new file extension exists already

*Notify message was added but no translation done.

Mandatory checks

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.
[x ] 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.

…tExternalFileTypeEntryDialog class

…empty

…ccessfully

Siedlerchr · 2023-10-16T15:46:16Z

There are failing tests
some are for l10n
see https://devdocs.jabref.org/code-howtos/localization.html

papatekken · 2023-10-16T22:30:18Z

thanks for your feedback, I will follow up soon later this week.

koppor · 2023-10-21T07:37:05Z

fixed duplicate issue when change langauge #10127

This issue is JabRef/JabRefOnline#2136 - I think, you linked the wrong number. Can you check?

koppor · 2023-10-21T07:38:11Z

fixed duplicate issue when change langauge #10127

This issue is JabRef/JabRefOnline#2136 - I think, you linked the wrong number. Can you check?

Ah, it is #10271 (not #10127 - numbers swapped) - I fixed it in the PR descriptoin, too.

koppor · 2023-10-21T07:40:13Z

@papatekken Would it be possible to add tests to ExternalFileTypesTabViewModel? With your changes, the ViewModel gets more complicated and should be testet.. - I know, this could be hard, but a good learning of mock frameworks.

koppor · 2023-10-21T07:42:23Z

OpenRewrite complains because of style - see the failing "Tests / Code style check" at https://github.com/JabRef/jabref/actions/runs/6593745520/job/17916664973?pr=10496.

Please run ./gradlew rewriteRun to rewrite your code - and check if the resulting code is "better"

papatekken · 2023-10-21T09:30:28Z

@koppor sorry for mix up the number.

I followed the documentation and tried checkStyle plugin locally but all passed.

Anyway I will try to run rewriteRun and see if that can help. I will add the test as you suggested too. Thanks for your comments.

koppor · 2023-10-21T09:48:50Z

I followed the documentation and tried checkStyle plugin locally but all passed.

No worries. -- We have some more steps in there. See

jabref/.github/workflows/tests.yml

Line 63 in 1fb17d3

- name: Run OpenRewrite

We did not mention that in the documentation. I am trying to get a bot commenting on the PR to guide more. See #10532.

Updating the documentation is future work :).

koppor · 2023-10-21T21:21:33Z

Time is currently shor tin our side. May I ask for a test of ExternalFileTypesTabViewModel (at least of the changes you did)?

papatekken · 2023-10-21T21:32:16Z

Time is currently shor tin our side. May I ask for a test of ExternalFileTypesTabViewModel (at least of the changes you did)?

Sure, I planned to do so, now I target to complete test before Monday hope this is ok, thanks

…elTestData class

koppor

Known file type added. I am not able to submit (which is OK). However, there should be an error indicator shown at the text field causing the issue. I think, this is difficult to implement, thus, OK for me.
If I edit an entry and re-use an existing mime-type (2.g., image/vnd.djvu). I can still submit. Mime types are globally unique. Thus, would it be possible to add a check for that?
If I edit an entry - and use an existing name (e.g., CHM), I can still submit. Thus, would it be possible to add a check for that?

Otherwise: Looks good to me!

koppor · 2023-12-11T14:20:55Z

+                    }
+                    return true;
+                },
+                ValidationMessage.error(Localization.lang("There is already an exists extension with the same name."))


Where do I see these messages? I did not see them here? @Siedlerchr Do we need to change something in the overall code?

@koppor
I following the code from GroupDialogViewModel.java.
In that class there is a function validationHandler. However even with that function I still cannot see the error message, so I didnt include that.

papatekken · 2023-12-12T01:30:52Z

koppor

No problem, I will add unique validation for name and mime-type as well. For error message, I am not sure at the moment.

…FileTypeView

koppor

Tried it out. Works good. I will ask @Siedlerchr about the validation messages.

Siedlerchr · 2023-12-16T16:51:48Z

I was so free to add the missing visualizaion implemention.
Basically you missed the initVisualization for the property/control. This also required me to combine the two validators forreach field, e.g. same name and isNotBlank

papatekken · 2023-12-17T01:09:29Z

I was so free to add the missing visualizaion implemention. Basically you missed the initVisualization for the property/control. This also required me to combine the two validators forreach field, e.g. same name and isNotBlank

thanks @Siedlerchr , I get it now, and it's good to learn something new

papatekken added 8 commits September 25, 2023 00:46

change name to extension comparison in fromString and toStringList

2a4d378

Merge branch 'JabRef:main' into fix-10127

5bb7c74

fix problem of abort button still add external file type entry in Edi…

7187b69

…tExternalFileTypeEntryDialog class

Merge branch 'JabRef:main' into fix-10127

cee577c

Block the Close action of EditExternalFileTypeEntryDialog when value …

341c52c

…empty

Merge branch 'JabRef:main' into fix-10127

76519e5

ExternalFileTypesTab only scroll to bottom when new fileType added su…

f7cc511

…ccessfully

check if external file type exists then cannot add a new one

f6166c0

papatekken changed the title ~~Fix 10127~~ Fix duplicate external file type issue when change language and other corresponding issue of external file type preferences Oct 15, 2023

papatekken added 2 commits October 19, 2023 01:40

Merge branch 'main' into fix-10127

df69445

update changelog.md

5fe6df9

papatekken force-pushed the fix-10127 branch from 0060bcc to 5fe6df9 Compare October 19, 2023 00:41

papatekken added 2 commits October 20, 2023 23:52

Merge branch 'JabRef:main' into fix-10127

e194bb9

reformat the new entry in CHANGELOG.md

f0b21d0

koppor mentioned this pull request Oct 21, 2023

Split Java code style checks #10532

Merged

6 tasks

koppor reviewed Oct 21, 2023

View reviewed changes

Comment thread src/main/java/org/jabref/gui/preferences/externalfiletypes/EditExternalFileTypeEntryDialog.java Outdated

ThiloteE added the component: preferences label Oct 22, 2023

papatekken added 3 commits October 22, 2023 14:17

Merge branch 'JabRef:main' into fix-10127

7ccf1f6

added ExternalFileTypesTabViewModelTest and run rewriteRun

a84d4da

updated the comment format

b5c1d16

papatekken added 7 commits December 11, 2023 00:39

change message File Extension exists already from info to debug

ca9eaeb

refactor ExternalFileTypesTabViewModelTest.java

eec134f

updated TestArchitectureTest for removing ExternalFileTypeItemViewMod…

73cf3d5

…elTestData class

extract methods for logics in isValidExternalFileType()

d983edd

add validator for name and MIME type

3fa21a3

code reformatting

59e8ff4

reformatting code

0a2bcf0

papatekken requested a review from koppor December 11, 2023 01:30

papatekken marked this pull request as ready for review December 11, 2023 01:31

update language key

275cf46

koppor requested changes Dec 11, 2023

View reviewed changes

papatekken marked this pull request as draft December 12, 2023 01:31

papatekken added 3 commits December 12, 2023 13:19

add unique name and mimetype validation for OK Button of EditExternal…

fd0835f

…FileTypeView

Merge branch 'main' into fix-10127

cb9210e

add message in JabRef_en.properties

134251d

papatekken marked this pull request as ready for review December 12, 2023 14:16

papatekken requested a review from koppor December 12, 2023 14:16

Merge branch 'main' into fix-10127

a9db8af

koppor reviewed Dec 15, 2023

View reviewed changes

add validation visualizaion and improve error message grammar

b4e8c21

Siedlerchr enabled auto-merge December 16, 2023 17:20

Siedlerchr approved these changes Dec 16, 2023

View reviewed changes

Siedlerchr disabled auto-merge December 17, 2023 12:55

Siedlerchr merged commit 4d86fb4 into JabRef:main Dec 17, 2023

papatekken deleted the fix-10127 branch February 1, 2024 10:01

This was referenced Dec 21, 2024

Fix method name #12322

Merged

Fix application not saved in external file type #12321

Merged

Uh oh!

Conversation

papatekken commented Oct 15, 2023 • edited by ThiloteE Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

Siedlerchr commented Oct 16, 2023

Uh oh!

papatekken commented Oct 16, 2023

Uh oh!

koppor commented Oct 21, 2023

Uh oh!

koppor commented Oct 21, 2023

Uh oh!

koppor commented Oct 21, 2023

Uh oh!

koppor commented Oct 21, 2023

Uh oh!

papatekken commented Oct 21, 2023

Uh oh!

koppor commented Oct 21, 2023

Uh oh!

koppor commented Oct 21, 2023

Uh oh!

papatekken commented Oct 21, 2023

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

koppor Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

papatekken Dec 12, 2023

Choose a reason for hiding this comment

Uh oh!

papatekken commented Dec 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Siedlerchr commented Dec 16, 2023

Uh oh!

papatekken commented Dec 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

papatekken commented Oct 15, 2023 •

edited by ThiloteE

Loading

papatekken commented Dec 12, 2023 •

edited

Loading

papatekken commented Dec 17, 2023 •

edited

Loading