Remove stale expired-permissions scheduled jobs during migration

[Copilot]

Sites upgraded from older Drive versions may retain drive.api.permissions.auto_delete_expired_perms, which queries the removed valid_until field and fails with an unknown-column error.

• Migration cleanup
  • Added a post-model-sync patch targeting obsolete scheduled job records.
  • Removes jobs whose method is:

    drive.api.permissions.auto_delete_expired_perms

• Patch registration
  • Registered the cleanup in drive/patches.txt so affected sites are repaired during migration.

[greptile-apps]

Confidence Score: 4/5

The cleanup is narrowly scoped, but the repository’s migration-test requirement should be satisfied before merging.

_{Reviews (1) · Last reviewed commit: "Remove stale expired permissions job"}

[greptile-apps]

Migration Lacks Regression Coverage

This destructive migration has no corresponding test. The repository requires data-patch coverage, so add a test that creates matching and unrelated Scheduled Job Type rows and verifies that execute() deletes only the obsolete job before merging.

Context Used: Guidelines for reviewing Frappe Framework applications. (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: drive/patches/remove_expired_permissions_job.py
Line: 5

Comment:
**Migration Lacks Regression Coverage**

This destructive migration has no corresponding test. The repository requires data-patch coverage, so add a test that creates matching and unrelated Scheduled Job Type rows and verifies that `execute()` deletes only the obsolete job before merging.

**Context Used:** Guidelines for reviewing Frappe Framework applications. ([source](https://github.com/frappe/skills/blob/main/skills/quality-code-review/SKILL.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[BreadGenie]

why do you need tests for patches???

[greptile-apps]

You don't inherently need tests for every patch, and I overstated that the repository requires patch coverage.

I suggested one here because drive/patches/remove_expired_permissions_job.py:5 performs a destructive database delete. A small regression test would verify that the exact obsolete method is removed while unrelated Scheduled Job Type rows remain, protecting against an overly broad filter or typo. It also documents the intended migration behavior for future changes.

That said, this is a simple, idempotent cleanup, so if the project convention is to leave one-off data patches untested, the test can reasonably be omitted.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.