FriendsOfFlarum / polls

A Flarum extension that adds polls to your discussions
https://discuss.flarum.org/d/20586
MIT License
23 stars 12 forks source link

Removing a poll requires a page refresh #32

Closed imorland closed 3 years ago

imorland commented 3 years ago

When removing a poll, after selecting 'Remove poll', the poll remains visible in the post until a page refresh is performed

beta 15 / Polls 0.3.0

SKevo18 commented 3 years ago

I think that the last comma at line 53 shouldn't be there (?)

https://github.com/FriendsOfFlarum/polls/blob/fb10129d11589ca6e625cdffe25c34dfac013a99/src/Api/Controllers/DeletePollController.php#L51-L54

Not a PHP developer, but my forum reports error at line 54 (unexpected "(") Ref: https://discuss.flarum.org/d/20586-friendsofflarum-polls/112

dsevillamartin commented 3 years ago

@CWKevo The comma would cause a syntax error before Flarum loaded, that's not the issue I'm pretty sure.

It's probably caused by the DeletePollHandler

EDIT: just saw your ref, it might be syntax then, interesting

imorland commented 3 years ago

What PHP version do you use @CWKevo ?

For me, using 7.4.11, this is fine, but it may cause issues on (I think 7.2)

EvilExecutor commented 3 years ago

I couldn't reproduce this on localhost!?

EDIT: Issue persists, but no errors.

SKevo18 commented 3 years ago

We're using PHP 7.2, might be that

imorland commented 3 years ago

I've just pushed the removal of the extra ,

Are you able to try composer require fof/polls:dev-master to see if the error is resolved for you @CWKevo

SKevo18 commented 3 years ago

Yes, that fixed the issue, thank you I am not sure how this will perform on +7.2 however, but shouldn't cause issues?

imorland commented 3 years ago

Shouldn't make any difference tbh

SKevo18 commented 3 years ago

Shouldn't make any difference tbh

🤷‍♂️

We might look into updating PHP too, if that's the issue. But there's nothing after the comma, so what's there to continue? It's similar to JSON syntax, for example:

[
  {
    "key": "value", <--- This comma shouldn't be there
  }
]

I am not a PHP dev, but it did make a difference for some reason. Perhaps the syntax changed in PHP ^7.2, so it's less strict now about these kind of issues.

imorland commented 3 years ago

There are some performance gains to be had from PHP 7.4, that's for sure.

SKevo18 commented 3 years ago

Anyways, thank you for the quick fix :) I guess this can be closed now

imorland commented 3 years ago

You're welcome 👍 This will have to stay open in regards to the UI refresh side of the original issue...

EvilExecutor commented 3 years ago

Well, still the remove/refresh persists... oof Ian was quick 😂

SKevo18 commented 3 years ago

Well, not on my side 😂 I do not have to refresh it in order for the poll to be removed, it happens in real time. Strange, but I do not believe that 1 comma is doing all of this.

Just in case, here's php flarum info of our beta forum, I think that it might help:

Flarum core 0.1.0-beta.15
PHP version: 7.2.24-0ubuntu0.18.04.7
Loaded extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, pcntl, Reflection, SPL, sodium, session, standard, mysqlnd, PDO, xml, calendar, ctype, curl, dom, mbstring, fileinfo, ftp, gd, gettext, iconv, json, exif, mysqli, pdo_mysql, Phar, posix, readline, shmop, SimpleXML, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, wddx, xmlreader, xmlwriter, xsl, zip, Zend OPcache
+--------------------------------------+------------------+------------------------------------------+
| Flarum Extensions                    |                  |                                          |
+--------------------------------------+------------------+------------------------------------------+
| ID                                   | Version          | Commit                                   |
+--------------------------------------+------------------+------------------------------------------+
| flarum-approval                      | v0.1.0-beta.15   |                                          |
| flarum-bbcode                        | v0.1.0-beta.15   |                                          |
| flarum-emoji                         | v0.1.0-beta.15   |                                          |
| flarum-lang-english                  | v0.1.0-beta.15   |                                          |
| flarum-flags                         | v0.1.0-beta.15   |                                          |
| flarum-likes                         | v0.1.0-beta.15   |                                          |
| flarum-lock                          | v0.1.0-beta.15   |                                          |
| flarum-markdown                      | v0.1.0-beta.15   |                                          |
| flarum-mentions                      | v0.1.0-beta.15   |                                          |
| flarum-statistics                    | v0.1.0-beta.15   |                                          |
| flarum-sticky                        | v0.1.0-beta.15   |                                          |
| flarum-subscriptions                 | v0.1.0-beta.15   |                                          |
| flarum-suspend                       | v0.1.0-beta.15   |                                          |
| fof-sitemap                          | 0.6.0            |                                          |
| fof-default-user-preferences         | 0.3.1            |                                          |
| migratetoflarum-canonical            | 0.2.3            |                                          |
| customworld-lang-slovak              | v1.5.2           |                                          |
| fof-best-answer                      | 0.3.1            |                                          |
| fof-byobu                            | 0.6.0            |                                          |
| flarum-tags                          | v0.1.0-beta.15   |                                          |
| askvortsov-discussion-templates      | v0.3.0           |                                          |
| fof-drafts                           | 0.3.1            |                                          |
| fof-gamification                     | 0.4.0            |                                          |
| fof-polls                            | dev-master       | b6f01e59cdf53b1a26093714322a2cec68660aa7 |
| fof-prevent-necrobumping             | 0.4.0            |                                          |
| fof-reactions                        | 0.5.0            |                                          |
| clarkwinkelmann-create-user-modal    | 1.2.0            |                                          |
| clarkwinkelmann-first-post-approval  | 0.1.3            |                                          |
| fof-ban-ips                          | 0.4.0            |                                          |
| fof-default-group                    | 0.3.1            |                                          |
| fof-doorman                          | 0.3.0            |                                          |
| fof-filter                           | 0.3.0            |                                          |
| fof-merge-discussions                | 0.5.1            |                                          |
| fof-spamblock                        | 0.4.0            |                                          |
| fof-split                            | 0.6.0            |                                          |
| fof-stopforumspam                    | 0.4.0            |                                          |
| flarum-embed                         | v0.1.0-beta.15   |                                          |
| fof-analytics                        | 0.11.0           |                                          |
| fof-forum-statistics-widget          | 0.4.0            |                                          |
| fof-links                            | 0.5.0            |                                          |
| fof-nightmode                        | 0.7.1            |                                          |
| fof-pages                            | 0.6.0            |                                          |
| fof-profile-image-crop               | 0.2.1            |                                          |
| fof-secure-https                     | 0.3.0            |                                          |
| fof-pretty-mail                      | 0.3.0            |                                          |
| fof-share-social                     | 0.3.0            |                                          |
| fof-socialprofile                    | 0.2.0            |                                          |
| fof-terms                            | 0.6.0            |                                          |
| fof-transliterator                   | 0.3.0            |                                          |
| fof-upload                           | 0.12.0           |                                          |
| fof-user-bio                         | 0.4.0            |                                          |
| fof-user-directory                   | 0.5.0            |                                          |
| askvortsov-moderator-warnings        | v0.4.0           |                                          |
| flarum-nicknames                     | v0.1.0-beta.15   |                                          |
| sycho-profile-cover                  | v1.2.1           |                                          |
| v17development-seo                   | 1.5.1            |                                          |
| fof-bbcode-details                   | 0.2.1            |                                          |
| fof-formatting                       | 0.3.1            |                                          |
| neercsys-bosanski                    | v0.37            |                                          |
| neercsys-lang-bosanski               | v0.15            |                                          |
| madnest-lang-czech                   | v0.1.0-beta.14.1 |                                          |
| kakifrucht-de                        | 0.13.1           |                                          |
| askvortsov-categories                | v2.0.0           |                                          |
| kakifrucht-de-extended               | 0.1.3            |                                          |
| fof-linguist                         | 0.5.0            |                                          |
| qiaeru-lang-french                   | v1.8.5           |                                          |
| realodix-indonesian                  | 1.14.4           |                                          |
| rikusen0335-lang-japanese-extended   | v1.6.0           |                                          |
| menomenta-norwegian                  | 1.0.1            |                                          |
| persianfla-persian                   | v0.0.1           |                                          |
| rob006-lang-polish                   | v0.4.0           |                                          |
| bertaveira-pt-pt                     | v1.2.0           |                                          |
| darkfoxdeveloper-lang-spanish        | v1.0.8           |                                          |
| tolgaaaltas-lang-turkish             | 0.15             |                                          |
| tolgaaaltas-turkish                  | 0.15             |                                          |
| luatdolphin-lang-vietnamese          | 0.1.0-beta.14    |                                          |
| marketplace-l10n-core-russian        | 0.1.0-beta.13-2  |                                          |
| marketplace-l10n-ext-russian         | 0.1.6            |                                          |
| littlegolden-lang-japanese           | v0.1.67.1        |                                          |
| littlegolden-lang-simplified-chinese | v0.1.70-beta.15  |                                          |
| cryental-l10n-ext-korean             | 1.1              |                                          |
| michaelbelgium-discussion-views      | v5.0.0           |                                          |
| clarkwinkelmann-emojionearea         | 0.3.0            |                                          |
| fof-masquerade                       | 0.3.6            |                                          |
| nearata-lang-italian                 | v0.1.0-beta.14.3 |                                          |
| fof-oauth                            | 0.2.0            |                                          |
| matteocontrini-imgur-upload          | v3.3.1           |                                          |
| jslirola-login2seeplus               | v0.1.7           |                                          |
+--------------------------------------+------------------+------------------------------------------+
Base URL: https://beta.flarum.cloud
Installation path: /data/beta/app
Debug mode: off

Feel free to go there and test it, if you need

clarkwinkelmann commented 3 years ago

The reason the poll doesn't disappear from the first post upon removal is because the post subtree retrainer prevents all redraws.

This will be fixed in my PR #35 that adds a subtree check rule for Polls.

clarkwinkelmann commented 3 years ago

Fixed in versions 0.3.3 for beta 15 and 0.4.0 for beta 16