From c3e7dd6e9f67430e95c1542c5e03399e9c68a308 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     | 97 ++++---------------
 9 files changed, 38 insertions(+), 99 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 04b26fc558..b423ff45f6 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 aa0c4fe4b7..92d9c098a4 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 fd7b19b14d..9a5c9125bd 100644
--- a/api/v1/swagger/paths.json
+++ b/api/v1/swagger/paths.json
@@ -104,11 +104,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..46e0eb7ac0 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;
                     }
                 }
@@ -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?"))) {
@@ -444,14 +382,15 @@
                     .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."));
-- 
2.25.0