-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Old polls cause web exceptions and broken appearance #30148
Comments
Hello @zulip/server-api members, this issue was labeled with the "area: api" label, so you may want to check it out! |
Hi @gnprice |
@zulipbot claim! |
Hello @Mukul1235! Thanks for your interest in Zulip! You have attempted to claim an issue without the label "help wanted". You can only claim and submit pull requests for issues with the help wanted label. If this is your first time here, we recommend reading our guide for new contributors before getting started. |
@N-Shar-ma can you fix this one? |
@gnprice Thank you for the detailed issue report! I've started a thread at https://chat.zulip.org/#narrow/stream/101-design/topic/Old.20polls.20cause.20web.20exceptions.20and.20broken.20appearance to discuss how to deal with these old broken polls. PS: Interestingly, new votes and options do work, they just need a reload |
I believe this was fixed through surgery on the chat.zulip.org database, but we should leave this open until we've decided what to do with the old |
To reproduce
Expected
No exceptions in the console.
The poll either appears accurately, or shows a clear error message saying it can't be shown.
Actual
The poll will appear but with no options or votes:
![image](https://private-user-images.githubusercontent.com/28173/332198540-6fb04f52-3fae-4eea-97d5-cb427dcc4649.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4Mjg1MDYsIm5iZiI6MTcxODgyODIwNiwicGF0aCI6Ii8yODE3My8zMzIxOTg1NDAtNmZiMDRmNTItM2ZhZS00ZWVhLTk3ZDUtY2I0MjdkY2M0NjQ5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA2MTklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNjE5VDIwMTY0NlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTMwMTkxOTBiMWZmZWMxM2FjYTFlZTcyYjY1OTc4ODA5YmI0NjQwNTM1NTdkNTgzMjhmMTY1OTc5OGQyYmYwZDgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.l6GUer1N1NrE9DtTsT6JVFvhpWwPrqI7L_WPbhk8IK0)
The JS console will show a burst of exceptions:
From those exceptions, I'm pretty sure the poll indeed had some options in it and some votes.
Discussion
It looks like these are from before Zulip Server 2.0, where we released the polls feature:
https://zulip.readthedocs.io/en/latest/overview/changelog.html#zulip-server-2-0-0
And in particular that
new_comment
name apparently was used before 2019-01, when it was renamed tonew_option
in c176891 .So what's happening is that we had an experimental API, sent messages on chat.zulip.org that used it, and they're now in the history but not handled by the current code.
It's probably fine to not support the old experimental API. Probably nobody outside chat.zulip.org was using it (at least I hope not); and I don't know how much we really used it outside tests, but even where we did, it's unlikely we care too much about those polls 5+ years later.
But if we don't have support for accurately rendering the message, we should tell the user that — we shouldn't just show something misleading.
(I noticed this because zulip-mobile has the same behavior, because it's using shared code. I'll make a note on the polls issue for zulip-flutter that we should reject polls that have this old experimental API.)
The text was updated successfully, but these errors were encountered: