Skip to content

Fix library sort order get lost#6379

Merged
koppor merged 1 commit into
JabRef:masterfrom
hrandrianasolo:fix-for-issue-6091
May 12, 2020
Merged

Fix library sort order get lost#6379
koppor merged 1 commit into
JabRef:masterfrom
hrandrianasolo:fix-for-issue-6091

Conversation

@hrandrianasolo

@hrandrianasolo hrandrianasolo commented Apr 30, 2020

Copy link
Copy Markdown
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.

@hrandrianasolo @Hugoo98 @micipsa1998
Fix library sort order get lost
d0f6d06 
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

Siedlerchr commented Apr 30, 2020

Copy link
Copy Markdown
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

Siedlerchr commented May 2, 2020

Copy link
Copy Markdown
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

Hugoo98 commented May 9, 2020

Copy link
Copy Markdown

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

Hugoo98 commented May 9, 2020

Copy link
Copy Markdown

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

hrandrianasolo commented May 10, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Siedlerchr commented May 10, 2020

Copy link
Copy Markdown
Member

Thanks for the explanation. LGTM

@koppor

koppor commented May 12, 2020

Copy link
Copy Markdown
Member

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
@calixtus
Added CHANGELOG.md entry for #6379
a8cf628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Library sort order get lost

5 participants

@hrandrianasolo @Siedlerchr @Hugoo98 @koppor @calixtus