From 38650a8102fe0841617cd80f3a6e45b1f5f62fd5 Mon Sep 17 00:00:00 2001
From: Wojtek Kosior <koszko@koszko.org>
Date: Sat, 8 Jan 2022 19:23:17 +0100
Subject: further item list work

There is now a mechanism for removing an item and there are more tests for item list.
The entire thing is still work in progress.
---
 html/item_list.html            |   2 +-
 html/item_list.js              |  62 ++++++++++++++---
 html/item_preview.js           |   2 +-
 test/unit/test_item_list.py    | 155 +++++++++++++++++++++++++++++++++++++----
 test/unit/test_item_preview.py |  15 ++--
 test/unit/utils.py             |   9 +++
 6 files changed, 210 insertions(+), 35 deletions(-)

diff --git a/html/item_list.html b/html/item_list.html
index 41c7734..d082c5d 100644
--- a/html/item_list.html
+++ b/html/item_list.html
@@ -48,7 +48,7 @@
 <template>
   <div id="item_list" data-template="main_div" class="grid_2">
     <ul data-template="ul"></ul>
-    <div data-template="preview_container">
+    <div data-template="preview_container" class="hide">
       <!-- preview div will be dynamically inserted here -->
 
       <button data-template="remove_but">Remove</button>
diff --git a/html/item_list.js b/html/item_list.js
index f6b9bd3..55e54fb 100644
--- a/html/item_list.js
+++ b/html/item_list.js
@@ -52,10 +52,12 @@ function preview_item(list_ctx, item, ignore_dialog=false)
     if (list_ctx.dialog_ctx.shown && !ignore_dialog)
 	return;
 
-    list_ctx.preview_ctx =
-	list_ctx.preview_cb(item.definition, list_ctx.preview_ctx);
-    list_ctx.preview_container
-	.prepend(list_ctx.preview_ctx.main_div);
+    list_ctx.preview_ctx = list_ctx.preview_cb(
+	item.definition,
+	list_ctx.preview_ctx,
+	list_ctx.dialog_ctx
+    );
+    list_ctx.preview_container.prepend(list_ctx.preview_ctx.main_div);
 
     if (list_ctx.previewed_item !== null)
 	list_ctx.previewed_item.li.classList.remove("item_li_highlight");
@@ -110,7 +112,6 @@ function find_item_idx(definition)
 
 function item_changed(list_ctx, change)
 {
-
     /* Remove item. */
     const old_item = list_ctx.by_identifier.get(change.key);
     if (old_item !== undefined) {
@@ -133,7 +134,26 @@ function item_changed(list_ctx, change)
 	preview_item(list_ctx, new_item, true);
 }
 
-async function item_list(preview_cb, track_cb)
+async function remove_clicked(list_ctx)
+{
+    if (list_ctx.dialog_ctx.shown || list_ctx.previewed_item === null)
+	return;
+
+    const identifier = list_ctx.previewed_item.definition.identifier;
+
+    if (!(await dialog.ask(list_ctx.dialog_ctx,
+			   `Are you sure you want to delete '${identifier}'?`)))
+	return;
+
+    try {
+	await list_ctx.remove_cb(identifier);
+    } catch(e) {
+	console.error(e);
+	dialog.error(list_ctx.dialog_ctx, `Couldn't remove '${identifier}' :(`)
+    }
+}
+
+async function item_list(preview_cb, track_cb, remove_cb)
 {
     const list_ctx = clone_template("item_list");
 
@@ -148,6 +168,7 @@ async function item_list(preview_cb, track_cb)
 	tracking,
 	previewed_item: null,
 	preview_cb,
+	remove_cb,
 	dialog_ctx: dialog.make(() => on_dialog_show(list_ctx),
 				() => on_dialog_hide(list_ctx))
     });
@@ -156,6 +177,9 @@ async function item_list(preview_cb, track_cb)
     for (const def of definitions)
 	insert_item(list_ctx, def, list_ctx.items.length);
 
+    list_ctx.remove_but
+	.addEventListener("click", () => remove_clicked(list_ctx));
+
     return list_ctx;
 }
 
@@ -169,16 +193,32 @@ function on_dialog_show(list_ctx)
 function on_dialog_hide(list_ctx)
 {
     list_ctx.ul;
-    list_ctx.preview_container.classList.remove("hide");
+    if (list_ctx.previewed_item !== null)
+	list_ctx.preview_container.classList.remove("hide");
     list_ctx.dialog_container.classList.add("hide");
 }
 
-const resource_list =
-      () => item_list(resource_preview, haketilodb.track.resources);
+async function remove_single_item(item_type, identifier)
+{
+    const store = ({resource: "resources", mapping: "mappings"})[item_type];
+    const transaction_ctx =
+	  await haketilodb.start_items_transaction([store], {});
+    await haketilodb[`remove_${item_type}`](identifier, transaction_ctx);
+    await haketilodb.finalize_transaction(transaction_ctx);
+}
+
+function resource_list()
+{
+      return item_list(resource_preview, haketilodb.track.resources,
+		       id => remove_single_item("resource", id));
+}
 #EXPORT resource_list
 
-const mapping_list =
-      () => item_list(mapping_preview, haketilodb.track.mappings);
+function mapping_list()
+{
+      return item_list(mapping_preview, haketilodb.track.mappings,
+		       id => remove_single_item("mapping", id));
+}
 #EXPORT mapping_list
 
 function destroy_list(list_ctx)
diff --git a/html/item_preview.js b/html/item_preview.js
index f59e30e..447b16a 100644
--- a/html/item_preview.js
+++ b/html/item_preview.js
@@ -64,7 +64,7 @@ async function file_link_clicked(preview_object, file_ref, event)
 					  "files", file_ref.hash_key);
     if (file === undefined) {
 	dialog.error(preview_object.dialog_context,
-		     "File missing from Haketilo's inernal database :(");
+		     "File missing from Haketilo's internal database :(");
     } else {
 	const encoded_file = encodeURIComponent(file.contents);
 	open(`data:text/plain;charset=utf8,${encoded_file}`, '_blank');
diff --git a/test/unit/test_item_list.py b/test/unit/test_item_list.py
index 3aba006..e2e1af8 100644
--- a/test/unit/test_item_list.py
+++ b/test/unit/test_item_list.py
@@ -22,8 +22,7 @@ from selenium.webdriver.support.ui import WebDriverWait
 
 from ..extension_crafting import ExtraHTML
 from ..script_loader import load_script
-from .utils import sample_files, sample_files_by_hash, sample_file_ref, \
-    item_version_string
+from .utils import *
 
 broker_js = lambda: load_script('background/broadcast_broker.js') + ';start();'
 
@@ -71,6 +70,10 @@ def make_sample_resource(identifier, long_name):
         ]
     }
 
+def make_item(item_type, *args):
+    return make_sample_resource(*args) if item_type == 'resource' \
+        else make_sample_mapping(*args)
+
 @pytest.mark.ext_data({
     'background_script': broker_js,
     'extra_html': ExtraHTML('html/item_list.html', {}),
@@ -84,9 +87,6 @@ def test_item_list_ordering(driver, execute_in_page, item_type):
     """
     execute_in_page(load_script('html/item_list.js'))
 
-    make_item = make_sample_resource if item_type == 'resource' \
-        else make_sample_mapping
-
     # Choose sample long names so as to test automatic sorting of items.
     long_names = ['sample', 'sample it', 'Sample it', 'SAMPLE IT',
                   'test', 'test it', 'Test it', 'TEST IT']
@@ -94,13 +94,12 @@ def test_item_list_ordering(driver, execute_in_page, item_type):
     long_names_reversed = [*long_names]
     long_names_reversed.reverse()
 
-    items = [make_item(f'it_{hex(2 * i + copy)[-1]}', name)
+    items = [make_item(item_type, f'it_{hex(2 * i + copy)[-1]}', name)
              for i, name in enumerate(long_names_reversed)
              for copy in (1, 0)]
     # When adding/updating items this item will be updated at the end and this
     # last update will be used to verify that a set of opertions completed.
-    extra_item = make_item('extraitem', 'extra item')
-    extra_dict = {'extraitem': {item_version_string(extra_item): extra_item}}
+    extra_item = make_item(item_type, 'extraitem', 'extra item')
 
     # After this reversal items are sorted in the exact order they are expected
     # to appear in the HTML list.
@@ -131,21 +130,19 @@ def test_item_list_ordering(driver, execute_in_page, item_type):
                 it['long_name'] = f'somewhat renamed {it["long_name"]}'
 
         items_to_inclue = [items[i] for i in sorted(to_include)]
-        sample_data[item_type + 's'] = \
-            dict([(it['identifier'], {item_version_string(it): it})
-                  for it in items_to_inclue])
+        sample_data[item_type + 's'] = sample_data_dict(items_to_inclue)
         execute_in_page('returnval(haketilodb.save_items(arguments[0]));',
                         sample_data)
 
         extra_item['long_name'] = f'{iteration} {extra_item["long_name"]}'
-        sample_data[item_type + 's'] = extra_dict
+        sample_data[item_type + 's'] = sample_data_dict([extra_item])
         execute_in_page('returnval(haketilodb.save_items(arguments[0]));',
                         sample_data)
 
         if iteration == 0:
             execute_in_page(
                 f'''
-                let list_ctx, items = arguments[0];
+                let list_ctx;
                 async function create_list() {{
                     list_ctx = await {item_type}_list();
                     document.body.append(list_ctx.main_div);
@@ -178,3 +175,135 @@ def test_item_list_ordering(driver, execute_in_page, item_type):
         for i, text in zip(sorted(indexes_added), preview_texts):
             assert items[i]['identifier'] in text
             assert items[i]['long_name']  in text
+
+@pytest.mark.ext_data({
+    'background_script': broker_js,
+    'extra_html': ExtraHTML('html/item_list.html', {}),
+    'navigate_to': 'html/item_list.html'
+})
+@pytest.mark.usefixtures('webextension')
+@pytest.mark.parametrize('item_type', ['resource', 'mapping'])
+def test_item_list_displaying(driver, execute_in_page, item_type):
+    """
+    A test case of items list interaction with preview and dialog.
+    """
+    execute_in_page(load_script('html/item_list.js'))
+
+    items = [make_item(item_type, f'item{i}', f'Item {i}') for i in range(3)]
+
+    sample_data = {
+        'resources': {},
+        'mappings': {},
+        'files': sample_files_by_hash
+    }
+    sample_data[item_type + 's'] = sample_data_dict(items)
+
+    preview_container, dialog_container = execute_in_page(
+        f'''
+        let list_ctx, sample_data = arguments[0];
+        async function create_list() {{
+            await haketilodb.save_items(sample_data);
+            list_ctx = await {item_type}_list();
+            document.body.append(list_ctx.main_div);
+            return [list_ctx.preview_container, list_ctx.dialog_container];
+        }}
+        returnval(create_list());
+        ''',
+        sample_data)
+
+    assert not preview_container.is_displayed()
+
+    # Check that preview is displayed correctly.
+    for i in range(3):
+        execute_in_page('list_ctx.ul.children[arguments[0]].click();', i)
+        assert preview_container.is_displayed()
+        text = preview_container.text
+        assert f'item{i}' in text
+        assert f'Item {i}' in text
+
+    # Check that file preview link works.
+    window0 = driver.window_handles[0]
+    driver.find_element_by_link_text('report.spdx').click()
+    WebDriverWait(driver, 10).until(lambda _: len(driver.window_handles) == 2)
+    window1 = next(filter(lambda w: w != window0, driver.window_handles))
+    driver.switch_to.window(window1)
+    assert 'dummy report' in driver.page_source
+
+    driver.close()
+    driver.switch_to.window(window0)
+
+    # Check that item removal confirmation dialog is displayed correctly.
+    execute_in_page('list_ctx.remove_but.click();')
+    WebDriverWait(driver, 10).until(lambda _: dialog_container.is_displayed())
+    assert not preview_container.is_displayed()
+    msg = execute_in_page('returnval(list_ctx.dialog_ctx.msg.textContent);')
+    assert msg == "Are you sure you want to delete 'item2'?"
+
+    # Check that previewing other item is impossible while dialog is open.
+    execute_in_page('list_ctx.ul.children[0].click();')
+    assert dialog_container.is_displayed()
+    assert not preview_container.is_displayed()
+
+    # Check that queuing multiple removal confirmation dialogs is impossible.
+    execute_in_page('list_ctx.remove_but.click();')
+
+    # Check that answering "No" causes the item not to be removed and unhides
+    # item preview.
+    execute_in_page('list_ctx.dialog_ctx.no_but.click();')
+    WebDriverWait(driver, 10).until(lambda _: preview_container.is_displayed())
+    assert not dialog_container.is_displayed()
+    assert execute_in_page('returnval(list_ctx.ul.children.length);') == 3
+
+    # Check that item removal works properly.
+    def remove_current_item():
+        execute_in_page('list_ctx.remove_but.click();')
+        WebDriverWait(driver, 10)\
+            .until(lambda _: dialog_container.is_displayed())
+        execute_in_page('list_ctx.dialog_ctx.yes_but.click();')
+
+    remove_current_item()
+
+    def item_deleted(driver):
+        return execute_in_page('returnval(list_ctx.ul.children.length);') == 2
+    WebDriverWait(driver, 10).until(item_deleted)
+    assert not dialog_container.is_displayed()
+    assert not preview_container.is_displayed()
+
+    execute_in_page('list_ctx.ul.children[1].click();')
+
+    # Check that missing file causes the right error dialog to appear.
+    execute_in_page(
+        '''{
+        async function steal_file(hash_key)
+        {
+            const db = await haketilodb.get();
+            const transaction = db.transaction("files", "readwrite");
+            transaction.objectStore("files").delete(hash_key);
+        }
+        returnval(steal_file(arguments[0]));
+        }''',
+        sample_files['LICENSES/CC0-1.0.txt']['hash_key'])
+    driver.find_element_by_link_text('LICENSES/CC0-1.0.txt').click()
+    WebDriverWait(driver, 10).until(lambda _: dialog_container.is_displayed())
+    assert not preview_container.is_displayed()
+
+    msg = execute_in_page('returnval(list_ctx.dialog_ctx.msg.textContent);')
+    assert msg == "File missing from Haketilo's internal database :("
+
+    execute_in_page('returnval(list_ctx.dialog_ctx.ok_but.click());')
+    WebDriverWait(driver, 10).until(lambda _: preview_container.is_displayed())
+
+    # Check that item removal failure causes the right error dialog to appear.
+    execute_in_page('haketilodb.finalize_transaction = () => {throw "sth";};')
+    remove_current_item()
+    WebDriverWait(driver, 10).until(lambda _: dialog_container.is_displayed())
+    msg = execute_in_page('returnval(list_ctx.dialog_ctx.msg.textContent);')
+    assert msg == "Couldn't remove 'item1' :("
+
+    # Destroy item list.
+    assert True == execute_in_page(
+        '''
+        const main_div = list_ctx.main_div;
+        destroy_list(list_ctx);
+        returnval(main_div.parentElement === null);
+        ''')
diff --git a/test/unit/test_item_preview.py b/test/unit/test_item_preview.py
index 887e4f4..c3aaf1f 100644
--- a/test/unit/test_item_preview.py
+++ b/test/unit/test_item_preview.py
@@ -199,13 +199,8 @@ def test_file_preview_link(driver, execute_in_page):
 
     sample_resource = make_sample_resource()
     sample_data = {
-        'resources': {
-            sample_resource['identifier']: {
-                item_version_string(sample_resource): sample_resource
-            }
-        },
-        'mappings': {
-        },
+        'resources': sample_data_dict([sample_resource]),
+        'mappings': {},
         'files': sample_files_by_hash
     }
     execute_in_page('returnval(haketilodb.save_items(arguments[0]));',
@@ -231,5 +226,7 @@ def test_file_preview_link(driver, execute_in_page):
 
     driver.switch_to.window(window0)
     driver.find_element_by_link_text('bye.js').click()
-    assert driver.execute_script('return window.error_args;') == \
-        ['dummy dialog ctx', "File missing from Haketilo's inernal database :("]
+    assert driver.execute_script('return window.error_args;') == [
+        'dummy dialog ctx',
+        "File missing from Haketilo's internal database :("
+    ]
diff --git a/test/unit/utils.py b/test/unit/utils.py
index e2d89b9..b6b389f 100644
--- a/test/unit/utils.py
+++ b/test/unit/utils.py
@@ -57,3 +57,12 @@ def item_version_string(definition, include_revision=False):
     ver = '.'.join([str(num) for num in definition['version']])
     revision = definition.get('revision') if include_revision else None
     return f'{ver}-{revision}' if revision is not None else ver
+
+def sample_data_dict(items):
+    """
+    Some indexeddb functions expect saved items to be provided in a nested dict
+    that makes them queryable by identifier by version. This function converts
+    items list to such dict.
+    """
+    return dict([(it['identifier'], {item_version_string(it): it})
+                 for it in items])
-- 
cgit v1.2.3