Ensure action is always set when records are saved

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}']

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:

openlibrary/core/models.py

@@ -1179,9 +1179,9 @@ def render_link(self, cls=None):
         # Why nofollow?
         return f'<a rel="nofollow" href="{self.key}" {extra_attrs}>{web.net.htmlquote(self.displayname)}</a>'
 
-    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
 
 

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)
 
 

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!")

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):

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(

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)

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):

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)

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))

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)
 
 

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")

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