-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
🚨 Adding, editing, deleting options is broken (MANY BUGS!) 🚨 #1226
Comments
Cheers @jonnymarshall. Appreciate you taking the time to walk through some of the issues. No doubt there's some new bugs introduced in that last change which is why I'm reverting it. The unfortunate truth is that the current implementation is flawed and will require a rewrite. This part of the codebase grew to accommodate more than it was designed to handle and trying to patch these issues is not worth the time it would take. One thing that sticks out to me from your recording is that you had a poll where multiple options had the same start time. I'm not sure if this served a specific purpose but this isn't really a use-case that is supported and it is a flaw that it is allowed. This would interfere when updating options since we compare start times to decide what options need to be removed and added. I'll be looking deeper in to this soon. Sorry for the inconvenience. Working on a solution for all this :) |
All good @lukevella , I understand the complexity creep involved in developing these kinds of projects, and agree that a revert probably seemed like the most logical temporary solution. As for the same timeslots in the polls you were seeing, these were only introduced due to the duplication bug I was demonstrating. It wasn't something I was doing deliberately, just a result of my testing. Good luck with the next build! |
Closing this since the "fix" that caused these issue has been reverted. |
Describe the bug
Video demonstration of the bugs here @lukevella
To Reproduce BUG A
Expected behavior
The option should be edited.
Actual behavior
A new option is created (the previous option remains).
To Reproduce BUG B
Expected behavior
The option should be deleted.
Actual behavior
The option may or may not be deleted (I haven't been able to identify a pattern/commonality for whether it is deleted or not. It could be that it applies to DAYS that already exist, but if you create a new day option then this bug is no longer observed. (Not 100% sure on this.)
To Reproduce BUG C
Expected behavior
The time of the added or edited option will be correct upon saving.
Actual behavior
The timezone of the new option when displayed in the poll (even when the timezone of the poll view page matches that of the edit page) will be incorrect. When you go back to edit it, the timezone on the edit page will have changed to match the timezone of the view page (presumably the timezone of your current location?)
The only way to fix it is to counter the timezone inconsistency. However if you do change it and it is an old poll, you also run into BUG A where the option is duplicated upon save.
Desktop
Additional context
Bug may have been introduced with PR 1221?
The text was updated successfully, but these errors were encountered: