-
Notifications
You must be signed in to change notification settings - Fork 279
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement server-side bookmarks (#2843)
* Implement basic CRUD operations for bookmarks * Delete bookmarks when entity is deleted * Run Alembic migrations in test environment Before, we created the database schema based on model classes which meant we'd end up with a slightly different schema in test and dev/prod environments. * Add endpoint to migrate bookmarks from client-side storage * Return whether entity is bookmarked in entity API response * Remove warning popover when using bookmarks feature for the first time * Load bookmarks from API * Automatically migrate local bookmarks * Extend data fetching logic to support partial invalidations As a reminder to my future self: In Aleph’s frontend, we use our own mini data fetching framework built on top of Redux. One thing it does is caching data from API responses. For example, when a user views their bookmarks, does something else and then views the bookmarks again, the bookmarks are only fetched once. When viewing the bookmarks the second time, we render them based on a runtime cache. This can lead to outdated data being displayed. For example, when the user creates a new bookmark *after* the bookmarks have been loaded, the list of bookmarks would be outdated. Our mini framework does handle data invalidation, but only globally, for all cached data. That works ok in most cases, but for bookmarks, it leads to a bad UX. When you view an entity, then click on the bookmarks button, it would cause the entire page (all the data about the entity) to reload, even though none of that data has changed. The only thing that has changed is the list of bookmarks. We handle data invalidation by storing the timestamp when a data object was loaded and the timestamp of the last mutation. Whenever we render cached data, we check whether the cached data might be outdated (i.e. when it has been loaded before the latest mutation). Until now, we only stored one global mutation timestamp. Whenever that timestamp was updated, all cached data became outdated. Now, in addition to the global mutation timestamp, we have an option to store mutation timestamp for specific subsets of the cached data. So when creating or deleting a bookmark, instead of updating the global mutation timestamp (which would invalidate all cached data), we can update the timestamp for the `bookmarks` mutation key. This would invalidate only cached bookmarks, but no other data. * Actually commit ORM session to execute queries * Update wording
- Loading branch information
1 parent
a26ea5c
commit e140997
Showing
31 changed files
with
1,040 additions
and
321 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
41 changes: 41 additions & 0 deletions
41
aleph/migrate/versions/c52a1f469ac7_create_bookmark_table.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
"""create bookmark table | ||
Revision ID: c52a1f469ac7 | ||
Revises: 274270e01613 | ||
Create Date: 2023-01-30 08:53:59.370110 | ||
""" | ||
|
||
# revision identifiers, used by Alembic. | ||
from alembic import op | ||
import sqlalchemy as sa | ||
|
||
revision = "c52a1f469ac7" | ||
down_revision = "274270e01613" | ||
|
||
|
||
def upgrade(): | ||
op.create_table( | ||
"bookmark", | ||
sa.Column("id", sa.Integer(), primary_key=True), | ||
sa.Column("created_at", sa.DateTime(), nullable=False), | ||
sa.Column("role_id", sa.Integer(), sa.ForeignKey("role.id"), nullable=False), | ||
sa.Column("entity_id", sa.String(length=128), nullable=False), | ||
sa.Column( | ||
"collection_id", | ||
sa.Integer(), | ||
sa.ForeignKey("collection.id"), | ||
nullable=False, | ||
), | ||
sa.UniqueConstraint("role_id", "entity_id"), | ||
) | ||
|
||
op.create_index( | ||
op.f("ix_bookmark_role_id_collection_id_created_at"), | ||
"bookmark", | ||
["role_id", "collection_id", "created_at"], | ||
) | ||
|
||
|
||
def downgrade(): | ||
op.drop_table("bookmark") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
from datetime import datetime | ||
from normality import stringify | ||
from aleph.core import db | ||
from aleph.model.common import ENTITY_ID_LEN, IdModel | ||
|
||
|
||
class Bookmark(db.Model, IdModel): | ||
"""A bookmark of an entity created by a user.""" | ||
|
||
created_at = db.Column(db.DateTime, default=datetime.utcnow) | ||
role_id = db.Column(db.Integer, db.ForeignKey("role.id")) | ||
collection_id = db.Column(db.Integer, db.ForeignKey("collection.id")) | ||
entity_id = db.Column(db.String(ENTITY_ID_LEN)) | ||
|
||
def to_dict(self): | ||
return { | ||
"id": stringify(self.id), | ||
"created_at": self.created_at, | ||
"entity_id": self.entity_id, | ||
"collection_id": self.collection_id, | ||
} | ||
|
||
@classmethod | ||
def delete_by_entity(cls, entity_id): | ||
query = db.session.query(Bookmark) | ||
query = query.filter(Bookmark.entity_id == entity_id) | ||
query.delete(synchronize_session=False) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,290 @@ | ||
import json | ||
import datetime | ||
from aleph.index.entities import index_entity | ||
from aleph.tests.util import TestCase, JSON | ||
from aleph.model import Bookmark | ||
from aleph.core import db | ||
|
||
|
||
class BookmarksApiTestCase(TestCase): | ||
def setUp(self): | ||
super(BookmarksApiTestCase, self).setUp() | ||
|
||
self.role, self.headers = self.login() | ||
self.collection = self.create_collection(self.role, label="Politicians") | ||
|
||
data = {"schema": "Person", "properties": {"name": "Angela Merkel"}} | ||
self.entity = self.create_entity(data=data, collection=self.collection) | ||
index_entity(self.entity) | ||
|
||
def test_bookmarks_index_auth(self): | ||
res = self.client.get("/api/2/bookmarks") | ||
assert res.status_code == 403, res | ||
|
||
res = self.client.get("/api/2/bookmarks", headers=self.headers) | ||
assert res.status_code == 200, res | ||
assert res.json["total"] == 0, res.json | ||
|
||
def test_bookmarks_index_results(self): | ||
other_role, _ = self.login("tester2") | ||
other_entity = self.create_entity( | ||
data={"schema": "Person", "properties": {"name": "Barack Obama"}}, | ||
collection=self.collection, | ||
) | ||
index_entity(other_entity) | ||
|
||
bookmarks = [ | ||
Bookmark( | ||
entity_id=self.entity.id, | ||
collection_id=self.entity.collection_id, | ||
role_id=self.role.id, | ||
), | ||
Bookmark( | ||
entity_id=other_entity.id, | ||
collection_id=self.entity.collection_id, | ||
role_id=other_role.id, | ||
), | ||
] | ||
db.session.add_all(bookmarks) | ||
db.session.commit() | ||
|
||
res = self.client.get("/api/2/bookmarks", headers=self.headers) | ||
assert res.status_code == 200, res | ||
assert res.json["total"] == 1, res.json | ||
entity = res.json["results"][0]["entity"] | ||
assert entity["properties"]["name"][0] == "Angela Merkel", res.json | ||
|
||
def test_bookmarks_index_order(self): | ||
other_entity = self.create_entity( | ||
data={"schema": "Person", "properties": {"name": "Barack Obama"}}, | ||
collection=self.collection, | ||
) | ||
index_entity(other_entity) | ||
|
||
old_bookmark = Bookmark( | ||
entity_id=self.entity.id, | ||
collection_id=self.entity.collection_id, | ||
role_id=self.role.id, | ||
created_at=datetime.date(2005, 11, 22), | ||
) | ||
new_bookmark = Bookmark( | ||
entity_id=other_entity.id, | ||
collection_id=other_entity.collection_id, | ||
role_id=self.role.id, | ||
created_at=datetime.date(2009, 1, 20), | ||
) | ||
db.session.add_all([old_bookmark, new_bookmark]) | ||
db.session.commit() | ||
|
||
res = self.client.get("/api/2/bookmarks", headers=self.headers) | ||
assert res.status_code == 200, res | ||
assert res.json["total"] == 2, res.json | ||
first = res.json["results"][0]["entity"] | ||
second = res.json["results"][1]["entity"] | ||
assert first["properties"]["name"][0] == "Barack Obama", first | ||
assert second["properties"]["name"][0] == "Angela Merkel", second | ||
|
||
def test_bookmarks_index_access(self): | ||
other_role = self.create_user(foreign_id="other") | ||
secret_collection = self.create_collection(other_role, label="Top Secret") | ||
|
||
data = {"schema": "Person", "properties": {"name": ["Mister X"]}} | ||
secret_entity = self.create_entity(data, secret_collection) | ||
index_entity(secret_entity) | ||
|
||
bookmark = Bookmark( | ||
entity_id=secret_entity.id, | ||
collection_id=secret_entity.collection_id, | ||
role_id=self.role.id, | ||
) | ||
db.session.add(bookmark) | ||
db.session.commit() | ||
|
||
res = self.client.get("/api/2/bookmarks", headers=self.headers) | ||
|
||
assert res.status_code == 200, res | ||
assert res.json["total"] == 0, res.json | ||
assert len(res.json["results"]) == 0, res.json | ||
|
||
def test_bookmarks_create(self): | ||
count = Bookmark.query.count() | ||
assert count == 0, count | ||
|
||
res = self.client.post( | ||
"/api/2/bookmarks", | ||
headers=self.headers, | ||
data=json.dumps({"entity_id": self.entity.id}), | ||
content_type=JSON, | ||
) | ||
assert res.status_code == 201, res | ||
entity = res.json.get("entity") | ||
assert entity["properties"]["name"][0] == "Angela Merkel", res.json | ||
|
||
count = Bookmark.query.count() | ||
assert count == 1, count | ||
bookmark = Bookmark.query.first() | ||
assert bookmark.entity_id == self.entity.id, bookmark.entity_id | ||
assert bookmark.role_id == self.role.id, bookmark.role_id | ||
|
||
def test_bookmarks_create_validate_access(self): | ||
other_role = self.create_user(foreign_id="other") | ||
secret_collection = self.create_collection(other_role, label="Top Secret") | ||
|
||
data = {"schema": "Person", "properties": {"name": ["Mister X"]}} | ||
secret_entity = self.create_entity(data, secret_collection) | ||
index_entity(secret_entity) | ||
|
||
res = self.client.post( | ||
"/api/2/bookmarks", | ||
headers=self.headers, | ||
data=json.dumps({"entity_id": secret_entity.id}), | ||
content_type=JSON, | ||
) | ||
assert res.status_code == 400, res | ||
message = res.json["message"] | ||
assert message.startswith("Could not bookmark the given entity"), message | ||
|
||
count = Bookmark.query.count() | ||
assert count == 0, count | ||
|
||
def test_bookmarks_create_validate_exists(self): | ||
invalid_entity_id = self.create_entity( | ||
{"schema": "Company"}, self.collection | ||
).id | ||
|
||
res = self.client.post( | ||
"/api/2/bookmarks", | ||
headers=self.headers, | ||
data=json.dumps({"entity_id": invalid_entity_id}), | ||
content_type=JSON, | ||
) | ||
assert res.status_code == 400, res | ||
message = res.json["message"] | ||
assert message.startswith("Could not bookmark the given entity"), message | ||
|
||
count = Bookmark.query.count() | ||
assert count == 0, count | ||
|
||
def test_bookmarks_create_idempotent(self): | ||
count = Bookmark.query.count() | ||
assert count == 0, count | ||
|
||
res = self.client.post( | ||
"/api/2/bookmarks", | ||
headers=self.headers, | ||
data=json.dumps({"entity_id": self.entity.id}), | ||
content_type=JSON, | ||
) | ||
assert res.status_code == 201 | ||
|
||
count = Bookmark.query.count() | ||
assert count == 1, count | ||
|
||
res = self.client.post( | ||
"/api/2/bookmarks", | ||
headers=self.headers, | ||
data=json.dumps({"entity_id": self.entity.id}), | ||
content_type=JSON, | ||
) | ||
assert res.status_code == 201 | ||
|
||
count = Bookmark.query.count() | ||
assert count == 1, count | ||
|
||
def test_bookmarks_delete(self): | ||
bookmark = Bookmark( | ||
entity_id=self.entity.id, | ||
collection_id=self.entity.collection_id, | ||
role_id=self.role.id, | ||
) | ||
db.session.add(bookmark) | ||
db.session.commit() | ||
|
||
count = Bookmark.query.count() | ||
assert count == 1, count | ||
|
||
res = self.client.delete( | ||
f"/api/2/bookmarks/{self.entity.id}", headers=self.headers | ||
) | ||
assert res.status_code == 204, res | ||
|
||
count = Bookmark.query.count() | ||
assert count == 0, count | ||
|
||
def test_bookmarks_delete_idempotent(self): | ||
count = Bookmark.query.count() | ||
assert count == 0, count | ||
|
||
res = self.client.delete( | ||
f"/api/2/bookmarks/{self.entity.id}", headers=self.headers | ||
) | ||
assert res.status_code == 204, res | ||
|
||
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 |
Oops, something went wrong.