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

Fix library sort order get lost #6379

Merged
merged 1 commit into from
May 12, 2020

Conversation

hrandrianasolo
Copy link
Contributor

This PR fixes #6091

Co-authored-by: Heriniaina Randrianasolo hjsrandrianasolo@gmail.com
Co-authored-by: Omar Tachour omar.tchr@hotmail.com
Co-authored-by: Sadji Micipsa micipsasadji@gmail.com

This error is due to the absence of an object equality test for the saveInSpecifiedOrder attribute.
So we have handled this error by correcting the equals function in the SaveOrderConfig class.

  • Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

fixes JabRef#6091

Co-authored-by: Heriniaina Randrianasolo <hjsrandrianasolo@gmail.com>
Co-authored-by: Omar Tachour <omar.tchr@hotmail.com>
Co-authored-by: Sadji Micipsa <micipsasadji@gmail.com>
@Siedlerchr
Copy link
Member

Your changes look good to me, but I'm currently trying to understand why this fixes the error. Could you please explain that in a bit more detail?

@Siedlerchr
Copy link
Member

@hrandrianasolo I tested your changes and they don't seem to make a difference. So could you please explain it

@calixtus calixtus added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label May 8, 2020
@Siedlerchr Siedlerchr closed this May 8, 2020
@Siedlerchr Siedlerchr reopened this May 8, 2020
@Siedlerchr Siedlerchr closed this May 8, 2020
@Hugoo98
Copy link

Hugoo98 commented May 9, 2020

hi @Siedlerchr
we apologize for the delay, I am the colleague of @hrandrianasolo, the bug correction works very well on a version which is not up to date.
We have well compiled and executed the modified code and we found that the bug is corrected, there I am trying to test it on the new version.
Regarding the explanation, the configuration of the export sort order is represented by the class SaveOrderConfig, in LibraryPropertiesDialogView there are two objects of type SaveOrderConfig:
→ oldSaveOrderConfig: represents the old configuration,
→ newSaveOrderConfig: represents the new configuration.
We compare the equality of its last two to see if a change has been made or not. After several tests, we realized that the equals function in saveOrderConfig did not take into account the equality of all attributes.

@Hugoo98
Copy link

Hugoo98 commented May 9, 2020

Hi @Siedlerchr I inform you that I tried our correction on today's version it also worked !!

@Siedlerchr Siedlerchr reopened this May 10, 2020
@hrandrianasolo
Copy link
Contributor Author

hrandrianasolo commented May 10, 2020

Hi, everyone, sorry for the delay. As my colleague explained above regarding the explanation of the fix, I put here screenshots of the bug fix instead of the screen recorder because I can't put it in the comments.

Sélection_054
Sélection_055
Sélection_056

@Siedlerchr Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: waiting-for-feedback The submitter or other users need to provide more information about the issue labels May 10, 2020
@Siedlerchr
Copy link
Member

Thanks for the explanation. LGTM

@koppor
Copy link
Member

koppor commented May 12, 2020

We merge to get things moving forward.

We will add the CHANGELOG.md change by ourselves.

@koppor koppor merged commit 993e8f9 into JabRef:master May 12, 2020
calixtus added a commit that referenced this pull request May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Library sort order get lost
5 participants