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

🚨 Adding, editing, deleting options is broken (MANY BUGS!) 🚨 #1226

Closed
jonnymarshall opened this issue Jul 29, 2024 · 3 comments
Closed

Comments

@jonnymarshall
Copy link

jonnymarshall commented Jul 29, 2024

Describe the bug

  • BUG A: Editing timeslot options on polls duplicates the option (old polls only).
  • BUG B: Adding an option OR editing an option doesn't honour the defined time parameters (old and new polls).
  • BUG C: Deleting timeslot options may or may not have any effect (old polls only).

Video demonstration of the bugs here @lukevella


To Reproduce BUG A

  1. Go to an any poll (old or new)
  2. Click on manage > edit options
  3. Change the time on one of the options
  4. Click save

Expected behavior

The option should be edited.

Actual behavior

A new option is created (the previous option remains).


To Reproduce BUG B

  1. Go to an existing poll (possibly before latest update?)
  2. Click on manage > edit options
  3. Delete a time option
  4. Click save

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

  1. Go to an existing poll (possibly before latest update?)
  2. Click on manage > edit options
  3. Add an option or edit an existing option
  4. Click save

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

  • OS: Mac OS
  • Browser Chrome
  • Version 126.0.6478.183

Additional context

Bug may have been introduced with PR 1221?

@lukevella
Copy link
Owner

lukevella commented Jul 29, 2024

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 :)

@jonnymarshall
Copy link
Author

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!

@lukevella
Copy link
Owner

Closing this since the "fix" that caused these issue has been reverted.

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

2 participants