flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.29k stars 832 forks source link

Admin page's Save button won't reset if an error occurs #3962

Closed YUCLing closed 2 weeks ago

YUCLing commented 8 months ago

Current Behavior

While saving settings, some errors might occur, however, the state of the save button won't reset if an error occurs in saveSettings()

https://github.com/flarum/framework/blob/721e2eae3db2ca59116d17f7b001298d99418953/framework/core/js/src/admin/components/AdminPage.tsx#L412

This makes it not ideal to return a ValidationException in Saving events, nor for other cases where unexpected errors occur since the user must refresh the page to reset the state.

Steps to Reproduce

  1. Go to a settings page of an extension
  2. Click the save button (make sure an error response will be returned from the saving endpoint)

Expected Behavior

Save button is not in loading state after receiving the server response.

Screenshots

image

Environment

Output of php flarum info

Flarum core: 1.8.5
PHP version: 8.3.2
MySQL version: 11.2.2-MariaDB-1:11.2.2+maria~ubu2204
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, ctype, curl, dom, fileinfo, filter, hash, iconv, json, mbstring, SPL, session, PDO, pdo_sqlite, standard, posix, random, readline, Reflection, Phar, SimpleXML, tokenizer, xml, xmlreader, xmlwriter, mysqlnd, exif, gd, imagick, pdo_mysql, sodium
+--------------------------------+------------+------------------------------------------+
| Flarum Extensions              |            |                                          |
+--------------------------------+------------+------------------------------------------+
| ID                             | Version    | Commit                                   |
+--------------------------------+------------+------------------------------------------+
| flarum-flags                   | v1.8.0     |                                          |
| flarum-approval                | v1.8.1     |                                          |
| flarum-tags                    | v1.8.0     |                                          |
| rhd-ranks                      | dev-main   | 322bf563737f53e8468b61c07ce5ab17e434fc4c |
| rhd-extension                  | dev-main   | cd90d5caf040a27b97c3f2bc366ce5ce7e92aaf5 |
| flarum-suspend                 | v1.8.1     |                                          |
| flarum-subscriptions           | v1.8.0     |                                          |
| flarum-sticky                  | v1.8.0     |                                          |
| flarum-statistics              | v1.8.0     |                                          |
| flarum-mentions                | v1.8.3     |                                          |
| flarum-markdown                | v1.8.0     |                                          |
| flarum-lock                    | v1.8.0     |                                          |
| flarum-likes                   | v1.8.0     |                                          |
| flarum-lang-chinese-simplified | dev-master |                                          |
| flarum-emoji                   | v1.8.0     |                                          |
| flarum-bbcode                  | v1.8.0     |                                          |
+--------------------------------+------------+------------------------------------------+
Base URL: http://127.0.0.1:8181
Installation path: /var/www/web
Queue driver: sync
Session driver: file
Mail driver: mail
Debug mode: off

Possible Solution

Add a catch handler for saveSettings()'s promise and reset the save button state

Additional Context

No response

YUCLing commented 8 months ago

FYI: Just confirmed this also affects the validators.