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

Changing a specialFieldValue does not update the sorting of entries immediately #9334

Closed
2 tasks done
SebieF opened this issue Oct 31, 2022 · 3 comments · Fixed by #9338
Closed
2 tasks done

Changing a specialFieldValue does not update the sorting of entries immediately #9334

SebieF opened this issue Oct 31, 2022 · 3 comments · Fixed by #9338
Labels
bug Confirmed bugs or reports that are very likely to be bugs maintable

Comments

@SebieF
Copy link
Contributor

SebieF commented Oct 31, 2022

JabRef version

5.7 (latest release)

Operating system

GNU / Linux

Details on version and operating system

Ubuntu 22.04 LTS

Checked with the latest development build

  • I made a backup of my libraries before testing the latest development version.
  • I have tested the latest development version and the problem persists

Steps to reproduce the behaviour

  1. Create a new library and add at least two bib entries
  2. Sort your columns by ranking (highest ranking first)
  3. Set the ranking of the second entry to any rank: It should now be on top (as the other entry has a ranking of 0), but the position does only change after another change in the UI (e.g. setting the read status or setting a different rank value).

To be more precise, I inspected the code and found that the RankingFieldComparator always gets called with the old rank value, not the updated one. This is due to BibEntryTableViewModel which has both, the BibEntry entry and Map<SpecialField, OptionalBinding<SpecialFieldValueViewModel>> specialFieldValues attributes. The RankingFieldComparator is getting called with the value returned by public ObservableValue<Optional<SpecialFieldValueViewModel>> getSpecialField(SpecialField field) of the BibEntryTableViewModel object. This method returns the value from the specialFieldValues attribute, which, however, has not yet been updated as the comparator is called. The BibEntry entry field has in fact already been updated, which enables the following (dirty) workaround:

public ObservableValue<Optional<SpecialFieldValueViewModel>> getSpecialField(SpecialField field) {
        OptionalBinding<SpecialFieldValueViewModel> value = specialFieldValues.get(field);
        // Fetch already updated value from BibEntry entry
        Optional<String> currentValue = this.entry.getField(field);
        if (value != null) {
            // BEGIN WORKAROUND
            if(currentValue.isPresent()) {
                if (!value.getValue().get().getValue().getFieldValue().equals(currentValue)) {
                    value = getField(field).flatMapOpt(fieldValue -> field.parseValue(fieldValue).map(SpecialFieldValueViewModel::new));
                    specialFieldValues.put(field, value);
                    return value;
                }
            }
            // END WORKAROUND
            return value;
        } else {
            value = getField(field).flatMapOpt(fieldValue -> field.parseValue(fieldValue).map(SpecialFieldValueViewModel::new));
            specialFieldValues.put(field, value);
            return value;
        }
    }

I am new to JabRef, so I am not sure, why the extra specialFieldValues is actually needed, given that these values are already included in the BibEntry entry itself.

I like JabRef very much and am currently using it for my thesis, it is however a bit annoying to be forced to set the ranking twice by triggering the UI again. I would also be happy to create a PR for this myself, however, as already mentioned, I think that the "root cause" for the problem is nested a bit deeper, so more information would be necessary and much appreciated :)

@Siedlerchr
Copy link
Member

Thanks for your investigation! That is really helpful.
For reference, there were also other issues with setting the rank #9326 and #9031

@Siedlerchr Siedlerchr added the bug Confirmed bugs or reports that are very likely to be bugs label Oct 31, 2022
@SebieF
Copy link
Contributor Author

SebieF commented Nov 1, 2022

I looked into the rank-clearing issue a bit and updated the workaround to also work with zero-rankings:

public ObservableValue<Optional<SpecialFieldValueViewModel>> getSpecialField(SpecialField field) {
        OptionalBinding<SpecialFieldValueViewModel> value = specialFieldValues.get(field);
        // Fetch already updated value from BibEntry entry
        Optional<String> currentValue = this.entry.getField(field);
        if (value != null) {
            // BEGIN WORKAROUND
            if (currentValue.isEmpty() && value.getValue().isEmpty()) {
                var zeroValue = getField(field).flatMapOpt(fieldValue -> field.parseValue("CLEAR_RANK").map(SpecialFieldValueViewModel::new));
                specialFieldValues.put(field, zeroValue);
                return zeroValue;
            } else if (value.getValue().isEmpty() || !value.getValue().get().getValue().getFieldValue().equals(currentValue)) {
                value = getField(field).flatMapOpt(fieldValue -> field.parseValue(fieldValue).map(SpecialFieldValueViewModel::new));
                specialFieldValues.put(field, value);
                return value;
            }

            // END WORKAROUND
            return value;
        } else {
            value = getField(field).flatMapOpt(fieldValue -> field.parseValue(fieldValue).map(SpecialFieldValueViewModel::new));
            specialFieldValues.put(field, value);
            return value;
        }
    }

@Siedlerchr
Copy link
Member

@SebieF We would appreciate it if you could create a PR, I think your workaround is an acceptable solution. Better than nothing. The root cause is probably somewhere in the controlsfx control not having the option to remember the previous value

SebieF added a commit to SebieF/jabref that referenced this issue Nov 2, 2022
SebieF added a commit to SebieF/jabref that referenced this issue Nov 2, 2022
Siedlerchr pushed a commit that referenced this issue Nov 3, 2022
…9338)

* Updating specialFieldValues with possibly modified value from BibEntry

Workaround that is necessary because specialFieldValues and entry.fields get updated separately (entry.fields before speicalFieldValues). This lead to the behaviour that when sorting for a special field in the maintaible (like ranking or read status), the sorting does not get updated properly on change.

* Updating changelog with #9334

* Removing unnecessary comment

Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
@koppor koppor moved this to Done in Prioritization Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs maintable
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants