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

Problems during group renaming/changing in some cases #8824

Open
claell opened this issue May 18, 2022 · 31 comments
Open

Problems during group renaming/changing in some cases #8824

claell opened this issue May 18, 2022 · 31 comments
Labels
Milestone

Comments

@claell
Copy link
Contributor

claell commented May 18, 2022

Is your suggestion for improvement related to a problem? Please describe.
When I rename a group, I am asked whether I want all old entries to that group. However, the old group name remains assigned to this group.

Describe the solution you'd like
I had expected that the old group gets removed from the entries, at least an option for that should be provided.

Additional context

@ThiloteE
Copy link
Member

ThiloteE commented May 18, 2022

Edit: I now CAN Cannot reproduce. See #8824 (comment)

JabRef 5.7--2022-05-16--6a2332f
Linux 5.4.0-110-generic amd64
Java 18.0.1
JavaFX unknown

Maybe you have run into an index out of bounds issue and then the UI will depict weird stuff?
I would suggest to restart JabRef whenever you run into an index out of bounds issue.

@claell
Copy link
Contributor Author

claell commented May 18, 2022

Hm, interesting. Not sure whether that is what happened. I'll do some other tests, then.

@claell
Copy link
Contributor Author

claell commented May 18, 2022

Just tried again and couldn't reproduce, either. I think, I experienced this problem already before. So it was not the first time, I ran into it.

What do you mean with "the UI will depict weird stuff"? That did not happen to me. Possibly, I got an exception that I ignored. But I am not confident that it was an index out of bounds exception.

I'll leave this open for a few more days in case I run into this again. At least, it seems clear that this behavior is not normal (so no need for improvement).

@claell claell changed the title Improve handling of group renaming/changing Problems during group renaming/changing in some cases May 18, 2022
@ThiloteE
Copy link
Member

ThiloteE commented May 18, 2022

See e.g. issues #8012 and #8527 for "weird stuff"

  • 8012 will probably (hopefully) be fixed soon. There is a pull request in the pipeline
  • 8527 seems quite complicated and would require some more debugging I guess.

In general: As far as I understand it, if there is an out of bounds syntax error (which are somehow connected to the bi-directional binding), the UI will either depict an old state as compared to what you entered or it will depict a new state, but on disc there is still old data. So you can't trust what is shown on the UI. If you get one, restart JabRef, the best thing you can do.

Of course, right now, we don't know if it was an index out of bounds issue. I was just speculating.

@claell
Copy link
Contributor Author

claell commented May 18, 2022

From what you describe, that is not what happened to me.

I saw this looking at the .bib file itself. So the old category was still in there.

@ThiloteE
Copy link
Member

refs #8826

@ThiloteE
Copy link
Member

I assume you renamed groups by right clicking on a group and choosing "edit"?

@claell
Copy link
Contributor Author

claell commented May 19, 2022

Yes, that is how I renamed the group.

@ThiloteE ThiloteE added the bug Confirmed bugs or reports that are very likely to be bugs label May 21, 2022
@ThiloteE ThiloteE removed the bug Confirmed bugs or reports that are very likely to be bugs label May 21, 2022
@ThiloteE
Copy link
Member

ThiloteE commented Jun 28, 2022

I can now reproduce:

  1. Create group.

    • Name group 1.
    • Choose "Collect by" with Explicit selection.
    • Add entry to this group.
      Resulting data in {}biblatex source tab: groups = {1},
  2. Edit group. Change "Collect by" to Searching for a keyword. Example:
    grafik
    Add old entries to this group.
    Resulting data in {}biblatex source tab: groups = {1, 2},

Desired result:

groups = {2},

@ThiloteE
Copy link
Member

It also works the other way round:

Do the same thing, except

  1. first choose "Collect by" with Searching for a keyword
  2. then edit to "Collect by" with Explicit selection

@ThiloteE
Copy link
Member

@DohaRamadan, since you seem to be interested in UI / groups issues, and you already have gained some experience, I suppose this issue might be of interest. This is a very annyoing one, as it is the root cause of very unintuitive experiences.

@DohaRamadan
Copy link
Contributor

Thank you! i will check it out

@ThiloteE
Copy link
Member

ThiloteE commented Aug 31, 2023

This is an easy issue at first glance (e.g. just remove the old group), but it is a little bit more complicated than the others you have worked on, since we need to take into account situations where the entry is part of multiple groups e.g. via "explicit selection". For example:
image

Also, I hope we do not run into index out of bounds issues, if groups are automatically removed from entries.

@ThiloteE
Copy link
Member

A very closely related, but different issue is #8836. Reading through this bug might help you to better understand how the groups feature works and it's current limitations.

@DohaRamadan
Copy link
Contributor

I found out that there is another issue,
If I created a group and assigned an entry to it, and then deleted this group, the entry's groups are not updated and you can still see that the deleted group is still present in the entry's groups.
Is there an issue for this bug?

@DohaRamadan
Copy link
Contributor

Also, I'm having trouble understanding the whole edit group logic, shouldn't entries be assigned to the new group and removed from the old group always? why is there a dialog to assign entries to the new group when it's by intuition should always be assigned to the new group, and the removePreviousAssignments variable is vague too?

@ThiloteE
Copy link
Member

ThiloteE commented Sep 1, 2023

If I created a group and assigned an entry to it, and then deleted this group, the entry's groups are not updated and you can still see that the deleted group is still present in the entry's groups.
Is there an issue for this bug?

I don't think there is a specific issue for this apart from this issue here.

@ThiloteE
Copy link
Member

ThiloteE commented Sep 1, 2023

Also, I'm having trouble understanding the whole edit group logic, shouldn't entries be assigned to the new group and removed from the old group always? why is there a dialog to assign entries to the new group when it's by intuition should always be assigned to the new group, and the removePreviousAssignments variable is vague too?

If you ALWAYS remove it automatically from the group, you will also remove the entry from all other groups with this name.

So if you have a group hierarchy like this:

Bugs:

  • prio1
  • prio2

Birds:

  • prio1
  • prio2

and you delete the prio1 group from bugs, you will also delete the prio1 from birds, which is NOT what you would want. This could lead to unintentional dataloss and users will be confused too. We would fix one issue, but simply create another issue.

So we have to think hard how to solve this. The easy way out is to come up with a preference or clickable option during the ui popup that allows users to decide. The more complicated, but maybe better solution, would be to change the whole groups architecture.

@ThiloteE
Copy link
Member

ThiloteE commented Sep 1, 2023

(PS, I am not good at coding. Hoping for other maintainers to comment on the coding stuff)

@DohaRamadan
Copy link
Contributor

Also, I'm having trouble understanding the whole edit group logic, shouldn't entries be assigned to the new group and removed from the old group always? why is there a dialog to assign entries to the new group when it's by intuition should always be assigned to the new group, and the removePreviousAssignments variable is vague too?

If you ALWAYS remove it automatically from the group, you will also remove the entry from all other groups with this name.

So if you have a group hierarchy like this:

Bugs:

  • prio1
  • prio2

Birds:

  • prio1
  • prio2

and you delete the prio1 group from bugs, you will also delete the prio1 from birds, which is NOT what you would want. This could lead to unintentional dataloss and users will be confused too. We would fix one issue, but simply create another issue.

So we have to think hard how to solve this. The easy way out is to come up with a preference or clickable option during the ui popup that allows users to decide. The more complicated, but maybe better solution, would be to change the whole groups architecture.

I understand the problem better now, the group identifier is its name and that's the main problem in my opinion, there are 2 solutions to this
1- We impose a unique constraint on group names such that each group name must be unique.
2- We identify groups by a unique identifier (something like an integer called groupID) and when editing a group, we reference the old group and the new group by their groupID rather than their names.
I think solution 2 is better and best practice but I need a second opinion here.

@DohaRamadan
Copy link
Contributor

I want to work on this.
Also, maintainers's insight on my previous comment would be highly appreciated.

@tobiasdiez
Copy link
Member

@DohaRamadan This opens a very old box of the pandora, see #1495. I tend to agree that id-based serialization is a good idea, although it invalidates some of the advantages outlined at #629. With JabRef Online we will have the same issue to not be able to identify what group an entry actually belongs to.

@JabRef/developers please discuss this in the jabcon/next devcall

@ThiloteE
Copy link
Member

ThiloteE commented Sep 7, 2023

@DohaRamadan just so you know, the next regular devcall is in four days (Monday evening, every two weeks).

@koppor koppor added this to the v6.0 milestone Sep 11, 2023
@koppor
Copy link
Member

koppor commented Sep 11, 2023

Meta: We should write down a clean ADR (Folder: https://github.com/JabRef/jabref/tree/main/docs/decisions)

+1: For IDs.

ID stays same even if name changes.

ID technology:

We have to ensure that we do not simply rewrite the library, but ask for it. - Thus, #10370 is a pre-condition of the solution of this issue. And that issue also has a pre-condition

@tobiasdiez
Copy link
Member

In jabrefonline, I use cuid's.

@koppor
Copy link
Member

koppor commented Sep 12, 2023

Any pro/con list? I vote for CUID2, because if "Deterministic Length" in comparison to CUID1.

With length 10, CUID2 is short enough for me (remember the storage in plain text and that BibTeX users will check the file)

https://github.com/paralleldrive/cuid2#configuration

@tobiasdiez
Copy link
Member

Mostly because prisma doesn't yet support cuid2 by default (prisma/prisma#17102), but there are workarounds...so no hard requirement from my side.

@claell
Copy link
Contributor Author

claell commented Oct 14, 2023

Nice to see some activity on this.

Reading the discussion, I was thinking, whether some hybrid approach with both IDs and names might be interesting.

For storage, that would then allow users to read the group names in the BibTeX file. Additionally, it might be helpful for copying between libraries. In such cases, the current approach of IDs would probably generate new groups. With a name attached, one could try to match groups with the same name and open a dialog where the user can manually select whether the pasted entry groups should be merged with existing ones (of the same name) or a new group should be created.

@claell
Copy link
Contributor Author

claell commented Oct 14, 2023

For now, as it seems that the current implementation timeline for an ID-based system is pretty long (please correct me if this assumption is wrong): Might it be sensible to require unique group names for now as a quick solution that already fixes many problems?

@ThiloteE
Copy link
Member

Yes, the timeline seems to be long, as for all problems here, nothing is really broken. I realized the other day, that our groups feature is actually a "keywords/tags/labels" feature, so we could theoretically rename groups to tags and then introduce a "real" groups feature.

For all problems here, there is a (lenghty and uncomfortable) workaround by adjusting user workflows and giving groups unique names, so in my opinion, the priority is "I am looking forward to and it would be very nice to have a real groups feature, which I personally find kinda important, but urgency is medium/low, as workarounds exist". Also, this feature competes with other issues that are arguably even more important (e.g. CSL styles, finishing the new Search, fixing performance, JabRef online, Fixing Grobid, Fixing ISBN fetcher, etc.)

@claell
Copy link
Contributor Author

claell commented Oct 14, 2023

I think, we had the discussion about groups being more like keywords/tags/labels already some time ago :)

I'd argue that right now, there is quite something not working correctly (or as expected) for the group feature. It is not robust in any way.

But yes, if there are no resources for this, that's fine. I just thought about not blocking @DohaRamadan or others from improving the feature with postponing the point where it can be really improved unnecessarily into the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Normal priority
Development

No branches or pull requests

5 participants