Merge pull request #94 from birarda/atp-mappings

clearer error message handling

assignment-client/src/assets/AssetServer.cpp

@@ -464,7 +464,6 @@ void AssetServer::loadMappingsFromFile() {
                         shouldDrop = true;
                     }
 
-
                     if (!isValidHash(it.value().toString())) {
                         qWarning() << "Will not keep mapping for" << it.key() << "since it does not have a valid hash.";
                         shouldDrop = true;

interface/resources/qml/AssetServer.qml

@@ -21,7 +21,7 @@ import "dialogs"
 Window {
     id: root
     objectName: "AssetServer"
-    title: "Asset Server"
+    title: "My Asset Server"
     resizable: true
     destroyOnInvisible: true
     x: 40; y: 40
@@ -56,12 +56,13 @@ Window {
         Assets.deleteMappings(path, function(err) {
             if (err) {
                 console.log("Error deleting path: ", path, err);
-                errorMessage("There was an error deleting:\n" + path + "\n\nPlease try again.");
+
+                box = errorMessageBox("There was an error deleting:\n" + path + "\n" + Assets.getErrorString(err));
+                box.selected.connect(reload);
             } else {
                 console.log("Finished deleting path: ", path);
+                reload();
             }
-
-            reload();
         });
 
     }
@@ -76,7 +77,8 @@ Window {
         Assets.renameMapping(oldPath, newPath, function(err) {
             if (err) {
                 console.log("Error renaming: ", oldPath, "=>", newPath, " - error ", err);
-                errorMessage("There was an error renaming:\n" + oldPath + " to " + newPath + "\n\nPlease try again.");
+                box = errorMessageBox("There was an error renaming:\n" + oldPath + " to " + newPath + "\n" + Assets.getErrorString(err));
+                box.selected.connect(reload);
             } else {
                 console.log("Finished rename: ", oldPath, "=>", newPath);
             }
@@ -91,12 +93,11 @@ Window {
 
     function askForOverride(path, callback) {
         var object = desktop.messageBox({
-            icon: OriginalDialogs.StandardIcon.Question,
+            icon: hifi.icons.question,
             buttons: OriginalDialogs.StandardButton.Yes | OriginalDialogs.StandardButton.No,
-            defaultButton: OriginalDialogs.StandardButton.No,
+            defaultButton: OriginalDialogs.StandardButton.Yes,
-            text: "Override?",
+            title: "Overwrite File",
-            informativeText: "The following file already exists:\n" + path +
+            text: path + "\n" + "This file already exists. Do you want to overwrite it?"
-                             "\nDo you want to override it?"
         });
         object.selected.connect(function(button) {
             if (button === OriginalDialogs.StandardButton.Yes) {
@@ -119,29 +120,32 @@ Window {
         Assets.mappingModel.refresh();
     }
 
-    function handleGetMappingsError() {
+    function handleGetMappingsError(errorCode) {
-        errorMessage("There was a problem retreiving the list of assets from your Asset Server.\n"
+        errorMessageBox(
-                     + "Please make sure you are connected to the Asset Server and try again. ");
+            "There was a problem retreiving the list of assets from your Asset Server.\n"
+            + Assets.getErrorString(errorCode)
+        );
     }
 
     function addToWorld() {
-        var url = assetMappingsModel.data(treeView.currentIndex, 0x102);
+        var url = assetProxyModel.data(treeView.currentIndex, 0x103);
         if (!url) {
             return;
         }
-
+        var addPosition = Vec3.sum(MyAvatar.position, Vec3.multiply(2, Quat.getFront(MyAvatar.orientation)));
-        Entities.addModelEntity(url, MyAvatar.position);
+        Entities.addModelEntity(url, addPosition);
     }
 
-    function copyURLToClipboard(index) {
+    function copyURLToClipboard() {
         if (!index) {
             index = treeView.currentIndex;
         }
-        var path = assetProxyModel.data(index, 0x103);
+
-        if (!path) {
+        var url = assetProxyModel.data(treeView.currentIndex, 0x103);
+        if (!url) {
             return;
         }
-        Window.copyToClipboard(path);
+        Window.copyToClipboard(url);
     }
 
     function renameFile(index) {
@@ -184,13 +188,11 @@ Window {
         var typeString = isFolder ? 'folder' : 'file';
 
         var object = desktop.messageBox({
-            icon: OriginalDialogs.StandardIcon.Question,
+            icon: hifi.icons.question,
-            buttons: OriginalDialogs.StandardButton.Yes | OriginalDialogs.StandardButton.No,
+            buttons: OriginalDialogs.StandardButton.Yes + OriginalDialogs.StandardButton.No,
-            defaultButton: OriginalDialogs.StandardButton.No,
+            defaultButton: OriginalDialogs.StandardButton.Yes,
-            text: "Deleting",
+            title: "Delete",
-            informativeText: "You are about to delete the following " + typeString + ":\n" +
+            text: "You are about to delete the following " + typeString + ":\n" + path + "\nDo you want to continue?"
-                             path +
-                             "\nDo you want to continue?"
         });
         object.selected.connect(function(button) {
             if (button === OriginalDialogs.StandardButton.Yes) {
@@ -234,12 +236,12 @@ Window {
         });
     }
 
-    function errorMessage(message) {
+    function errorMessageBox(message) {
-        desktop.messageBox({
+        return desktop.messageBox({
-            icon: OriginalDialogs.StandardIcon.Error,
+            icon: hifi.icons.warning,
-            buttons: OriginalDialogs.StandardButton.Ok,
+            defaultButton: OriginalDialogs.StandardButton.Ok,
-            text: "Error",
+            title: "Error",
-            informativeText: message
+            text: message
         });
     }
 

interface/src/Menu.cpp

@@ -95,10 +95,6 @@ Menu::Menu() {
     addActionToQMenuAndActionHash(editMenu, MenuOption::RunningScripts, Qt::CTRL | Qt::Key_J,
         qApp, SLOT(toggleRunningScriptsWidget()));
 
-    // Edit > Asset Server
-    addActionToQMenuAndActionHash(editMenu, MenuOption::AssetServer, 0,
-        qApp, SLOT(toggleAssetServerWidget()));
-
     // Edit > Open and Run Script from File... [advanced]
     addActionToQMenuAndActionHash(editMenu, MenuOption::LoadScript, Qt::CTRL | Qt::Key_O,
         qApp, SLOT(loadDialog()),
@@ -131,10 +127,16 @@ Menu::Menu() {
         SLOT(toggleConsole()),
         QAction::NoRole, UNSPECIFIED_POSITION, "Advanced");
 
+    editMenu->addSeparator();
+
+    // Edit > My Asset Server
+    addActionToQMenuAndActionHash(editMenu, MenuOption::AssetServer, Qt::CTRL | Qt::SHIFT | Qt::Key_A,
+                                  qApp, SLOT(toggleAssetServerWidget()));
+
     // Edit > Reload All Content [advanced]
     addActionToQMenuAndActionHash(editMenu, MenuOption::ReloadContent, 0, qApp, SLOT(reloadResourceCaches()),
         QAction::NoRole, UNSPECIFIED_POSITION, "Advanced");
-
+    
 
     // Edit > Package Model... [advanced]
     addActionToQMenuAndActionHash(editMenu, MenuOption::PackageModel, 0,

interface/src/Menu.h

@@ -34,7 +34,7 @@ namespace MenuOption {
     const QString AnimDebugDrawDefaultPose = "Debug Draw Default Pose";
     const QString AnimDebugDrawPosition= "Debug Draw Position";
     const QString AssetMigration = "ATP Asset Migration";
-    const QString AssetServer = "Asset Server";
+    const QString AssetServer = "My Asset Server";
     const QString Attachments = "Attachments...";
     const QString AudioNetworkStats = "Audio Network Stats";
     const QString AudioNoiseReduction = "Audio Noise Reduction";

libraries/networking/src/AssetUtils.h

@@ -31,7 +31,7 @@ const size_t SHA256_HASH_LENGTH = 32;
 const size_t SHA256_HASH_HEX_LENGTH = 64;
 const uint64_t MAX_UPLOAD_SIZE = 1000 * 1000 * 1000; // 1GB
 
-const QString ASSET_PATH_REGEX_STRING = "^\\/(?:[^\\/]|\\/(?!\\/))*$";
+const QString ASSET_PATH_REGEX_STRING = "^\\/(?!\\/)(?:[^\\/]|\\/(?!\\/))*$";
 const QString ASSET_HASH_REGEX_STRING = QString("^[a-fA-F0-9]{%1}$").arg(SHA256_HASH_HEX_LENGTH);
 
 enum AssetServerError : uint8_t {

libraries/script-engine/src/AssetMappingsScriptingInterface.cpp

@@ -25,6 +25,27 @@ AssetMappingsScriptingInterface::AssetMappingsScriptingInterface() {
     _proxyModel.sort(0);
 }
 
+QString AssetMappingsScriptingInterface::getErrorString(int errorCode) const {
+    switch (errorCode) {
+        case MappingRequest::NoError:
+            return "No error";
+        case MappingRequest::NotFound:
+            return "Asset not found";
+        case MappingRequest::NetworkError:
+            return "Unable to communicate with Asset Server";
+        case MappingRequest::PermissionDenied:
+            return "Permission denied";
+        case MappingRequest::InvalidPath:
+            return "Path is invalid";
+        case MappingRequest::InvalidHash:
+            return "Hash is invalid";
+        case MappingRequest::UnknownError:
+            return "Asset Server internal error";
+        default:
+            return QString();
+    }
+}
+
 void AssetMappingsScriptingInterface::setMapping(QString path, QString hash, QJSValue callback) {
     auto assetClient = DependencyManager::get<AssetClient>();
     auto request = assetClient->createSetMappingRequest(path, hash);
@@ -113,22 +134,16 @@ void AssetMappingsScriptingInterface::renameMapping(QString oldPath, QString new
 }
 
 
-AssetMappingItem::AssetMappingItem(const QString& name, const QString& fullPath, bool isFolder)
+AssetMappingItem::AssetMappingItem(const QString& name, const QString& fullPath, bool isFolder) :
-    : name(name),
+    name(name),
     fullPath(fullPath),
-    isFolder(isFolder) {
+    isFolder(isFolder)
+{
 
 }
 
 static int assetMappingModelMetatypeId = qRegisterMetaType<AssetMappingModel*>("AssetMappingModel*");
 
-AssetMappingModel::AssetMappingModel() {
-}
-
-AssetMappingModel::~AssetMappingModel() {
-    qDebug() << " DEST";
-}
-
 void AssetMappingModel::refresh() {
     qDebug() << "Refreshing asset mapping model";
     auto assetClient = DependencyManager::get<AssetClient>();
@@ -221,7 +236,7 @@ void AssetMappingModel::refresh() {
                 //removeitem->index();
             }
         } else {
-            emit errorGettingMappings(uint8_t(request->getError()));
+            emit errorGettingMappings(static_cast<int>(request->getError()));
         }
     });
 

libraries/script-engine/src/AssetMappingsScriptingInterface.h

@@ -33,17 +33,12 @@
  class AssetMappingModel : public QStandardItemModel {
      Q_OBJECT
  public:
-     AssetMappingModel();
-     ~AssetMappingModel();
-
-//     QVariant AssetMappingModel::data(const QModelIndex& index, int role) const;
-
      Q_INVOKABLE void refresh();
 
      bool isKnownMapping(QString path) const { return _pathToItemMap.contains(path); };
 
  signals:
-     void errorGettingMappings(uint8_t error);
+     void errorGettingMappings(int error);
 
  private:
      QHash<QString, QStandardItem*> _pathToItemMap;
@@ -62,6 +57,8 @@ public:
 
     Q_INVOKABLE bool isKnownMapping(QString path) const { return _assetMappingModel.isKnownMapping(path); };
 
+    Q_INVOKABLE QString getErrorString(int errorCode) const;
+
     Q_INVOKABLE void setMapping(QString path, QString hash, QJSValue callback);
     Q_INVOKABLE void getMapping(QString path, QJSValue callback);
     Q_INVOKABLE void deleteMappings(QStringList paths, QJSValue callback);