Repository: codalab/codalab-worksheets

Issue:
Add host_worksheets to genpath
We should be able to do

```
% display table host_worksheet
[bundle]
```

to show a column with a link to the host worksheet (if there is more than one, just return the first one).

To do this, several things need to happen:

1) We need to have get_bundle_infos return host_worksheet (similar to how we need the `-v` in  `cl info -f host_worksheets <bundle> -v`).

2) The returned value from worksheet_util.interpret_genpath should be a dict {type: 'worksheet', uuid: ..., name: ...} (or something like this; need to sync with displaying bundle links).

3) On the frontend, a link should be displayed.


Discussion:
yipenghe: It's working on dashboard
![Screenshot from 2019-11-26 00-53-46](https://user-images.githubusercontent.com/23012631/69613971-494f0980-0fe7-11ea-8b80-469e8f0d1dcd.png)

But not on public home..
![Screenshot from 2019-11-26 00-50-09](https://user-images.githubusercontent.com/23012631/69613772-00975080-0fe7-11ea-9434-bb1469fdbea8.png)

teetone:  In public home, you have it as host_worksheets. It should be host_worksheet.
    On Tuesday, November 26, 2019, 12:55:08 AM PST, yipenghe <notifications@github.com> wrote:  
 
 
It's working on dashboard


But not on public home..


—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
  
yipenghe: > In public home, you have it as host_worksheets. It should be host_worksheet. On Tuesday, November 26, 2019, 12:55:08 AM PST, yipenghe <notifications@github.com> wrote: It's working on dashboard But not on public home.. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Yeah I figured...
yipenghe: Since this is on the old ui, can you check it works with the current frontend?
teetone: > Since this is on the old ui, can you check it works with the current frontend?

@yipenghe: Good call, I just cherry-picked it and it works. Thanks.

Patch:
diff --git a/codalab/model/bundle_model.py b/codalab/model/bundle_model.py
index 74b13f0c..5ef696fc 100644
--- a/codalab/model/bundle_model.py
+++ b/codalab/model/bundle_model.py
@@ -258,11 +258,40 @@ class BundleModel(object):
             result[row.parent_uuid].append(row.child_uuid)
         return result
 
-    def get_host_worksheet_uuids(self, bundle_uuids):
+    def get_host_worksheet_uuids(self, bundle_uuids, max_worksheets):
         """
-        Return list of worksheet uuids that contain the given bundle_uuids.
-        bundle_uuids = ['0x12435']
-        Return {'0x12435': [host_worksheet_uuid, ...], ...}
+        Get up to n host_worksheet uuids per bundle uuid. n of 0 will return an empty dictionary.
+        bundle_uuids: list of bundle uuid's (e.g. ['0x12345', '0x23456'])
+        max_worksheets: max limit of host_worksheet uuid's to fetch per bundle
+        Return dict of bundle uuid's to a list of host worksheet uuid's {'0x12345': [host_worksheet_uuid, ...], ...}
+        """
+        if max_worksheets < 0:
+            raise ValueError('Invalid n: {}. n has to be 0 or greater.'.format(max_worksheets))
+        if max_worksheets == 0:
+            return dict()
+
+        with self.engine.begin() as connection:
+            rows = connection.execute(
+                select(
+                    [
+                        cl_worksheet_item.c.bundle_uuid,
+                        func.substring_index(
+                            func.group_concat(cl_worksheet_item.c.worksheet_uuid),
+                            ',',
+                            max_worksheets,
+                        ).label('worksheet_uuids'),
+                    ]
+                )
+                .where(cl_worksheet_item.c.bundle_uuid.in_(bundle_uuids))
+                .group_by(cl_worksheet_item.c.bundle_uuid)
+            ).fetchall()
+        return dict((row.bundle_uuid, row.worksheet_uuids.split(',')) for row in rows)
+
+    def get_all_host_worksheet_uuids(self, bundle_uuids):
+        """
+        Return list of all worksheet uuids that contain the given bundle_uuids.
+        bundle_uuids: list of bundle uuid's (e.g. ['0x12345', '0x23456']
+        Return dict of bundle uuid's to a list of host worksheet uuid's {'0x12345': [host_worksheet_uuid, ...], ...}
         """
         with self.engine.begin() as connection:
             rows = connection.execute(
diff --git a/codalab/rest/bundles.py b/codalab/rest/bundles.py
index ddead415..689cd146 100644
--- a/codalab/rest/bundles.py
+++ b/codalab/rest/bundles.py
@@ -788,7 +788,7 @@ def delete_bundles(uuids, force, recursive, data_only, dry_run):
         )
 
     # Make sure that bundles are not referenced in multiple places (otherwise, it's very dangerous)
-    result = local.model.get_host_worksheet_uuids(relevant_uuids)
+    result = local.model.get_all_host_worksheet_uuids(relevant_uuids)
     for uuid, host_worksheet_uuids in result.items():
         worksheets = local.model.batch_get_worksheets(fetch_items=False, uuid=host_worksheet_uuids)
         frozen_worksheets = [worksheet for worksheet in worksheets if worksheet.frozen]
diff --git a/codalab/rest/interpret.py b/codalab/rest/interpret.py
index a9cce0f0..2dc661f2 100644
--- a/codalab/rest/interpret.py
+++ b/codalab/rest/interpret.py
@@ -540,7 +540,7 @@ def resolve_items_into_infos(items):
     # optimized batch_get_bundles multiget method.
     bundle_uuids = set(i['bundle_uuid'] for i in items if i['bundle_uuid'] is not None)
 
-    bundle_dict = rest_util.get_bundle_infos(bundle_uuids)
+    bundle_dict = rest_util.get_bundle_infos(bundle_uuids, get_single_host_worksheet=True)
 
     # Go through the items and substitute the components
     new_items = []
diff --git a/codalab/rest/util.py b/codalab/rest/util.py
index d813353d..f3bd3246 100644
--- a/codalab/rest/util.py
+++ b/codalab/rest/util.py
@@ -46,29 +46,35 @@ def resolve_owner_in_keywords(keywords):
 def get_bundle_infos(
     uuids,
     get_children=False,
+    get_single_host_worksheet=False,
     get_host_worksheets=False,
     get_permissions=False,
     ignore_not_found=True,
+    model=None,
 ):
     """
     Return a map from bundle uuid to info.
 
     :param Collection[str] uuids: uuids of bundles to fetch
     :param bool get_children: include children
-    :param bool get_host_worksheets: include host worksheets
+    :param bool get_single_host_worksheet: include one host_worksheet per bundle uuid
+    :param bool get_host_worksheets: include all host worksheets
     :param bool get_permissions: include group permissions
     :param bool ignore_not_found: abort with 404 NOT FOUND when False and bundle doesn't exist
+    :param BundleModel model: model used to make database queries
     :rtype: dict[str, dict]
     """
+    if model is None:
+        model = local.model
     if len(uuids) == 0:
         return {}
-    bundles = local.model.batch_get_bundles(uuid=uuids)
+    bundles = model.batch_get_bundles(uuid=uuids)
     bundle_infos = {
-        bundle.uuid: bundle_util.bundle_to_bundle_info(local.model, bundle) for bundle in bundles
+        bundle.uuid: bundle_util.bundle_to_bundle_info(model, bundle) for bundle in bundles
     }
 
     # Implement permissions policies
-    perms = _get_user_bundle_permissions(uuids)
+    perms = _get_user_bundle_permissions(model, uuids)
     readable = {u for u, perm in perms.items() if perm >= GROUP_OBJECT_PERMISSION_READ}
     anonymous = {
         u
@@ -87,9 +93,9 @@ def get_bundle_infos(
         # Replace bundles that the user does not have read access to
         elif uuid not in readable:
             bundle_infos[uuid] = bundle_util.bundle_to_bundle_info(
-                local.model, PrivateBundle.construct(uuid)
+                model, PrivateBundle.construct(uuid)
             )
-        # Mask owners of anonymous bundles that user does not have all acccess to
+        # Mask owners of anonymous bundles that user does not have all access to
         elif uuid in anonymous:
             bundle['owner_id'] = None
 
@@ -97,14 +103,14 @@ def get_bundle_infos(
         bundle['permission'] = perms[uuid]
 
     if get_children:
-        parent2children = local.model.get_children_uuids(readable)
+        parent2children = model.get_children_uuids(readable)
 
         # Gather all children bundle uuids and fetch permissions
         child_uuids = [uuid for l in parent2children.values() for uuid in l]
-        child_perms = _get_user_bundle_permissions(child_uuids)
+        child_perms = _get_user_bundle_permissions(model, child_uuids)
 
         # Lookup bundle names
-        child_names = local.model.get_bundle_names(child_uuids)
+        child_names = model.get_bundle_names(child_uuids)
 
         # Set children infos
         for parent_uuid, children in parent2children.items():
@@ -114,39 +120,41 @@ def get_bundle_infos(
                 if child_perms[child_uuid] >= GROUP_OBJECT_PERMISSION_READ
             ]
 
+    if get_single_host_worksheet:
+        # Query for 5 worksheet uuids per bundle to check the read permissions for, since we
+        # just need a single host worksheet per bundle uuid. This is much faster than fetching all
+        # worksheet uuid's per bundle.
+        host_worksheets = model.get_host_worksheet_uuids(readable, 5)
+        worksheet_uuids = [uuid for l in host_worksheets.values() for uuid in l]
+        worksheet_names = _get_readable_worksheet_names(model, worksheet_uuids)
+
+        for bundle_uuid, host_uuids in host_worksheets.items():
+            for host_uuid in host_uuids:
+                if host_uuid in worksheet_names:
+                    bundle_infos[bundle_uuid]['host_worksheet'] = {
+                        'uuid': host_uuid,
+                        'name': worksheet_names[host_uuid],
+                    }
+                    # Just set a single host worksheet per bundle uuid
+                    break
+
     if get_host_worksheets:
-        # bundle_uuids -> list of worksheet_uuids
-        host_worksheets = local.model.get_host_worksheet_uuids(readable)
+        host_worksheets = model.get_all_host_worksheet_uuids(readable)
         # Gather all worksheet uuids
         worksheet_uuids = [uuid for l in host_worksheets.values() for uuid in l]
-        wpermissions = local.model.get_user_worksheet_permissions(
-            request.user.user_id,
-            worksheet_uuids,
-            local.model.get_worksheet_owner_ids(worksheet_uuids),
-        )
-        readable_worksheet_uuids = set(
-            uuid
-            for uuid, permission in wpermissions.items()
-            if permission >= GROUP_OBJECT_PERMISSION_READ
-        )
-        # Lookup names
-        worksheets = dict(
-            (worksheet.uuid, worksheet)
-            for worksheet in local.model.batch_get_worksheets(
-                fetch_items=False, uuid=readable_worksheet_uuids
-            )
-        )
+        worksheet_names = _get_readable_worksheet_names(model, worksheet_uuids)
+
         # Fill the info
         for bundle_uuid, host_uuids in host_worksheets.items():
             bundle_infos[bundle_uuid]['host_worksheets'] = [
-                {'uuid': host_uuid, 'name': worksheets[host_uuid].name}
+                {'uuid': host_uuid, 'name': worksheet_names[host_uuid]}
                 for host_uuid in host_uuids
-                if host_uuid in readable_worksheet_uuids
+                if host_uuid in worksheet_names
             ]
 
     if get_permissions:
         # Fill the permissions info
-        bundle2group_perms = local.model.batch_get_group_bundle_permissions(
+        bundle2group_perms = model.batch_get_group_bundle_permissions(
             request.user.user_id, readable
         )
         for uuid, group_perms in bundle2group_perms.items():
@@ -160,9 +168,32 @@ def get_bundle_infos(
     return bundle_infos
 
 
-def _get_user_bundle_permissions(uuids):
-    return local.model.get_user_bundle_permissions(
-        request.user.user_id, uuids, local.model.get_bundle_owner_ids(uuids)
+def _get_user_bundle_permissions(model, uuids):
+    return model.get_user_bundle_permissions(
+        request.user.user_id, uuids, model.get_bundle_owner_ids(uuids)
+    )
+
+
+def _get_readable_worksheet_names(model, worksheet_uuids):
+    # Returns a dictionary of readable worksheet uuid's as keys and corresponding names as values
+    readable_worksheet_uuids = _filter_readable_worksheet_uuids(model, worksheet_uuids)
+    return dict(
+        (worksheet.uuid, worksheet.name)
+        for worksheet in model.batch_get_worksheets(
+            fetch_items=False, uuid=readable_worksheet_uuids
+        )
+    )
+
+
+def _filter_readable_worksheet_uuids(model, worksheet_uuids):
+    # Returns a set of worksheet uuid's the user has read permission for
+    worksheet_permissions = model.get_user_worksheet_permissions(
+        request.user.user_id, worksheet_uuids, model.get_worksheet_owner_ids(worksheet_uuids)
+    )
+    return set(
+        uuid
+        for uuid, permission in worksheet_permissions.items()
+        if permission >= GROUP_OBJECT_PERMISSION_READ
     )
 
 
diff --git a/frontend/src/components/worksheets/items/TableItem.js b/frontend/src/components/worksheets/items/TableItem.js
index e99d8d65..31f3b150 100644
--- a/frontend/src/components/worksheets/items/TableItem.js
+++ b/frontend/src/components/worksheets/items/TableItem.js
@@ -89,9 +89,17 @@ class TableItem extends React.Component {
             if (rowItems[0][x] && rowItems[0][x]['path']) columnWithHyperlinks.push(x);
         });
         var bodyRowsHtml = rowItems.map(function(rowItem, rowIndex) {
-            var rowRef = 'row' + rowIndex;
-            var rowFocused = self.props.focused && rowIndex == self.props.subFocusIndex;
-            var url = '/bundles/' + bundleInfos[rowIndex].uuid;
+            let bundleInfo = bundleInfos[rowIndex];
+            let rowRef = 'row' + rowIndex;
+            let rowFocused = self.props.focused && rowIndex == self.props.subFocusIndex;
+            let url = '/bundles/' + bundleInfo.uuid;
+            let worksheet = bundleInfo.host_worksheet;
+            let worksheetName, worksheetUrl;
+            if (worksheet !== undefined) {
+                worksheetName = worksheet.name;
+                worksheetUrl = '/worksheets/' + worksheet.uuid;
+            }
+
             return (
                 <TableRow
                     key={rowIndex}
@@ -101,14 +109,16 @@ class TableItem extends React.Component {
                     focused={rowFocused}
                     focusIndex={self.props.focusIndex}
                     url={url}
-                    bundleInfo={bundleInfos[rowIndex]}
-                    uuid={bundleInfos[rowIndex].uuid}
+                    bundleInfo={bundleInfo}
+                    uuid={bundleInfo.uuid}
                     headerItems={headerItems}
                     columnClasses={columnClasses}
                     canEdit={canEdit}
                     updateRowIndex={self.updateRowIndex}
                     columnWithHyperlinks={columnWithHyperlinks}
                     handleContextMenu={self.props.handleContextMenu}
+                    worksheetName={worksheetName}
+                    worksheetUrl={worksheetUrl}
                 />
             );
         });
@@ -141,14 +151,21 @@ class TableRow extends React.Component {
         var baseUrl = this.props.url;
         var uuid = this.props.uuid;
         var columnWithHyperlinks = this.props.columnWithHyperlinks;
+        var worksheetName = this.props.worksheetName;
+        var worksheetUrl = this.props.worksheetUrl;
         var rowCells = this.props.headerItems.map(function(headerKey, col) {
             var rowContent = rowItems[headerKey];
 
             // See if there's a link
             var url;
-            if (col == 0) {
+            if (headerKey === 'host_worksheet' && worksheetUrl !== undefined) {
+                url = worksheetUrl;
+                rowContent = worksheetName;
+            }
+            else if (col === 0) {
                 url = baseUrl;
-            } else if (columnWithHyperlinks.indexOf(headerKey) != -1) {
+            }
+            else if (columnWithHyperlinks.indexOf(headerKey) !== -1) {
                 url = '/rest/bundles/' + uuid + '/contents/blob' + rowContent['path'];
                 if ('text' in rowContent) {
                     rowContent = rowContent['text'];


Test patch:
diff --git a/tests/rest/__init__.py b/tests/rest/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/rest/util_test.py b/tests/rest/util_test.py
new file mode 100644
index 00000000..27d98a33
--- /dev/null
+++ b/tests/rest/util_test.py
@@ -0,0 +1,92 @@
+import unittest
+from mock import Mock
+
+from codalab.model.tables import GROUP_OBJECT_PERMISSION_ALL, GROUP_OBJECT_PERMISSION_NONE
+from codalab.rest import util
+
+
+class UtilTest(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        util.request.user = Mock(user_id='test_user')
+
+    def _mock_model_call(self, expected_uuids, to_return, uuid_arg_position=0):
+        def side_effect(*args, **kwargs):
+            actual_uuids = kwargs.get('uuid') if 'uuid' in kwargs else set(args[uuid_arg_position])
+            if actual_uuids == expected_uuids:
+                return to_return
+            else:
+                self.fail(
+                    'BundleModel function called with the wrong arguments. Passed in %s but expected %s.'
+                    % (actual_uuids, expected_uuids)
+                )
+
+        return side_effect
+
+    def _mock_bundle(self, uuid):
+        metadata_mock = Mock()
+        metadata_mock.to_dict.side_effect = lambda: dict()
+        return Mock(
+            uuid=uuid,
+            bundle_type=None,
+            owner_id=None,
+            command=None,
+            data_hash=None,
+            state=None,
+            is_anonymous=False,
+            metadata=metadata_mock,
+            dependencies=[],
+        )
+
+    def test_get_bundle_infos_single_worksheet(self):
+        bundle_uuids = {'0x123', '0x234', '0x345'}
+        worksheet_uuids = {'0x111', '0x222', '0x333', '0x444'}
+        model = Mock()
+        model.batch_get_bundles.side_effect = self._mock_model_call(
+            bundle_uuids, [self._mock_bundle(uuid) for uuid in bundle_uuids]
+        )
+        bundle_permissions = {
+            '0x123': GROUP_OBJECT_PERMISSION_ALL,
+            '0x234': GROUP_OBJECT_PERMISSION_ALL,
+            '0x345': GROUP_OBJECT_PERMISSION_ALL,
+        }
+        model.get_user_bundle_permissions.side_effect = self._mock_model_call(
+            bundle_uuids, bundle_permissions, 1
+        )
+        bundle_worksheet_uuids = {
+            '0x123': ['0x111', '0x444'],
+            '0x234': ['0x222'],
+            '0x345': ['0x333'],
+        }
+        model.get_host_worksheet_uuids.side_effect = self._mock_model_call(
+            bundle_uuids, bundle_worksheet_uuids
+        )
+        worksheet_permissions = {
+            '0x111': GROUP_OBJECT_PERMISSION_ALL,
+            '0x222': GROUP_OBJECT_PERMISSION_ALL,
+            '0x333': GROUP_OBJECT_PERMISSION_NONE,
+            '0x444': GROUP_OBJECT_PERMISSION_NONE,
+        }
+        model.get_user_worksheet_permissions.side_effect = self._mock_model_call(
+            worksheet_uuids, worksheet_permissions, 1
+        )
+        worksheets = [Mock(uuid='0x111'), Mock(uuid='0x222')]
+        worksheets[0].configure_mock(name='ws1')
+        worksheets[1].configure_mock(name='ws2')
+        model.batch_get_worksheets.side_effect = self._mock_model_call(
+            {'0x111', '0x222'}, worksheets
+        )
+        infos = util.get_bundle_infos(
+            bundle_uuids,
+            get_children=False,
+            get_single_host_worksheet=True,
+            get_host_worksheets=False,
+            get_permissions=False,
+            ignore_not_found=True,
+            model=model,
+        )
+
+        # Bundles 0x123 and 0x234 had a worksheet with correct permissions. 0x345 had a worksheet that can't be read.
+        self.assertDictEqual(infos['0x123']['host_worksheet'], {'uuid': '0x111', 'name': 'ws1'})
+        self.assertDictEqual(infos['0x234']['host_worksheet'], {'uuid': '0x222', 'name': 'ws2'})
+        self.assertTrue('host_worksheet' not in infos['0x345'])