[PR #1319] [MERGED] Extract compute_user_attribute_changes from update_user_with_transaction #1256

New issue

Closed

opened 2026-02-27 09:11:30 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 09:11:30 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/lldap/lldap/pull/1319
Author: @Copilot
Created: 10/9/2025
Status: ✅ Merged
Merged: 10/10/2025
Merged by: @nitnelave

Base: main ← Head: copilot/refactor-update-user-transaction

📝 Commits (3)

a99374d Initial plan
e5bb0b7 Extract compute_user_attribute_changes function from update_user_with_transaction
edb83b7 Pass attributes by value to avoid unnecessary clones

📊 Changes

1 file changed (+40 additions, -23 deletions)

View changed files

📝 crates/sql-backend-handler/src/sql_user_backend_handler.rs (+40 -23)

📄 Description

Analyze the update_user_with_transaction function to understand the code to extract
Create a new helper function to compute attribute updates and removals
Refactor update_user_with_transaction to use the new helper function
Run tests to ensure the refactoring doesn't break functionality
Check code formatting with cargo fmt
Address review feedback to pass attributes by value instead of by reference

Summary

Successfully extracted the attribute computation logic from update_user_with_transaction into a new helper function called compute_user_attribute_changes.

Changes Made:

Created compute_user_attribute_changes function that:
- Takes user_id, insert_attributes, delete_attributes, and schema as parameters
- Passes attributes by value (moved from request) to avoid unnecessary clones
- Returns a tuple of (update_user_attributes, remove_user_attributes)
- Contains all validation and processing logic for insert and delete attributes
Refactored update_user_with_transaction to:
- Extract insert_attributes and delete_attributes from the request and pass them to the helper
- Continue with the existing logic for updating the user and attributes
- Much cleaner and easier to understand

Testing:

All 61 tests in the lldap_sql_backend_handler crate pass, including:

test_update_user_all_values
test_update_user_some_values
test_update_user_insert_attribute
test_update_user_delete_attribute
test_update_user_replace_attribute
test_update_user_delete_avatar

The refactoring maintains complete backward compatibility and functionality while improving performance by avoiding unnecessary clones.

Original prompt

This section details on the original issue you should resolve

<issue_title>[FEATURE REQUEST] Split update_user_with_transaction</issue_title>
<issue_description>Motivation
The function is a bit too big and complex

Describe the solution you'd like
Extract code to compute the update_user_attributes and remove_user_attributes variables in a separate function.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes lldap/lldap#1318

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/lldap/lldap/pull/1319 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 10/9/2025 **Status:** ✅ Merged **Merged:** 10/10/2025 **Merged by:** [@nitnelave](https://github.com/nitnelave) **Base:** `main` ← **Head:** `copilot/refactor-update-user-transaction` --- ### 📝 Commits (3) - [`a99374d`](https://github.com/lldap/lldap/commit/a99374d5cd22d9c2a62b83399f5d3953b2b78f04) Initial plan - [`e5bb0b7`](https://github.com/lldap/lldap/commit/e5bb0b76e8ece2bff75753418005338bfb5f921d) Extract compute_user_attribute_changes function from update_user_with_transaction - [`edb83b7`](https://github.com/lldap/lldap/commit/edb83b7e1e48e9a4c280ec32ae971d79d3e2c8c8) Pass attributes by value to avoid unnecessary clones ### 📊 Changes **1 file changed** (+40 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `crates/sql-backend-handler/src/sql_user_backend_handler.rs` (+40 -23) </details> ### 📄 Description - [x] Analyze the `update_user_with_transaction` function to understand the code to extract - [x] Create a new helper function to compute attribute updates and removals - [x] Refactor `update_user_with_transaction` to use the new helper function - [x] Run tests to ensure the refactoring doesn't break functionality - [x] Check code formatting with cargo fmt - [x] Address review feedback to pass attributes by value instead of by reference ## Summary Successfully extracted the attribute computation logic from `update_user_with_transaction` into a new helper function called `compute_user_attribute_changes`. ### Changes Made: 1. Created `compute_user_attribute_changes` function that: - Takes `user_id`, `insert_attributes`, `delete_attributes`, and `schema` as parameters - Passes attributes by **value** (moved from request) to avoid unnecessary clones - Returns a tuple of `(update_user_attributes, remove_user_attributes)` - Contains all validation and processing logic for insert and delete attributes 2. Refactored `update_user_with_transaction` to: - Extract `insert_attributes` and `delete_attributes` from the request and pass them to the helper - Continue with the existing logic for updating the user and attributes - Much cleaner and easier to understand ### Testing: All 61 tests in the `lldap_sql_backend_handler` crate pass, including: - `test_update_user_all_values` - `test_update_user_some_values` - `test_update_user_insert_attribute` - `test_update_user_delete_attribute` - `test_update_user_replace_attribute` - `test_update_user_delete_avatar` The refactoring maintains complete backward compatibility and functionality while improving performance by avoiding unnecessary clones.  <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[FEATURE REQUEST] Split update_user_with_transaction</issue_title> > <issue_description>**Motivation** > The function is a bit too big and complex > > **Describe the solution you'd like** > Extract code to compute the update_user_attributes and remove_user_attributes variables in a separate function.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes lldap/lldap#1318  --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>