Remove bookmarks migration

When we first implemented bookmarks as an experimental feature, they were stored in local storage. People started using the feature so we decided to continue working on bookmarks to build it out into a mature feature.

Most importantly, this included storing bookmarks on the server-side in order to support growing numbers of bookmarks per user and to prevent users from losing their bookmarks when clearing browsing data/switching devices.

We also needed to migrate bookmarks that were currently stored locally to the server side. That means the code base had quite some code that only handled migrating the "legacy" local bookmarks to the server. As it’s been a while since we started migrating to server-side bookmarks, it should be safe to remove that migration logic now.

Fix #2864

Author: tillprochaska

aleph/tests/test_bookmarks_api.py

@@ -222,69 +222,3 @@ def test_bookmarks_delete_idempotent(self):
 
         count = Bookmark.query.count()
         assert count == 0, count
-
-    def test_bookmarks_migrate(self):
-        other_entity = self.create_entity(
-            data={"schema": "Person", "properties": {"name": "Barack Obama"}},
-            collection=self.collection,
-        )
-        index_entity(other_entity)
-
-        count = Bookmark.query.count()
-        assert count == 0, count
-
-        body = [
-            {"entity_id": self.entity.id, "created_at": "2005-11-22T00:00:00Z"},
-            {"entity_id": other_entity.id, "created_at": "2009-01-20T00:00:00Z"},
-        ]
-        res = self.client.post(
-            "/api/2/bookmarks/migrate",
-            headers=self.headers,
-            data=json.dumps(body),
-            content_type=JSON,
-        )
-        assert res.status_code == 201, res
-        assert res.json["errors"] == [], res.json
-
-        count = Bookmark.query.count()
-        assert count == 2, count
-
-        bookmarks = Bookmark.query.all()
-        assert bookmarks[0].entity_id == self.entity.id
-        assert bookmarks[0].created_at == datetime.datetime(2005, 11, 22), bookmarks[0]
-        assert bookmarks[1].entity_id == other_entity.id
-        assert bookmarks[1].created_at == datetime.datetime(2009, 1, 20), bookmarks[1]
-
-    def test_bookmarks_migrate_invalid_entity_id(self):
-        invalid_entity_id = self.create_entity(
-            {"schema": "Company"}, self.collection
-        ).id
-
-        body = [
-            {"entity_id": self.entity.id, "created_at": "2022-01-01T00:00:00Z"},
-            {"entity_id": invalid_entity_id, "created_at": "2022-01-01T00:00:00Z"},
-        ]
-        res = self.client.post(
-            "/api/2/bookmarks/migrate",
-            headers=self.headers,
-            data=json.dumps(body),
-            content_type=JSON,
-        )
-        assert res.status_code == 201, res
-        assert res.json["errors"] == [invalid_entity_id], res.json
-
-        count = Bookmark.query.count()
-        assert count == 1, count
-
-        bookmarks = Bookmark.query.all()
-        assert bookmarks[0].created_at == datetime.datetime(2022, 1, 1), bookmarks[0]
-        assert bookmarks[0].entity_id == self.entity.id
-
-    def test_bookmarks_migrate_required_timestamp(self):
-        res = self.client.post(
-            "/api/2/bookmarks/migrate",
-            headers=self.headers,
-            data=json.dumps([{"entity_id": self.entity.id}]),
-            content_type=JSON,
-        )
-        assert res.status_code == 400, res

aleph/validation/schema/bookmark.yml

@@ -25,18 +25,3 @@ BookmarkCreate:
       format: entity-id
   required:
     - entity_id
-
-BookmarkMigrate:
-  type: array
-  items:
-    type: object
-    properties:
-      entity_id:
-        type: string
-        format: entity-id
-      created_at:
-        type: string
-        format: date-time
-    required:
-      - entity_id
-      - created_at

aleph/views/bookmarks_api.py

@@ -1,7 +1,6 @@
 import logging
 from flask import Blueprint, request
 from werkzeug.exceptions import NotFound, Forbidden, BadRequest
-from sqlalchemy.dialects.postgresql import insert as postgres_insert
 
 from aleph.core import db
 from aleph.search import DatabaseQueryResult
@@ -124,58 +123,3 @@ def delete(entity_id):
     db.session.commit()
 
     return "", 204
-
-
-@blueprint.route("/api/2/bookmarks/migrate", methods=["POST"])
-def migrate():
-    """Migrate bookmarks.
-    ---
-    post:
-      summary: Migrate bookmarks
-      tags: [Bookmarks]
-      requestBody:
-        content:
-          application/json:
-            schema:
-              $ref: '#/components/schemas/BookmarkMigrate'
-      responses:
-        '201':
-          description: Created
-          content:
-            application/json:
-              schema:
-                type: object
-                properties:
-                  errors:
-                    items:
-                      type: string
-                      format: entity-id
-    """
-    require(request.authz.session_write)
-    data = parse_request("BookmarkMigrate")
-    values = []
-    errors = []
-
-    for bookmark in data:
-        try:
-            entity = get_index_entity(bookmark.get("entity_id"))
-        except (NotFound, Forbidden):
-            errors.append(bookmark.get("entity_id"))
-            continue
-        values.append(
-            {
-                "role_id": request.authz.id,
-                "entity_id": bookmark.get("entity_id"),
-                "collection_id": entity.get("collection_id"),
-                "created_at": bookmark.get("created_at"),
-            }
-        )
-
-    stmt = postgres_insert(Bookmark).values(values)
-    stmt = stmt.on_conflict_do_nothing(
-        index_elements=[Bookmark.role_id, Bookmark.entity_id],
-    )
-    db.session.execute(stmt)
-    db.session.commit()
-
-    return jsonify({"errors": errors}, 201)

ui/src/actions/bookmarkActions.js

@@ -17,15 +17,3 @@ export const deleteBookmark = asyncActionCreator(
   (entity) => async () => await endpoint.delete(`bookmarks/${entity.id}`),
   { name: 'DELETE_BOOKMARK' }
 );
-
-export const migrateLocalBookmarks = asyncActionCreator(
-  (bookmarks) => async () => {
-    const body = bookmarks.map((bookmark) => ({
-      entity_id: bookmark.id,
-      created_at: new Date(bookmark.bookmarkedAt).toISOString(),
-    }));
-    const response = await endpoint.post('bookmarks/migrate', body);
-    return { bookmarks, response };
-  },
-  { name: 'MIGRATE_BOOKMARKS' }
-);

ui/src/actions/index.js

@@ -70,7 +70,6 @@ export {
   queryBookmarks,
   createBookmark,
   deleteBookmark,
-  migrateLocalBookmarks,
 } from './bookmarkActions';
 
 export { createAction };

ui/src/app/storage.js

@@ -6,14 +6,7 @@ export const loadState = () => {
       return {};
     }
 
-    const storedState = JSON.parse(json);
-
-    // TODO: Remove after deadline
-    // See https://github.com/alephdata/aleph/issues/2864
-    storedState.localBookmarks = storedState.bookmarks;
-    delete storedState.bookmarks;
-
-    return storedState;
+    return JSON.parse(json);
   } catch (e) {
     // eslint-disable-next-line
     console.error('could not load state', e);

ui/src/app/store.js

@@ -21,16 +21,11 @@ const store = createStore(
 
 store.subscribe(
   throttle(() => {
-    const { session, config, messages, localBookmarks } = store.getState();
+    const { session, config, messages } = store.getState();
 
     saveState({
       session,
       config,
-
-      // TODO: Remove after deadline
-      // See https://github.com/alephdata/aleph/issues/2864
-      bookmarks: localBookmarks,
-
       // Do not persist the actual messages, only the dismissed message IDs.
       messages: { ...messages, messages: [] },
     });

ui/src/components/common/BookmarksDrawer.tsx

@@ -2,8 +2,6 @@ import { FC } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { useSelector } from 'react-redux';
 import { Classes, Drawer, DrawerSize } from '@blueprintjs/core';
-import { selectLocalBookmarks, selectConfigValue } from 'selectors';
-import BookmarksMigration from './BookmarksMigration';
 import BookmarksList from './BookmarksList';
 
 import './BookmarksDrawer.scss';
@@ -17,12 +15,6 @@ const BookmarksDrawer: FC<BookmarksDrawerProps> = ({
   isOpen,
   toggleDialog,
 }) => {
-  const localBookmarks = useSelector(selectLocalBookmarks);
-  const migrationCompleted = useSelector((state) =>
-    selectConfigValue(state, 'bookmarksMigrationCompleted')
-  );
-  const showMigration = localBookmarks.length > 0 && !migrationCompleted;
-
   const title = (
     <FormattedMessage id="bookmarks.title" defaultMessage="Your bookmarks" />
   );
@@ -39,11 +31,7 @@ const BookmarksDrawer: FC<BookmarksDrawerProps> = ({
     >
       <div className={Classes.DRAWER_BODY}>
         <div className="BookmarksDrawer__content">
-          {showMigration ? (
-            <BookmarksMigration />
-          ) : (
-            <BookmarksList onNavigate={toggleDialog} />
-          )}
+          <BookmarksList onNavigate={toggleDialog} />
         </div>
       </div>
     </Drawer>