Fix metadata cleanup loop to use variable k instead of literal attribute

[solarrezaei11]

Summary

Fixes #13016

The loop in author_import_record_to_author (load_book.py:324) was using existing.k which attempts to access a literal attribute named "k" rather than the loop variable k. Since no such attribute exists, the condition was always false and the metadata fields (last_modified, id, revision, created) were never cleaned up.

Change

# Before
for k in "last_modified", "id", "revision", "created":
    if existing.k:
        del existing.k

# After
for k in "last_modified", "id", "revision", "created":
    if k in existing:
        del existing[k]

This ensures the loop variable is used correctly as a dictionary key, so the metadata cleanup actually runs.

[openlibrary-bot]

Thank you @solarrezaei11 for submitting this PR! Welcome to Open Library — looks like this is your first contribution here. 🎉

🤖 Copilot has been assigned for an initial review.

The linked issue (#13016) hasn't been triaged yet — triage happens on Mondays and Fridays. There are currently ~185 open non-draft PRs ahead of yours.

PR triage checklist (maintainers / Pierre)

• PR description — explains the bug and shows a before/after diff
• References an issue — Fixes #13016
  • Linked issue is triaged — bug(add_book): metadata cleanup loop uses literal attribute 'k' instead of variable k #13016 has no Priority: * label yet
  • Linked issue is assigned — no assignee on bug(add_book): metadata cleanup loop uses literal attribute 'k' instead of variable k #13016
• Commit history clean — 1 commit with a clear message
• CI passing — pre-commit.ci is still queued
• Test cases present — small 2-line fix; existing tests cover the load_book path
• Proof of testing — no explicit test run described (the before/after code makes the logic clear, but a brief note on how this was verified locally would help reviewers)

Note

This comment was automatically generated by PAM, Open Library's Project AI Manager. PAM provides status visibility, performs basic project management functions, and gives actionable feedback so contributors aren't left waiting.

[Copilot]

Pull request overview

Fixes the author-metadata cleanup loop in author_import_record_to_author so it correctly uses the loop variable as a dict key when removing internal metadata fields from a matched existing author record (per #13016).

Changes:

• Replace incorrect attribute access (existing.k) with dictionary key checks/deletions (k in existing, del existing[k]) for metadata fields (last_modified, id, revision, created).

[munzzyy]

Went through this to see if #13016 was still open for grabs, but the fix here already matches what the issue asks for. existing.k was hitting a literal attribute instead of the loop variable, and k in existing / del existing[k] is the right call since existing is an Infobase Thing (dict-style access), not a plain object.

One gap: no test exercises this path, so a future refactor could reintroduce the same typo without anything catching it. TestImportAuthor in openlibrary/catalog/add_book/tests/test_load_book.py already has the pieces for this (mock_site.save() sets revision, last_modified, and created on the saved doc per mock_infobase.py), so something like this would pin it down:

def test_existing_author_metadata_fields_are_stripped(self, mock_site):
    """last_modified, revision, and created should be stripped from a
    matched existing author before it's returned, so stale metadata
    isn't sent back on the next save."""
    author = {
        "name": "William H. Brewer",
        "key": "/authors/OL3A",
        "type": {"key": "/type/author"},
    }
    mock_site.save(author)

    found = author_import_record_to_author({"name": "William H. Brewer"})

    for field in ("last_modified", "revision", "created"):
        assert field not in found

(Left id out since the mock site doesn't actually set it, so asserting on it wouldn't prove anything either way.)

Didn't have a working Python/infogami setup on hand to run this against docker, so I only traced it through by hand rather than executing it — worth double-checking against a real run before merging. Fix itself looks correct either way.