From 174158aa63df89f49dee575625affc42a414a0f2 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 6 Oct 2019 18:59:13 +0200 Subject: [PATCH] Various UI small improvements (better error reporting in case of bad url, made the password change fields more explicit) --- .gitignore | 2 +- Makefile.am | 1 + approot/admin-database.xml | 12 ++-- approot/admin-user.xml | 6 +- approot/error.xml | 14 ++++ approot/messages.xml | 13 +++- approot/messages_fr.xml | 14 +++- approot/settings.xml | 4 +- src/Makefile.am | 1 + .../features/SimilarityFeaturesCache.cpp | 5 +- src/ui/LmsApplication.cpp | 68 +++++++++++++------ src/ui/LmsApplication.hpp | 9 +-- src/ui/LmsApplicationException.hpp | 59 ++++++++++++++++ src/ui/admin/UserView.cpp | 17 +++-- src/ui/explore/ArtistView.cpp | 6 +- src/ui/explore/ReleaseView.cpp | 6 +- src/utils/Exception.hpp | 2 +- 17 files changed, 182 insertions(+), 57 deletions(-) create mode 100644 approot/error.xml create mode 100644 src/ui/LmsApplicationException.hpp diff --git a/.gitignore b/.gitignore index d0e80c27..8a23214a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.sw? +*.in build/ Makefile.in aclocal.m4 @@ -7,4 +8,3 @@ configure depcomp install-sh missing -include/config.h.in diff --git a/Makefile.am b/Makefile.am index 2900892a..a82bd6d3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -36,6 +36,7 @@ dist_lms_approot_DATA = \ approot/artistlink.xml \ approot/artists.xml \ approot/artistsinfo.xml \ + approot/error.xml \ approot/explore.xml \ approot/login.xml \ approot/mediaplayer.xml \ diff --git a/approot/admin-database.xml b/approot/admin-database.xml index 51b9eeff..7674fab7 100644 --- a/approot/admin-database.xml +++ b/approot/admin-database.xml @@ -86,16 +86,20 @@ -
- ${last-scan} +
+
+ ${last-scan} +
-
- ${status} +
+
+ ${status} +
diff --git a/approot/admin-user.xml b/approot/admin-user.xml index ec00334a..f5d732cd 100644 --- a/approot/admin-user.xml +++ b/approot/admin-user.xml @@ -12,7 +12,9 @@ ${tr:Lms.Admin.User.last-login}
- ${last-login} +
+ ${last-login} +
${} @@ -31,7 +33,7 @@ ${}
${password} diff --git a/approot/error.xml b/approot/error.xml new file mode 100644 index 00000000..80657093 --- /dev/null +++ b/approot/error.xml @@ -0,0 +1,14 @@ + + + + +
+
+

${tr:Lms.Error.error-occurred}

+
+

${error}

+ ${btn-go-home class="btn btn-primary"} +
+ + + diff --git a/approot/messages.xml b/approot/messages.xml index 64a018fb..3e33bf4e 100644 --- a/approot/messages.xml +++ b/approot/messages.xml @@ -16,6 +16,7 @@ Bad login / password combination Login throttled, please try again later Confirm password +New password Old password Password too weak Passwords don't match @@ -23,9 +24,17 @@ Another session has been open. Reopen this one? Save + +Artist not found +Error occured! +Go home +Release not found +You are not allowed to perform this operation +User not found + Daily -Database +Music collection Scan now! Monthly Never @@ -134,8 +143,8 @@ -Account Audio +Change password Cannot save using a demo account! Bad password Old password must be filled in diff --git a/approot/messages_fr.xml b/approot/messages_fr.xml index 9b7b2ab5..50503410 100644 --- a/approot/messages_fr.xml +++ b/approot/messages_fr.xml @@ -16,6 +16,7 @@ Mauvaise combinaison login / mot de passe Trop de tentatives de connexion, veuillez réessayer plus tard Confirmation du mot de passe +Nouveau mot de passe Ancien mot de passe Mot de passe trop faible Les mots de passe ne correspondent pas @@ -23,9 +24,17 @@ Une autre session a été ouverte. Reouvrir celle-ci ? Sauvegarder + +Cet artiste n'existe pas +Une erreur est survenue! +Retour à l'accueil +Cet album n'existe pas +Vous n'avez pas les droits pour effectuer cette opération +L'utilisateur n'existe pas + Tous les jours -Base de données +Collection de musiques Scanner maintenant ! Tous les mois Jamais @@ -134,9 +143,8 @@ -Compte Audio -Auto +Changement de mot de passe Impossible de sauvegarder en utilisant un compte de démo ! Mauvais mot de passe L'ancien mot de passe doit être renseigné diff --git a/approot/settings.xml b/approot/settings.xml index 5a56a11e..964c8338 100644 --- a/approot/settings.xml +++ b/approot/settings.xml @@ -43,7 +43,7 @@
- ${tr:Lms.Settings.account} + ${tr:Lms.Settings.change-password}
${}
@@ -60,7 +60,7 @@ ${}
${password} diff --git a/src/Makefile.am b/src/Makefile.am index 6ce4c335..8121e124 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -82,6 +82,7 @@ lms_SOURCES = \ $(srcdir)/ui/Auth.hpp \ $(srcdir)/ui/LmsApplication.cpp \ $(srcdir)/ui/LmsApplication.hpp \ + $(srcdir)/ui/LmsApplicationException.hpp \ $(srcdir)/ui/LmsApplicationGroup.cpp \ $(srcdir)/ui/LmsApplicationGroup.hpp \ $(srcdir)/ui/MediaPlayer.cpp \ diff --git a/src/similarity/features/SimilarityFeaturesCache.cpp b/src/similarity/features/SimilarityFeaturesCache.cpp index 9cbbf261..4316af5e 100644 --- a/src/similarity/features/SimilarityFeaturesCache.cpp +++ b/src/similarity/features/SimilarityFeaturesCache.cpp @@ -92,8 +92,11 @@ networkToCacheFile(const SOM::Network& network, std::filesystem::path path) static std::optional -createNetworkFromCacheFile(std::filesystem::path path) +createNetworkFromCacheFile(const std::filesystem::path& path) { + if (!std::filesystem::exists(path)) + return std::nullopt; + try { LMS_LOG(SIMILARITY, INFO) << "Reading network from cache..."; diff --git a/src/ui/LmsApplication.cpp b/src/ui/LmsApplication.cpp index 1dc63910..a535d33e 100644 --- a/src/ui/LmsApplication.cpp +++ b/src/ui/LmsApplication.cpp @@ -48,6 +48,7 @@ #include "resource/ImageResource.hpp" #include "resource/AudioResource.hpp" #include "Auth.hpp" +#include "LmsApplicationException.hpp" #include "MediaPlayer.hpp" #include "PlayHistoryView.hpp" #include "PlayQueueView.hpp" @@ -134,6 +135,7 @@ LmsApplication::LmsApplication(const Wt::WEnvironment& env, messageResourceBundle().use(appRoot() + "artistlink"); messageResourceBundle().use(appRoot() + "artists"); messageResourceBundle().use(appRoot() + "artistsinfo"); + messageResourceBundle().use(appRoot() + "error"); messageResourceBundle().use(appRoot() + "explore"); messageResourceBundle().use(appRoot() + "login"); messageResourceBundle().use(appRoot() + "mediaplayer"); @@ -178,7 +180,20 @@ LmsApplication::LmsApplication(const Wt::WEnvironment& env, const auto userId {processAuthToken(env)}; if (userId) { - handleUserLoggedIn(*userId, false); + try + { + handleUserLoggedIn(*userId, false); + } + catch (LmsApplicationException& e) + { + LMS_LOG(UI, WARNING) << "Caught a LmsApplication exception: " << e.what(); + handleException(e); + } + catch (std::exception& e) + { + LMS_LOG(UI, ERROR) << "Caught exception: " << e.what(); + throw LmsException {"Internal error"}; // Do not put details here at it may appear on the user rendered html + } } else { @@ -267,9 +282,19 @@ LmsApplication::createCluster(Database::Cluster::pointer cluster, bool canDelete } void -LmsApplication::goHome() +LmsApplication::handleException(LmsApplicationException& e) { - setInternalPath("/artists", true); + root()->clear(); + Wt::WTemplate* t {root()->addNew(Wt::WString::tr("Lms.Error.template"))}; + t->addFunction("tr", &Wt::WTemplate::Functions::tr); + + t->bindString("error", e.what()); + Wt::WPushButton* btn {t->bindNew("btn-go-home", Wt::WString::tr("Lms.Error.go-home"))}; + btn->clicked().connect([this]() + { + setConfirmCloseMessage(""); + redirect("/"); + }); } void @@ -359,25 +384,17 @@ LmsApplication::handleUserLoggedIn(Database::IdType userId, bool strongAuth) root()->clear(); - try + const LmsApplicationInfo info {LmsApplicationInfo::fromEnvironment(environment())}; + + LMS_LOG(UI, INFO) << "User '" << getUserLoginName() << "' logged in from '" << environment().clientAddress() << "', user agent = " << environment().userAgent(); + getApplicationGroup().join(info); + + getApplicationGroup().postOthers([info] { - const LmsApplicationInfo info {LmsApplicationInfo::fromEnvironment(environment())}; + LmsApp->getEvents().appOpen(info); + }); - LMS_LOG(UI, INFO) << "User '" << getUserLoginName() << "' logged in from '" << environment().clientAddress() << "', user agent = " << environment().userAgent(); - getApplicationGroup().join(info); - - getApplicationGroup().postOthers([info] - { - LmsApp->getEvents().appOpen(info); - }); - - createHome(); - } - catch (std::exception& e) - { - LMS_LOG(UI, ERROR) << "Error while handling auth event: " << e.what(); - throw LmsException {"Internal error"}; // Do not put details here at it appears on the user rendered html - } + createHome(); } void @@ -581,10 +598,15 @@ LmsApplication::notify(const Wt::WEvent& event) { WApplication::notify(event); } + catch (LmsApplicationException& e) + { + LMS_LOG(UI, WARNING) << "Caught a LmsApplication exception: " << e.what(); + handleException(e); + } catch (std::exception& e) { LMS_LOG(UI, ERROR) << "Caught exception: " << e.what(); - throw LmsException("Internal error"); // Do not put details here at it appears on the user rendered html + throw LmsException {"Internal error"}; // Do not put details here at it may appear on the user rendered html } } @@ -606,7 +628,9 @@ LmsApplication::post(std::function func) Wt::WServer::instance()->post(LmsApp->sessionId(), std::move(func)); } -static std::string escape(std::string str) +static +std::string +escape(const std::string& str) { return replaceInString(std::move(str), "\'", "\\\'"); } diff --git a/src/ui/LmsApplication.hpp b/src/ui/LmsApplication.hpp index 2e3f52c4..a677ff50 100644 --- a/src/ui/LmsApplication.hpp +++ b/src/ui/LmsApplication.hpp @@ -38,8 +38,9 @@ namespace Database { namespace UserInterface { class AudioResource; -class ImageResource; class Auth; +class ImageResource; +class LmsApplicationException; // Events that can be listen from anywhere in the application struct Events @@ -91,9 +92,6 @@ class LmsApplication : public Wt::WApplication Events& getEvents() { return _events; } // Utils - void goHome(); - void goHomeAndQuit(); - void post(std::function func); void notifyMsg(MsgType type, const Wt::WString& message, std::chrono::milliseconds duration = std::chrono::milliseconds(4000)); @@ -108,6 +106,9 @@ class LmsApplication : public Wt::WApplication private: + void handleException(LmsApplicationException& e); + void goHomeAndQuit(); + LmsApplicationGroup& getApplicationGroup(); // Signal slots diff --git a/src/ui/LmsApplicationException.hpp b/src/ui/LmsApplicationException.hpp new file mode 100644 index 00000000..61ef2dbd --- /dev/null +++ b/src/ui/LmsApplicationException.hpp @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2019 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#pragma once + +#include "database/Types.hpp" +#include "utils/Exception.hpp" + +namespace UserInterface { + + +class LmsApplicationException : public LmsException +{ + public: + LmsApplicationException(const Wt::WString& error) : LmsException {error.toUTF8()} {} +}; + +class ArtistNotFoundException : public LmsApplicationException +{ + public: + ArtistNotFoundException(Database::IdType artistId) : LmsApplicationException {Wt::WString::tr("Lms.Error.artist-not-found").arg(artistId)} {} +}; + +class ReleaseNotFoundException : public LmsApplicationException +{ + public: + ReleaseNotFoundException(Database::IdType releaseId) : LmsApplicationException {Wt::WString::tr("Lms.Error.release-not-found").arg(releaseId)} {} +}; + +class UserNotFoundException : public LmsApplicationException +{ + public: + UserNotFoundException(Database::IdType userId) : LmsApplicationException {Wt::WString::tr("Lms.Error.user-not-found").arg(userId)} {} +}; + +class UserNotAllowedException : public LmsApplicationException +{ + public: + UserNotAllowedException() : LmsApplicationException {Wt::WString::tr("Lms.Error.user-not-allowed")} {} +}; + +} + diff --git a/src/ui/admin/UserView.cpp b/src/ui/admin/UserView.cpp index 7b8b6b5f..d08c2f83 100644 --- a/src/ui/admin/UserView.cpp +++ b/src/ui/admin/UserView.cpp @@ -39,6 +39,7 @@ #include "common/Validators.hpp" #include "common/ValueStringModel.hpp" #include "LmsApplication.hpp" +#include "LmsApplicationException.hpp" namespace UserInterface { @@ -54,8 +55,7 @@ class UserModel : public Wt::WFormModel static const Field DemoField; UserModel(std::optional userId) - : Wt::WFormModel(), - _userId(userId) + : _userId {userId} { if (!_userId) { @@ -126,8 +126,10 @@ class UserModel : public Wt::WFormModel auto transaction {LmsApp->getDbSession().createSharedTransaction()}; const Database::User::pointer user {Database::User::getById(LmsApp->getDbSession(), *_userId)}; - if (user == LmsApp->getUser()) - throw LmsException("Cannot edit ourselves"); + if (!user) + throw UserNotFoundException {*_userId}; + else if (user == LmsApp->getUser()) + throw UserNotAllowedException {}; auto transcodeBitrateLimitRow {_bitrateModel->getRowFromValue(user->getMaxAudioTranscodeBitrate())}; if (transcodeBitrateLimitRow) @@ -240,12 +242,13 @@ UserView::refreshView() auto transaction {LmsApp->getDbSession().createSharedTransaction()}; const Database::User::pointer user {Database::User::getById(LmsApp->getDbSession(), *userId)}; + if (!user) + throw UserNotFoundException {*userId}; + t->bindString("title", Wt::WString::tr("Lms.Admin.User.user-edit").arg(user->getLoginName()), Wt::TextFormat::Plain); t->setCondition("if-has-last-login", true); - Wt::WLineEdit *lastLogin {t->bindNew("last-login")}; - lastLogin->setText(user->getLastLogin().toString()); - lastLogin->setEnabled(false); + t->bindString("last-login", user->getLastLogin().toString(), Wt::TextFormat::Plain); } else { diff --git a/src/ui/explore/ArtistView.cpp b/src/ui/explore/ArtistView.cpp index f77373cb..35837d72 100644 --- a/src/ui/explore/ArtistView.cpp +++ b/src/ui/explore/ArtistView.cpp @@ -33,6 +33,7 @@ #include "resource/ImageResource.hpp" #include "LmsApplication.hpp" +#include "LmsApplicationException.hpp" #include "Filters.hpp" using namespace Database; @@ -70,10 +71,7 @@ Artist::refresh() const Database::Artist::pointer artist = Database::Artist::getById(LmsApp->getDbSession(), *artistId); if (!artist) - { - LmsApp->goHome(); - return; - } + throw ArtistNotFoundException {*artistId}; Wt::WTemplate* t {addNew(Wt::WString::tr("Lms.Explore.Artist.template"))}; t->addFunction("tr", &Wt::WTemplate::Functions::tr); diff --git a/src/ui/explore/ReleaseView.cpp b/src/ui/explore/ReleaseView.cpp index 114ef34e..ba6a868a 100644 --- a/src/ui/explore/ReleaseView.cpp +++ b/src/ui/explore/ReleaseView.cpp @@ -34,6 +34,7 @@ #include "resource/ImageResource.hpp" #include "LmsApplication.hpp" +#include "LmsApplicationException.hpp" #include "Filters.hpp" using namespace Database; @@ -71,10 +72,7 @@ Release::refresh() const Database::Release::pointer release {Database::Release::getById(LmsApp->getDbSession(), *releaseId)}; if (!release) - { - LmsApp->goHome(); - return; - } + throw ReleaseNotFoundException {*releaseId}; Wt::WTemplate* t {addNew(Wt::WString::tr("Lms.Explore.Release.template"))}; t->addFunction("tr", &Wt::WTemplate::Functions::tr); diff --git a/src/utils/Exception.hpp b/src/utils/Exception.hpp index eda418d9..1d238809 100644 --- a/src/utils/Exception.hpp +++ b/src/utils/Exception.hpp @@ -25,6 +25,6 @@ class LmsException : public std::runtime_error { public: - LmsException(const std::string& error) : std::runtime_error(error) {} + LmsException(const std::string& error = "") : std::runtime_error {error} {} };