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

Remove plot button removes all plots #457

Open
alexsb opened this issue Jan 29, 2025 · 6 comments
Open

Remove plot button removes all plots #457

alexsb opened this issue Jan 29, 2025 · 6 comments
Assignees

Comments

@alexsb
Copy link
Member

alexsb commented Jan 29, 2025

Only for the default movies state, clicking to remove 1 element view removes all 3.

If I add multiple plots removing it one by one works.

@NateLanza
Copy link
Contributor

Hey @alexsb this is something that @JakeWags and I noticed & discussed. It has to do with a now-fixed bug that added those 3 default plots with the same ID (the ID picker used Date.now() and I mistakenly copied that over without realizing that the UNIX timestamp would be the same for all 3!), so this bug only affects sessions that were created while that bug still existed. I looked into deduplicating the plot IDs as they come in from Multinet, but this is more involved than it first seems, so we decided not to implement that. That's the only fix for what you're observing, though if you create a fresh session out of a table, you shouldn't see the bug. Do you want me to put the time in to figure that out or should we close this?

@alexsb
Copy link
Member Author

alexsb commented Feb 1, 2025

Why not just create a new session? Or manually change the timestamp?

@NateLanza
Copy link
Contributor

The user decides when to create a new session and this does fix the bug; for sessions created after this fix was implemented, the ID is manually changed on plot creation so that the 3 default plots have different IDs. This bug only affects sessions that were created before this fix. So we can try to detect those sessions and update the ID, but that turned out to be more involved than I thought when I looked into it.

@alexsb
Copy link
Member Author

alexsb commented Feb 1, 2025

We can just create a new session; I'm talking about the ones we link from https://upset.multinet.app/

@NateLanza
Copy link
Contributor

Yes absolutely; sounds like this issue should be edited to replacing those sessions with fixed ones?

@alexsb
Copy link
Member Author

alexsb commented Feb 1, 2025

As you like, but the issue does describe the problem, not the solution :), so if you just update those states you can close it.

@NateLanza NateLanza assigned JakeWags and unassigned NateLanza Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants