Rdiscrowd 5851 taskbrowse bookmarks dates (#837)

* add dates to task browse bookmarks

* fix typo in test

* remove print

* switch bookmark endpoint output to array

* add bookmarks data to task browse

* fix typo in test

remove print

* add update bookmark test

* refactor

* update

* remove make_timestamp

* remove make_timestamp

* refactor

remove make_timestamp

remove make_timestamp

* refactor

* Fix tests

pybossa/view/account.py

@@ -36,6 +36,7 @@
 from flask_login import login_required, login_user, logout_user, \
     current_user
 from rq import Queue
+from datetime import datetime
 
 import pybossa.model as model
 from flask_babel import gettext
@@ -57,6 +58,7 @@
 from pybossa.core import user_repo, ldap
 from pybossa.feed import get_update_feed
 from pybossa.messages import *
+from pybossa.model import make_timestamp
 
 from pybossa.forms.forms import UserPrefMetadataForm, RegisterFormWithUserPrefMetadata
 from pybossa.forms.account_view_forms import *
@@ -1154,13 +1156,22 @@ def add_metadata(name):
     return redirect(url_for('account.profile', name=name))
 
 
-def _get_bookmarks(user_name, short_name):
+def bookmarks_dict_to_array(bookmarks_dict):
+    bookmarks_array = []
+    for name, meta in bookmarks_dict.items():
+        b = {'name': name}
+        b.update(meta)
+        bookmarks_array.append(b)
+    return bookmarks_array
+
+
+def get_bookmarks(user_name, short_name):
     taskbrowse_bookmarks = cached_users.get_taskbrowse_bookmarks(user_name)
     proj_bookmarks = taskbrowse_bookmarks.get(short_name, {})
-    return proj_bookmarks
+    return bookmarks_dict_to_array(proj_bookmarks)
 
 
-def _add_bookmark(user_name, short_name, bookmark_name, bookmark_url):
+def add_bookmark(user_name, short_name, bookmark_name, bookmark_url):
     user = user_repo.get_by_name(name=user_name)
     taskbrowse_bookmarks = user.info.get('taskbrowse_bookmarks', {})
     proj_bookmarks = taskbrowse_bookmarks.get(short_name, {})
@@ -1170,22 +1181,33 @@ def _add_bookmark(user_name, short_name, bookmark_name, bookmark_url):
     if bookmark_url is None or len(bookmark_url) > MAX_BOOKMARK_URL_LEN:
         raise ValueError('Bookmark URL must be between 1-100 characters.')
 
-    proj_bookmarks[bookmark_name] =  bookmark_url
+    old_bookmark = proj_bookmarks.get(bookmark_name, None)
+    if old_bookmark is not None:
+        created_date = old_bookmark['created']
+    else:
+        created_date = model.make_timestamp()
+    updated_date = model.make_timestamp()
+    bookmark_data = {
+        'url': bookmark_url,
+        'created': created_date,
+        'updated': updated_date
+    }
+    proj_bookmarks[bookmark_name] =  bookmark_data
     taskbrowse_bookmarks[short_name] = proj_bookmarks
     user.info['taskbrowse_bookmarks'] = taskbrowse_bookmarks
 
     user_repo.update(user)
     cached_users.delete_taskbrowse_bookmarks(user)
-    return proj_bookmarks
+    return bookmarks_dict_to_array(proj_bookmarks)
 
 
-def _delete_bookmark(user_name, short_name, bookmark_name):
+def delete_bookmark(user_name, short_name, bookmark_name):
     user = user_repo.get_by_name(name=user_name)
     taskbrowse_bookmarks = user.info.get('taskbrowse_bookmarks', {})
     proj_bookmarks = taskbrowse_bookmarks.get(short_name, {})
 
     if bookmark_name not in proj_bookmarks:
-        raise ValueError('Bookmark not found.')
+        raise ValueError('Bookmark not found')
     del proj_bookmarks[bookmark_name]
     # if no bookmarks left for this project, delete the mapping entry
     if len(proj_bookmarks) == 0:
@@ -1196,7 +1218,7 @@ def _delete_bookmark(user_name, short_name, bookmark_name):
     user.info['taskbrowse_bookmarks'] = taskbrowse_bookmarks
     user_repo.update(user)
     cached_users.delete_taskbrowse_bookmarks(user)
-    return proj_bookmarks
+    return bookmarks_dict_to_array(proj_bookmarks)
 
 
 @blueprint.route('/<user_name>/taskbrowse_bookmarks/<short_name>', methods=['GET', 'POST', 'DELETE'])
@@ -1208,14 +1230,14 @@ def taskbrowse_bookmarks(user_name, short_name):
 
     # get bookmarks for project from cache
     if request.method == 'GET':
-        res_bookmarks = _get_bookmarks(user_name, short_name)
+        res_bookmarks = get_bookmarks(user_name, short_name)
 
     # add a bookmark
     elif request.method == 'POST':
         bookmark_name = request.json.get('name', None)
         bookmark_url = request.json.get('url', None)
         try:
-            res_bookmarks = _add_bookmark(user_name, short_name, bookmark_name, bookmark_url)
+            res_bookmarks = add_bookmark(user_name, short_name, bookmark_name, bookmark_url)
         except ValueError as e:
             error_msg = str(e)
             current_app.logger.exception(f'Bad request: {error_msg},  project: {short_name}, bookmark_name:{bookmark_name}')
@@ -1225,7 +1247,7 @@ def taskbrowse_bookmarks(user_name, short_name):
     elif request.method == 'DELETE':
         bookmark_name = request.json.get('name', None)
         try:
-            res_bookmarks = _delete_bookmark(user_name, short_name, bookmark_name)
+            res_bookmarks = delete_bookmark(user_name, short_name, bookmark_name)
         except ValueError as e:
             error_msg = str(e)
             current_app.logger.exception(f'Bad request: {error_msg},  project: {short_name}, bookmark_name:{bookmark_name}')

pybossa/view/projects.py

@@ -1814,6 +1814,7 @@ def get_users_completed(task):
                     regular_user=regular_user,
                     admin_subadmin_coowner=admin_subadmin_coowner)
 
+
         return handle_content_type(data)
 
     def get_users_fullname(page_tasks, get_users_func, result_field):

test/test_web.py

@@ -10614,33 +10614,58 @@ def test_user_has_partial_answer_without_auth(self):
         resp = self.app_get_json(url)
         assert resp.status_code == 401, resp
 
+    def generate_sample_bookmarks(self, target_project="project1"):
+        bookmark_1_data = {
+                    "created": "2019-01-01T14:37:30.642119",
+                    "updated": "2019-01-01T14:37:30.642119",
+                    "url": "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time%22%2C%22gold_task%22%2C%22actions%22%2C%22lock_status%22%5D&order_by=task_id+asc&pcomplete_from=46&pcomplete_to=100&priority_from=0.45&priority_to=1.00&display_info_columns=%5B%5D"
+                }
+        bookmark_2_data = {
+                    "created": "2019-01-01T14:37:30.642119",
+                    "updated": "2023-01-01T14:37:30.642119",
+                    "url": "https://gigwork.net/project/testproject66/tasks/browse"
+                }
+        bookmark_3_data = {
+                    "created": "2022-01-01T14:37:30.642119",
+                    "updated": "2022-01-01T14:37:30.642119",
+                    "url": "https://gigwork.net/project/project2/tasks/browse"
+                }
+        bookmarks = {
+            target_project : {
+                "bookmark 1" : bookmark_1_data,
+                "bookmark 2" : bookmark_2_data
+            },
+            "project2" : {
+                "bookmark 3" : bookmark_3_data
+            }
+        }
+        return (bookmark_1_data, bookmark_2_data, bookmark_3_data, bookmarks)
+
 
     @with_context
     def test_get_taskbrowse_bookmarks(self):
         """Test get taskbrowse_bookmark works"""
         data = self.original
         target_project = "project1"
-        bookmarks = {
-                    target_project : {
-                                "bookmark 1" : "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time%22%2C%22gold_task%22%2C%22actions%22%2C%22lock_status%22%5D&order_by=task_id+asc&pcomplete_from=46&pcomplete_to=100&priority_from=0.45&priority_to=1.00&display_info_columns=%5B%5D",
-                                "bookmark 2" : "https://gigwork.net/project/testproject66/tasks/browse"
-                                },
-                    "project2" : {
-                        "bookmark 3" : "https://gigwork.net/project/project2/tasks/browse"
-                    }
-
-        }
+        bookmark_1_data, bookmark_2_data, bookmark_3_data, bookmarks = self.generate_sample_bookmarks(target_project)
         info = {
                 'taskbrowse_bookmarks' : bookmarks
-            }
+        }
         user = UserFactory.create(info=info)
         self.signin_user(user)
         url = f"/account/{user.name}/taskbrowse_bookmarks/{target_project}"
         res = self.app.get(url)
 
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str(bookmarks[target_project])
+
+        assert data[0]['url'] == bookmark_1_data['url']
+        assert data[0]['created'] == bookmark_1_data['created']
+        assert data[0]['updated'] == bookmark_1_data['updated']
+
+        assert data[1]['url'] == bookmark_2_data['url']
+        assert data[1]['created'] == bookmark_2_data['created']
+        assert data[1]['updated'] == bookmark_2_data['updated']
 
 
     @with_context
@@ -10656,24 +10681,15 @@ def test_get_taskbrowse_bookmarks_no_bookmarks(self):
 
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({})
+        assert len(data) == 0
 
 
     @with_context
     def test_get_taskbrowse_bookmarks_no_saved_bookmarks_for_project(self):
         """Test get taskbrowse_bookmark works when no saved bookmarks for project"""
         data = self.original
-        target_project = "project1"
-        bookmarks = {
-                    "project600" : {
-                                "bookmark 1" : "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time%22%2C%22gold_task%22%2C%22actions%22%2C%22lock_status%22%5D&order_by=task_id+asc&pcomplete_from=46&pcomplete_to=100&priority_from=0.45&priority_to=1.00&display_info_columns=%5B%5D",
-                                "bookmark 2" : "https://gigwork.net/project/testproject66/tasks/browse"
-                                },
-                    "project2" : {
-                        "bookmark 3" : "https://gigwork.net/project/project2/tasks/browse"
-                    }
-
-        }
+        target_project = "project_with_no_bookmarks"
+        _, _, _, bookmarks = self.generate_sample_bookmarks()
         info = {
                 'taskbrowse_bookmarks' : bookmarks
             }
@@ -10684,7 +10700,7 @@ def test_get_taskbrowse_bookmarks_no_saved_bookmarks_for_project(self):
 
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({})
+        assert len(data) == 0
 
 
     @with_context
@@ -10694,9 +10710,9 @@ def test_post_taskbrowse_bookmarks(self):
         target_project = "project1"
 
         url1 = "https://gigwork.net/project/testproject66/tasks/browse"
-        url2 = "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time%22%2C%22gold_task%22%2C%22actions%22%2C%22lock_status%22%5D&order_by=task_id+asc&pcomplete_from=46&pcomplete_to=100&priority_from=0.45&priority_to=1.00&display_info_columns=%5B%5D"
-        name1 = "bookmark1"
-        name2 = "bookmark2"
+        url2 = "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time&order_by=task_id"
+        name1 = "bookmark 1"
+        name2 = "bookmark 2"
 
         user = UserFactory.create()
         self.signin_user(user)
@@ -10706,32 +10722,59 @@ def test_post_taskbrowse_bookmarks(self):
         res = self.app.post(url, json={"name":name1, "url":url1})
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({name1:url1})
+        assert data[0]['url'] == url1
+        assert 'created' in data[0]
+        assert 'updated' in data[0]
 
-        # test if new url is appended correctly
+        # test second insertion
         res = self.app.post(url, json={"name":name2, "url":url2})
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({name1:url1, name2:url2})
+        assert data[0]['url'] == url1
+        assert data[1]['url'] == url2
 
         # test if data is saved in db
         res = self.app.get(url)
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({name1:url1, name2:url2})
+        assert data[0]['url'] == url1
+        assert data[1]['url'] == url2
 
         # test adding bookmark for a different project
         new_url = f"/account/{user.name}/taskbrowse_bookmarks/project2"
-        res = self.app.post(new_url, json={"name":name2, "url":url2})
+        res = self.app.post(new_url, json={"name":name2, "url":url1})
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({name2:url2})
+        assert data[0]['url'] == url1
 
         # test new post does not affect old data
         res = self.app.get(url)
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str({name1:url1, name2:url2})
+        assert data[0]['url'] == url1
+        assert data[1]['url'] == url2
+
+    @with_context
+    def test_update_taskbrowse_bookmark(self):
+        """Test update taskbrowse_bookmark via POST works"""
+        data = self.original
+        target_project = "project1"
+        bookmark_1_data, _, _, bookmarks = self.generate_sample_bookmarks(target_project)
+        info = {
+                'taskbrowse_bookmarks' : bookmarks
+        }
+        user = UserFactory.create(info=info)
+        self.signin_user(user)
+        url = f"/account/{user.name}/taskbrowse_bookmarks/{target_project}"
+
+        # update a bookmark that already exists
+        res = self.app.post(url, json={"name":"bookmark 1", "url":"www.google.com"})
+        assert res.status_code == 200, res.status_code
+        data = json.loads(res.data)
+        assert data[0]["updated"] != bookmark_1_data["updated"]
+        assert data[0]["created"] == bookmark_1_data["created"]
+        assert data[0]["url"] == "www.google.com"
+
 
     @with_context
     def test_post_taskbrowse_bookmarks_missing_arguments(self):
@@ -10760,56 +10803,42 @@ def test_delete_taskbrowse_bookmarks(self):
         """Test deleting taskbrowse bookmarks"""
         data = self.original
         target_project = "project1"
-        bookmarks = {
-                    target_project : {
-                        "bookmark1" : "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time%22%2C%22gold_task%22%2C%22actions%22%2C%22lock_status%22%5D&order_by=task_id+asc&pcomplete_from=46&pcomplete_to=100&priority_from=0.45&priority_to=1.00&display_info_columns=%5B%5D",
-                        "bookmark2" : "https://gigwork.net/project/testproject66/tasks/browse"
-                                },
-                    "project2" : {
-                        "bookmark3" : "https://gigwork.net/project/project2/tasks/browse"
-                    }
-
-        }
+        bookmark_1_data, bookmark_2_data ,_, bookmarks = self.generate_sample_bookmarks(target_project)
         info = {
                 'taskbrowse_bookmarks' : bookmarks
             }
 
         user = UserFactory.create(info=info)
         self.signin_user(user)
         url = f"/account/{user.name}/taskbrowse_bookmarks/{target_project}"
-        data = {"name": "bookmark1"}
+        data = {"name": "bookmark 1"}
         res = self.app.delete(url, json=data)
 
-        expected_res = {"bookmark2" : "https://gigwork.net/project/testproject66/tasks/browse"}
+        expected_res = {"bookmark 2" : "https://gigwork.net/project/testproject66/tasks/browse"}
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str(expected_res)
+        print(data)
+        assert len(data) == 1
+        assert data[0]['url'] == bookmark_2_data['url']
+        assert data[0]['created'] == bookmark_2_data['created']
+        assert data[0]['updated'] == bookmark_2_data['updated']
 
         # ensure deleting last bookmark does not result in error
         url = f"/account/{user.name}/taskbrowse_bookmarks/{target_project}"
-        data = {"name" : "bookmark2"}
+        data = {"name" : "bookmark 2"}
         res = self.app.delete(url, json=data)
 
         expected_res = {}
         assert res.status_code == 200, res.status_code
         data = json.loads(res.data)
-        assert str(data) == str(expected_res)
+        assert len(data) == 0
 
     @with_context
-    def test_delete_taskbrowse_bookmarks_invalid_bookmark_name(self):
+    def test_delete_taskbrowse_bookmarks_bookmark_not_found(self):
         """Test calling delete taskbrowse bookmarks with an invalid bookmark name"""
         data = self.original
         target_project = "project1"
-        bookmarks = {
-                    target_project : {
-                                "bookmark1" : "https://gigwork.net/project/testproject66/tasks/browse/1/10?changed=true&display_columns=%5B%22task_id%22%2C%22priority%22%2C%22pcomplete%22%2C%22created%22%2C%22finish_time%22%2C%22gold_task%22%2C%22actions%22%2C%22lock_status%22%5D&order_by=task_id+asc&pcomplete_from=46&pcomplete_to=100&priority_from=0.45&priority_to=1.00&display_info_columns=%5B%5D",
-                                "bookmark2" : "https://gigwork.net/project/testproject66/tasks/browse"
-                                },
-                    "project2" : {
-                        "bookmark3" : "https://gigwork.net/project/project2/tasks/browse"
-                    }
-
-        }
+        _,_,_, bookmarks = self.generate_sample_bookmarks(target_project)
         info = {
                 'taskbrowse_bookmarks' : bookmarks
             }
@@ -10822,7 +10851,7 @@ def test_delete_taskbrowse_bookmarks_invalid_bookmark_name(self):
 
 
     @with_context
-    def test_get_taskbrowse_bookmarks_user_errors(self):
+    def test_get_taskbrowse_bookmarks_user_auth_errors(self):
         """Test retrive taskbrowse bookmarks returns errors"""
         data = self.original
         target_project = "project1"