fix(patch): Merge roles: bulk insert (per chunk) (backport #6422)

[mergify]

---

This is an automatic backport of pull request #6422 done by [Mergify](https://mergify.com).

[greptile-apps]

Greptile Summary

This backport refactors the merge_press_admin_member_roles patch to use a bulk INSERT per chunk instead of one INSERT per user, reducing database round-trips from O(N) to O(N/chunk_size).

• The frappe.qb query object is now built once outside the inner loop, with each user's row accumulated via repeated .insert() calls, and a single .run() fires the combined multi-row INSERT at the end of each chunk.
• frappe.generate_hash length was bumped from 8 to 10 characters to lower the birthday-collision probability.
• frappe.utils.now() is called once per chunk, so all rows in a chunk share identical creation/modified timestamps (cosmetic, no functional impact).

Confidence Score: 4/5

The patch is a one-time data migration; the bulk-insert refactor is correct for all non-empty chunks.

The core logic of accumulating rows into a single PyPika query and running it once per chunk is correct. The only edge worth noting is that query.run() on a no-rows query would produce a SQL error, but the current loop structure prevents that. The hash-length bump and shared timestamp are intentional.

press/patches/v0_8_0/merge_press_admin_member_roles.py - worth confirming the bulk-insert accumulation pattern behaves as expected with the version of frappe-qb in use.

Important Files Changed

Filename	Overview
press/patches/v0_8_0/merge_press_admin_member_roles.py	Bulk-insert optimization: query object built and run once per chunk instead of per user; hash length bumped 8→10; timestamps are now shared across all users in a chunk.

Sequence Diagram

sequenceDiagram
    participant Patch as execute()
    participant DB as Database

    Patch->>DB: SELECT users without Press User role
    DB-->>Patch: users[]

    loop for each chunk of 100 users
        Patch->>Patch: build INSERT query with all chunk rows
        Patch->>DB: INSERT INTO has_role VALUES (u1), (u2), ...
        DB-->>Patch: ok
        Patch->>DB: COMMIT
    end

    Patch->>DB: DELETE FROM has_role WHERE role IN old_roles
    Patch->>DB: "UPDATE has_role SET role=Press User"
    loop for each old role
        Patch->>DB: "UPDATE role SET disabled=1"
    end
    Patch->>DB: COMMIT

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
press/patches/v0_8_0/merge_press_admin_member_roles.py:51-65
If `total` is 0 the outer loop never executes, which is fine. But if PyPika's accumulated `query` object (with no `.insert()` calls) were ever reached, `.run()` would emit a bare `INSERT INTO has_role (...) VALUES` with no rows — a SQL syntax error. The inner loop guard prevents this today, but adding an explicit check makes the intent clear and protects against future refactors that might reorder the loops.

```suggestion
		for i, user in enumerate(chunk, start=chunk_start):
			name = frappe.generate_hash(length=10)
			query = query.insert(
				name,
				now_str,
				now_str,
				"Administrator",
				"Administrator",
				user,
				"roles",
				"User",
				"Press User",
			)
			update_progress_bar("Updating users", i, total)
		if chunk:
			query.run()
```

_{Reviews (1): Last reviewed commit: "fix(patch): Merge roles: bulk insert (pe..." | Re-trigger Greptile}

[greptile-apps]

If total is 0 the outer loop never executes, which is fine. But if PyPika's accumulated query object (with no .insert() calls) were ever reached, .run() would emit a bare INSERT INTO has_role (...) VALUES with no rows — a SQL syntax error. The inner loop guard prevents this today, but adding an explicit check makes the intent clear and protects against future refactors that might reorder the loops.

Suggested change

	for i, user in enumerate(chunk, start=chunk_start):
	name = frappe.generate_hash(length=10)
	query = query.insert(
	name,
	now_str,
	now_str,
	"Administrator",
	"Administrator",
	user,
	"roles",
	"User",
	"Press User",
	)
	update_progress_bar("Updating users", i, total)
	if chunk:
	query.run()

Prompt To Fix With AI

This is a comment left during a code review.
Path: press/patches/v0_8_0/merge_press_admin_member_roles.py
Line: 51-65

Comment:
If `total` is 0 the outer loop never executes, which is fine. But if PyPika's accumulated `query` object (with no `.insert()` calls) were ever reached, `.run()` would emit a bare `INSERT INTO has_role (...) VALUES` with no rows — a SQL syntax error. The inner loop guard prevents this today, but adding an explicit check makes the intent clear and protects against future refactors that might reorder the loops.

```suggestion
		for i, user in enumerate(chunk, start=chunk_start):
			name = frappe.generate_hash(length=10)
			query = query.insert(
				name,
				now_str,
				now_str,
				"Administrator",
				"Administrator",
				user,
				"roles",
				"User",
				"Press User",
			)
			update_progress_bar("Updating users", i, total)
		if chunk:
			query.run()
```

How can I resolve this? If you propose a fix, please make it concise.

[frappe-pr-bot]

🎉 This PR is included in version 0.34.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀