diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index 758907dc0ee..cd261fc9c70 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -356,7 +356,7 @@ def anonymize(self, test=False): # Clear patron's profile page: data = {'key': patron.key, 'type': '/type/delete'} - patron.set_data(data) + patron.set_data(data, "delete-profile") # Remove account information from store: del web.ctx.site.store[f'account/{username}'] diff --git a/openlibrary/catalog/add_book/__init__.py b/openlibrary/catalog/add_book/__init__.py index a96d6628ee1..392721f1e3f 100644 --- a/openlibrary/catalog/add_book/__init__.py +++ b/openlibrary/catalog/add_book/__init__.py @@ -746,7 +746,11 @@ def load_data( # noqa: PLR0912, PLR0915 comment = ( "overwrite existing edition" if existing_edition else "import new book" ) - web.ctx.site.save_many(edits, comment=comment, action='add-book') + # This is still wrong -- the `edits` list can contain works, authors, + # and editions. Some records may be new, and some may be existing. + # XXX : bulk-* + action = 'edit-edition' if existing_edition else 'add-edition' + web.ctx.site.save_many(edits, comment=comment, action=action) # Writes back `openlibrary_edition` and `openlibrary_work` to # archive.org item after successful import: diff --git a/openlibrary/core/models.py b/openlibrary/core/models.py index 70a5635a259..e978258424c 100644 --- a/openlibrary/core/models.py +++ b/openlibrary/core/models.py @@ -1179,9 +1179,9 @@ def render_link(self, cls=None): # Why nofollow? return f'{web.net.htmlquote(self.displayname)}' - def set_data(self, data): + def set_data(self, data, action): self._data = data - self._save() + self._save(action=action) class UserGroup(Thing): @@ -1209,7 +1209,11 @@ def add_user(self, userkey: str) -> None: if not any(userkey == member['key'] for member in members): members.append({'key': userkey}) self.members = members - web.ctx.site.save(self.dict(), f"Adding {userkey} to {self.key}") + web.ctx.site.save( + self.dict(), + f"Adding {userkey} to {self.key}", + action="add-usergroup-member", + ) def remove_user(self, userkey): if not web.ctx.site.get(userkey): @@ -1224,7 +1228,11 @@ def remove_user(self, userkey): break self.members = members - web.ctx.site.save(self.dict(), f"Removing {userkey} from {self.key}") + web.ctx.site.save( + self.dict(), + f"Removing {userkey} from {self.key}", + action="remove-usergroup-member", + ) class Subject(web.storage): @@ -1308,10 +1316,7 @@ def create( with RunAs(patron): web.ctx.ip = web.ctx.ip or ip - t = web.ctx.site.save( - tag, - comment=comment, - ) + t = web.ctx.site.save(tag, comment=comment, action="add-tag") return t diff --git a/openlibrary/olbase/events.py b/openlibrary/olbase/events.py index 6974113e042..a56150584b0 100644 --- a/openlibrary/olbase/events.py +++ b/openlibrary/olbase/events.py @@ -44,6 +44,7 @@ def trigger_subevents(event): "Edit by %s, changeset_id=%s, changes=%s", author, changeset["id"], keys ) + # XXX : is this triggering a save or save_many call? eventer.trigger("infobase.edit", changeset) diff --git a/openlibrary/plugins/admin/code.py b/openlibrary/plugins/admin/code.py index 61a98bb34c0..391d806748f 100644 --- a/openlibrary/plugins/admin/code.py +++ b/openlibrary/plugins/admin/code.py @@ -103,7 +103,7 @@ def revert_all_user_edits(account: Account) -> tuple[int, int]: delete_payload = [ {'key': key, 'type': {'key': '/type/delete'}} for key in keys_to_delete ] - web.ctx.site.save_many(delete_payload, 'Delete spam') + web.ctx.site.save_many(delete_payload, 'Delete spam', action="bulk-revert-spam") return edit_count, len(delete_payload) @@ -515,6 +515,9 @@ def GET(self, today): def POST(self, today): """Update stats for today.""" doc = self.get_stats(today) + # This seems like it would save a Thing + # ...but, I don't know if this endpoint is in use today. + # I suspect not.... doc._save() raise web.seeother(web.ctx.path) @@ -551,7 +554,12 @@ def block_ips(self, ips): "/admin/block", {"key": "/admin/block", "type": "/type/object"} ) page.ips = [{'ip': ip} for ip in ips] - page._save("updated blocked IPs") + # I suspect that this doesn't get called much, but it's + # important to set an action on these types of transactions. + # + # I suspect that we'd want to filter these out from the + # `/recentchanges` page. + page._save("updated blocked IPs", action="block-ips") def get_blocked_ips(): @@ -745,7 +753,9 @@ def POST(self): books = self.set_permission("/books", i.perm_records) authors = self.set_permission("/authors", i.perm_records) web.ctx.site.save_many( - [root, works, books, authors], comment="Updated edit policy." + [root, works, books, authors], + comment="Updated edit policy.", + action="bulk-update-permissions", ) add_flash_message("info", "Edit policy has been updated!") diff --git a/openlibrary/plugins/openlibrary/api.py b/openlibrary/plugins/openlibrary/api.py index 86ddd4f7ab8..2499c824b20 100644 --- a/openlibrary/plugins/openlibrary/api.py +++ b/openlibrary/plugins/openlibrary/api.py @@ -645,7 +645,8 @@ def POST(self, work_id: str): keys_to_delete: list = [el.get("key") for el in [*editions, work.dict()]] delete_payload: list[dict] = [{"key": key, "type": {"key": "/type/delete"}} for key in keys_to_delete] - web.ctx.site.save_many(delete_payload, comment) + # this deletes editions and works + web.ctx.site.save_many(delete_payload, comment, action="bulk-delete-book-records") return delegate.RawText( json.dumps( { @@ -1025,7 +1026,7 @@ def make_dark(edition): data["source_records"] = [rec for rec in source_records if not rec.startswith("ia:")] if not data["source_records"]: del data["source_records"] - web.ctx.site.save(data, "Remove OCAID: Item no longer available to borrow.") + web.ctx.site.save(data, "Remove OCAID: Item no longer available to borrow.", action="update-edition") class monthly_logins(delegate.page): diff --git a/openlibrary/plugins/openlibrary/bulk_tag.py b/openlibrary/plugins/openlibrary/bulk_tag.py index 1d4444e8a3a..1a8629a425c 100644 --- a/openlibrary/plugins/openlibrary/bulk_tag.py +++ b/openlibrary/plugins/openlibrary/bulk_tag.py @@ -69,7 +69,9 @@ def POST(self): w.dict() ) # need to convert class to raw dict in order for save_many to work - web.ctx.site.save_many(docs_to_update, comment="Bulk tagging works") + web.ctx.site.save_many( + docs_to_update, comment="Bulk tagging works", action="bulk-update-work-tags" + ) def response(msg, status="success"): return delegate.RawText( diff --git a/openlibrary/plugins/openlibrary/code.py b/openlibrary/plugins/openlibrary/code.py index 03be2bd9192..fd6d2111230 100644 --- a/openlibrary/plugins/openlibrary/code.py +++ b/openlibrary/plugins/openlibrary/code.py @@ -238,7 +238,7 @@ def sampleload(filename='sampledump.txt.gz'): with gzip.open(filename) if filename.endswith('.gz') else open(filename) as file: queries = [json.loads(line) for line in file] - print(web.ctx.site.save_many(queries)) + print(web.ctx.site.save_many(queries, action="infogami-action-sampleload")) class routes(delegate.page): @@ -350,6 +350,7 @@ def POST(self): web.ctx.site.save( {'key': key, 'name': i.name, 'type': {'key': '/type/author'}}, comment='New Author', + action="add-author", ) raise web.HTTPError('200 OK', {}, key) @@ -942,7 +943,7 @@ def POST(self, key): d = self.load(i.body) p = web.ctx.site.new(key, d) try: - p._save(i._comment) + p._save(i._comment, action="update-yaml") except (client.ClientException, ValidationException) as e: add_flash_message('error', str(e)) return render.edit_yaml(key, i.body) diff --git a/openlibrary/plugins/openlibrary/lists.py b/openlibrary/plugins/openlibrary/lists.py index e314062d10a..b0f5431ebd8 100644 --- a/openlibrary/plugins/openlibrary/lists.py +++ b/openlibrary/plugins/openlibrary/lists.py @@ -396,9 +396,10 @@ def POST(self, user_key: str | None, list_type_plural: Literal["lists", "series" # Cast is needed since the infogami code isn't typed yet records_to_save.append(cast(dict, work.dict())) + action = "add-series" if list_type_plural == "series" else "add-list" web.ctx.site.save_many( records_to_save, - action="lists", + action=action, comment=web.input(_comment="")._comment or None, ) @@ -481,7 +482,7 @@ def process_delete(doc, key): cache.memcache_cache.delete(cache_key) delete_doc = {"key": key, "type": {"key": "/type/delete"}} - web.ctx.site.save(delete_doc, action="lists", comment="Deleted list.") + web.ctx.site.save(delete_doc, action="delete-list", comment="Deleted list.") class lists_json(delegate.page): @@ -583,7 +584,7 @@ def process_new_list(user, data, site): return site.save( lst.dict(), comment="Created new list.", - action="lists", + action="add-list", data={"list": {"key": lst.key}, "seeds": seeds}, ) @@ -734,7 +735,7 @@ def process_seeds_update(lst, data, key): "remove": data["remove"], } - return lst._save(comment="Updated list.", action="lists", data=changeset_data) + return lst._save(comment="Updated list.", action="update-list-items", data=changeset_data) class list_seed_yaml(list_seeds): diff --git a/openlibrary/plugins/upstream/addbook.py b/openlibrary/plugins/upstream/addbook.py index 16d7856942a..cec8cb014b8 100644 --- a/openlibrary/plugins/upstream/addbook.py +++ b/openlibrary/plugins/upstream/addbook.py @@ -131,12 +131,14 @@ class DocSaveHelper: def __init__(self): self.docs = [] - def save(self, doc) -> None: + def save(self, doc) -> None: # total misnomer -- nothing is saved in the method """Adds the doc to the list of docs to be saved.""" if not isinstance(doc, dict): # thing doc = doc.dict() self.docs.append(doc) + # `action` is always being passed to `commit` today, but maybe not tomorrow? + # Should `action` be a positional argument for this method? def commit(self, **kw) -> None: """Saves all the collected docs.""" if self.docs: @@ -488,6 +490,7 @@ def no_match(self, saveutil: DocSaveHelper, i: web.utils.Storage) -> NoReturn: saveutil.save(edition) comment = utils.get_message("comment_add_book") + # XXX : creates both work and edition saveutil.commit(action="add-book", comment=comment) raise safe_seeother(edition.url("/edit?mode=add-work")) @@ -691,6 +694,8 @@ def save(self, formdata: web.Storage) -> None: self.edition.update(edition_data) saveutil.save(self.edition) + # XXX : `edit-book` may be misleading + # This method sometimes creates new Work and Author objects saveutil.commit(comment=comment, action="edit-book") @staticmethod @@ -705,7 +710,8 @@ def new_work(edition: Edition) -> Work: @staticmethod def delete(key, comment=""): doc = web.ctx.site.new(key, {"key": key, "type": {"key": "/type/delete"}}) - doc._save(comment=comment) + # TODO : pull type from key, then compose `delete-{type}` action string + doc._save(comment=comment, action="delete-?") def process_input(self, i): if 'edition' in i: @@ -993,13 +999,13 @@ def POST(self, key): raise web.badrequest() elif "_save" in i: author.update(formdata) - author._save(comment=i._comment) + author._save(comment=i._comment, action="update-author") raise safe_seeother(key) elif "_delete" in i: author = web.ctx.site.new( key, {"key": key, "type": {"key": "/type/delete"}} ) - author._save(comment=i._comment) + author._save(comment=i._comment, action="delete-author") raise safe_seeother(key) except (ClientException, ValidationException) as e: add_flash_message('error', str(e)) @@ -1057,6 +1063,7 @@ def POST(self, edition): raise web.redirect(web.ctx.path) edition.set_identifiers(data) saveutil.save(edition) + # `action` : edit-book-isbn? saveutil.commit(comment="Added an %s identifier." % typ, action="edit-book") add_flash_message("info", "Thank you very much for improving that record!") raise web.redirect(web.ctx.path) diff --git a/openlibrary/plugins/upstream/addtag.py b/openlibrary/plugins/upstream/addtag.py index 9356d0ef2ae..8673688181f 100644 --- a/openlibrary/plugins/upstream/addtag.py +++ b/openlibrary/plugins/upstream/addtag.py @@ -185,10 +185,10 @@ def POST(self, key): tag = web.ctx.site.new( key, {"key": key, "type": {"key": "/type/delete"}} ) - tag._save(comment=i._comment) + tag._save(comment=i._comment, action="delete-tag") raise safe_seeother(key) tag.update(formdata) - tag._save(comment=i._comment) + tag._save(comment=i._comment, action="update-tag") raise safe_seeother(i.redir or key) except (ClientException, ValidationException) as e: add_flash_message('error', str(e)) diff --git a/openlibrary/plugins/upstream/code.py b/openlibrary/plugins/upstream/code.py index 628f15bedea..13c72e46fd1 100644 --- a/openlibrary/plugins/upstream/code.py +++ b/openlibrary/plugins/upstream/code.py @@ -333,7 +333,9 @@ def revert(thing): if prev.type.key in ["/type/delete", "/type/redirect"]: return revert(prev) else: - prev._save("revert to revision %d" % prev.revision) + prev._save( + "revert to revision %d" % prev.revision, action="revert-version" + ) return prev elif thing.type.key == "/type/redirect": redirect = web.ctx.site.get(thing.location) @@ -369,7 +371,7 @@ def process(value): thing[k] = process(thing[k]) comment = i._comment or "reverted to revision %d" % v - thing._save(comment) + thing._save(comment, action="revert-version") raise web.seeother(key) diff --git a/openlibrary/plugins/upstream/covers.py b/openlibrary/plugins/upstream/covers.py index bd6a4d4fddb..6dfc203a3fd 100644 --- a/openlibrary/plugins/upstream/covers.py +++ b/openlibrary/plugins/upstream/covers.py @@ -182,7 +182,8 @@ def get_image(self, book): def save_images(self, book, covers): book.covers = covers - book._save('Update covers') + # Must check `book.key` for record type, if needed + book._save('Update covers', action="update-cover") def POST(self, key): if not accounts.get_current_user(): @@ -216,4 +217,4 @@ def get_image(self, author): def save_images(self, author, photos): author.photos = photos - author._save('Update photos') + author._save('Update photos', action="update-photo") diff --git a/scripts/update_stale_ocaid_references.py b/scripts/update_stale_ocaid_references.py index bc6b4cba39e..574137c2eae 100644 --- a/scripts/update_stale_ocaid_references.py +++ b/scripts/update_stale_ocaid_references.py @@ -135,7 +135,9 @@ def disassociate_dark_ocaids(s3_keys, es_editions, test=True): counts["updated"] += len(updated_eds) with RunAs('ImportBot'): web.ctx.ip = web.ctx.ip or '127.0.0.1' - web.ctx.site.save_many(updated_eds, comment="Redacting ocaids") + web.ctx.site.save_many( + updated_eds, comment="Redacting ocaids", action="bulk-redact-ocaid" + ) print(counts) counts["batches"] += 1 return counts