From 2c416c7e27c97abea9e061c9a8e75f30ccf72bd0 Mon Sep 17 00:00:00 2001
From: Agustin Moyano <agustinmoyano@theke.io>
Date: Thu, 27 Aug 2020 19:09:05 -0300
Subject: [PATCH] Bug 23019: (follow-up) Fix usability issues

This patch place profile buttons in the bottom of the page, next to "Stage for import"

It removes the "Update profile" button, and changes "Add profile" to "Save profile". Save profile can be used to add and to update a profile.

It also adds feedback when a profile was saved or deleted.

Last, it also replaces input-batch-profile endpoint name for input_batch_profile, which is more aligned with other endpoint names.
---
 api/v1/swagger/definitions.json               |   8 +-
 .../definitions/import-batch-profiles.json    |   6 -
 ...profile.json => import_batch_profile.json} |   0
 .../definitions/import_batch_profiles.json    |   6 +
 api/v1/swagger/parameters.json                |   2 +-
 ...profile.json => import_batch_profile.json} |   0
 api/v1/swagger/paths.json                     |   8 +-
 ...ofiles.json => import_batch_profiles.json} |  10 +-
 .../en/modules/tools/stage-marc-import.tt     | 105 ++++--------------
 9 files changed, 42 insertions(+), 103 deletions(-)
 delete mode 100644 api/v1/swagger/definitions/import-batch-profiles.json
 rename api/v1/swagger/definitions/{import-batch-profile.json => import_batch_profile.json} (100%)
 create mode 100644 api/v1/swagger/definitions/import_batch_profiles.json
 rename api/v1/swagger/parameters/{import-batch-profile.json => import_batch_profile.json} (100%)
 rename api/v1/swagger/paths/{import-batch-profiles.json => import_batch_profiles.json} (97%)

diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json
index ac092785b2..e35054a456 100644
--- a/api/v1/swagger/definitions.json
+++ b/api/v1/swagger/definitions.json
@@ -38,11 +38,11 @@
   "ill_backend": {
     "$ref": "definitions/ill_backend.json"
   },
-  "import-batch-profile": {
-    "$ref": "definitions/import-batch-profile.json"
+  "import_batch_profile": {
+    "$ref": "definitions/import_batch_profile.json"
   },
-  "import-batch-profiles": {
-    "$ref": "definitions/import-batch-profiles.json"
+  "import_batch_profiles": {
+    "$ref": "definitions/import_batch_profiles.json"
   },
   "library": {
     "$ref": "definitions/library.json"
diff --git a/api/v1/swagger/definitions/import-batch-profiles.json b/api/v1/swagger/definitions/import-batch-profiles.json
deleted file mode 100644
index 8237f5c39c..0000000000
--- a/api/v1/swagger/definitions/import-batch-profiles.json
+++ /dev/null
@@ -1,6 +0,0 @@
-{
-    "type": "array",
-    "items": {
-      "$ref": "import-batch-profile.json"
-    }
-  }
\ No newline at end of file
diff --git a/api/v1/swagger/definitions/import-batch-profile.json b/api/v1/swagger/definitions/import_batch_profile.json
similarity index 100%
rename from api/v1/swagger/definitions/import-batch-profile.json
rename to api/v1/swagger/definitions/import_batch_profile.json
diff --git a/api/v1/swagger/definitions/import_batch_profiles.json b/api/v1/swagger/definitions/import_batch_profiles.json
new file mode 100644
index 0000000000..4c2b17b232
--- /dev/null
+++ b/api/v1/swagger/definitions/import_batch_profiles.json
@@ -0,0 +1,6 @@
+{
+    "type": "array",
+    "items": {
+      "$ref": "import_batch_profile.json"
+    }
+  }
\ No newline at end of file
diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json
index 7eecf8207b..97a3a5550c 100644
--- a/api/v1/swagger/parameters.json
+++ b/api/v1/swagger/parameters.json
@@ -12,7 +12,7 @@
     "$ref": "parameters/patron.json#/patron_id_qp"
   },
   "profile_id_pp": {
-    "$ref": "parameters/import-batch-profile.json#/profile_id_pp"
+    "$ref": "parameters/import_batch_profile.json#/profile_id_pp"
   },
   "city_id_pp": {
     "$ref": "parameters/city.json#/city_id_pp"
diff --git a/api/v1/swagger/parameters/import-batch-profile.json b/api/v1/swagger/parameters/import_batch_profile.json
similarity index 100%
rename from api/v1/swagger/parameters/import-batch-profile.json
rename to api/v1/swagger/parameters/import_batch_profile.json
diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json
index b6b82ee103..ebe8a0bf8a 100644
--- a/api/v1/swagger/paths.json
+++ b/api/v1/swagger/paths.json
@@ -110,11 +110,11 @@
   "/illrequests": {
     "$ref": "paths/illrequests.json#/~1illrequests"
   },
-  "/import-batch-profiles": {
-    "$ref": "paths/import-batch-profiles.json#/~1import-batch-profiles"
+  "/import_batch_profiles": {
+    "$ref": "paths/import_batch_profiles.json#/~1import_batch_profiles"
   },
-  "/import-batch-profiles/{profile_id}": {
-    "$ref": "paths/import-batch-profiles.json#/~1import-batch-profiles~1{profile_id}"
+  "/import_batch_profiles/{profile_id}": {
+    "$ref": "paths/import_batch_profiles.json#/~1import_batch_profiles~1{profile_id}"
   },
   "/rotas/{rota_id}/stages/{stage_id}/position": {
     "$ref": "paths/rotas.json#/~1rotas~1{rota_id}~1stages~1{stage_id}~1position"
diff --git a/api/v1/swagger/paths/import-batch-profiles.json b/api/v1/swagger/paths/import_batch_profiles.json
similarity index 97%
rename from api/v1/swagger/paths/import-batch-profiles.json
rename to api/v1/swagger/paths/import_batch_profiles.json
index 0500f36dfe..0e5860c67b 100644
--- a/api/v1/swagger/paths/import-batch-profiles.json
+++ b/api/v1/swagger/paths/import_batch_profiles.json
@@ -1,5 +1,5 @@
 {
-  "/import-batch-profiles": {
+  "/import_batch_profiles": {
     "get": {
       "x-mojo-to": "ImportBatchProfiles#list",
       "operationId": "listImportBatchProfiles",
@@ -37,7 +37,7 @@
         "200": {
           "description": "A list of import batch profiles",
           "schema": {
-            "$ref": "../definitions.json#/import-batch-profiles"
+            "$ref": "../definitions.json#/import_batch_profiles"
           }
         },
         "401": {
@@ -140,7 +140,7 @@
         "201": {
           "description": "Created Profile",
           "schema": {
-            "$ref": "../definitions.json#/import-batch-profile"
+            "$ref": "../definitions.json#/import_batch_profile"
           }
         },
         "400": {
@@ -187,7 +187,7 @@
       }
     }
   },
-  "/import-batch-profiles/{profile_id}": {
+  "/import_batch_profiles/{profile_id}": {
     "put": {
       "x-mojo-to": "ImportBatchProfiles#edit",
       "operationId": "editImportBatchProfiles",
@@ -260,7 +260,7 @@
         "200": {
           "description": "Updated profile",
           "schema": {
-            "$ref": "../definitions.json#/import-batch-profile"
+            "$ref": "../definitions.json#/import_batch_profile"
           }
         },
         "400": {
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt
index e6ed1dd4e6..f3f03c47c9 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt
@@ -103,18 +103,10 @@
             <select name="profile" id="profile">
                 <option value="">Do not use profile</option>
             </select>
-        </li>
-        <li>
-            <label for="profile_name">Profile name</label>
-            <input type="text" id="profile_name" name="profile_name" />
-
+            <div class="hint">When you select a profile it pre-fills your form with profile values.</div>
+            <div class="hint">Later you can modify your form and that's what matters on import.</div>
         </li>
     </ol>
-    <fieldset class="action">
-        <button id="add_profile" disabled>Add profile</button>
-        <button id="mod_profile" disabled>Update profile</button>
-        <button id="del_profile" disabled>Remove profile</button>
-    </fieldset>
 </fieldset>
 
     <form method="post" id="processfile" action="[% SCRIPT_NAME | html %]" enctype="multipart/form-data">
@@ -216,7 +208,12 @@
       </li>
     </ol>
   </fieldset>
-  <fieldset class="action"><input type="button" id="mainformsubmit" value="Stage for import" /></fieldset>
+  <fieldset class="action">
+    <input type="button" id="mainformsubmit" value="Stage for import" />
+    <button id="add_profile" disabled>Save profile</button>
+    <input type="text" id="profile_name" name="profile_name" placeholder="Profile name"/>
+    <button id="del_profile" disabled>Remove profile</button>
+  </fieldset>
  
        <div id="jobpanel"><div id="jobstatus" class="progress_panel">Job progress: <div id="jobprogress"></div> <span id="jobprogresspercent">0</span>%</div>
      <div id="jobfailed"></div></div>
@@ -241,6 +238,8 @@
     [% Asset.js("js/file-upload.js") | $raw %]
     <script>
         var xhr;
+        var PROFILE_SAVE_MSG = _("Profile saved");
+        var PROFILE_DEL_MSG = _("Profile deleted");
         $(document).ready(function(){
             $("#processfile").hide();
             $('#profile_fieldset').hide();
@@ -298,10 +297,10 @@
 
             $("#profile_name").keyup(function(){
                 $("#add_profile").prop("disabled", this.value.trim()=='');
-                $("#mod_profile").prop("disabled", this.value.trim()=='' || !$("#profile").val())
             });
 
-            $("#add_profile").click(function() {
+            $("#add_profile").click(function(event) {
+                event.preventDefault();
                 var name = $("#profile_name").val().trim();
                 if(!name) {
                     alert(_("Profile must have a name"));
@@ -317,7 +316,7 @@
                     });
 
                 if(profile.length) {
-                    if(!confirm(_("There is another profile with this name.")+"\n\n"+_("Do you want to replace it?"))) {
+                    if(!confirm(_("There is another profile with this name.")+"\n\n"+_("Do you want to update it with new values?"))) {
                         return;
                     }
                 }
@@ -340,7 +339,7 @@
 
                     if(profile.length) {
                         $.ajax({
-                            url: "/api/v1/import-batch-profiles/"+profile[0].profile_id,
+                            url: "/api/v1/import_batch_profiles/"+profile[0].profile_id,
                             method: "PUT",
                             data: JSON.stringify(params),
                             contentType: 'application/json'
@@ -349,7 +348,7 @@
                         .fail(reject);
                     } else {
                         $.ajax({
-                            url: "/api/v1/import-batch-profiles/",
+                            url: "/api/v1/import_batch_profiles/",
                             method: "POST",
                             data: JSON.stringify(params),
                             contentType: 'application/json'
@@ -359,6 +358,7 @@
                     }
                 })
                 .then(function(profile) {
+                    humanMsg.displayAlert(PROFILE_SAVE_MSG);
                     return getProfiles(profile.profile_id);
                 })
                 .catch(function(error) {
@@ -366,70 +366,8 @@
                 })
             });
 
-            $("#mod_profile").click(function() {
-                var name = $("#profile_name").val().trim();
-                var id = $("#profile").val();
-                if(!id) return;
-                if(!name) {
-                    alert(_("Profile must have a name"));
-                    return;
-                }
-                var profile = $("#profile option[value!='']")
-                    .map(function() {
-                        return $(this).data('profile');
-                    })
-                    .filter(function() {
-                        return this.name == name && this.profile_id != id;
-                    });
-
-                if(profile.length) {
-                    if(!confirm(_("There is another profile with this name.")+"\n\n"+_("Do you want to replace it?"))) {
-                        return;
-                    }
-                }
-                new Promise(function(resolve, reject) {
-                    if(!profile.length) return resolve();
-                    $.ajax({
-                        url: "/api/v1/import-batch-profiles/"+profile[0].profile_id,
-                        method: "DELETE"
-                    })
-                    .done(resolve)
-                    .fail(reject);
-                })
-                .then(function(){
-                    const params = {
-                        comments: $("#comments").val() || null,
-                        record_type: $("#record_type").val() || null,
-                        encoding: $("#encoding").val() || null,
-                        format: $("#format").val() || null,
-                        template_id: $("#marc_modification_template_id").val() || null,
-                        matcher_id: $("#matcher").val() || null,
-                        overlay_action: $("#overlay_action").val() || null,
-                        nomatch_action: $("#nomatch_action").val() || null,
-                        parse_items: !!parseInt($("input[name='parse_items']:checked").val()) || null,
-                        item_action: $("#item_action").val() || null,
-                        name: name
-                    };
-                    return new Promise(function(resolve, reject) {
-                        $.ajax({
-                            url: "/api/v1/import-batch-profiles/"+id,
-                            method: "PUT",
-                            data: JSON.stringify(params),
-                            contentType: 'application/json'
-                        })
-                        .done(resolve)
-                        .fail(reject);
-                    });
-                })
-                .then(function() {
-                    return getProfiles(id);
-                })
-                .catch(function(error) {
-                    alert(_("An error occurred")+"\n\n"+error.message);
-                })
-            });
-
-            $("#del_profile").click(function() {
+            $("#del_profile").click(function(event) {
+                event.preventDefault();
                 var id = $("#profile").val();
                 if(!id) return;
                 if(!confirm(_("Are you sure you want to delete this profile?"))) {
@@ -437,21 +375,22 @@
                 }
                 new Promise(function(resolve, reject) {
                     $.ajax({
-                        url: "/api/v1/import-batch-profiles/"+id,
+                        url: "/api/v1/import_batch_profiles/"+id,
                         method: "DELETE"
                     })
                     .done(resolve)
                     .fail(reject);
                 })
                 .then(function() {
+                    humanMsg.displayAlert(PROFILE_DEL_MSG);
                     return getProfiles();
                 })
                 .catch(function(error) {
                     alert(_("An error occurred")+"\n\n"+error);
                 })
             });
-
         });
+
         function CheckForm(f) {
             if ($("#fileToUpload").value == '') {
                 alert(_("Please upload a file first."));
@@ -510,7 +449,7 @@
             const select = $("#profile");
             $("option[value!='']", select).remove();
             return new Promise(function(resolve, reject) {
-                $.ajax("/api/v1/import-batch-profiles")
+                $.ajax("/api/v1/import_batch_profiles")
                 .then(resolve, reject);
             })
             .then(function(profiles) {
-- 
2.25.0