feat: expert category tagging#60
Conversation
Add category_id (u32) to ExpertRecord, propagated through add_expert, update_profile, batch_update_profiles, and preserved across ban/unban. Includes test for full category_id lifecycle. Closes LightForgeHub#41 Co-Authored-By: Claude Opus 4.6 <[email protected]>
📝 WalkthroughWalkthroughThe PR adds a category_id field to expert records, enabling experts to be categorizable by numerical tags. All functions in the contract, storage, and public API layers are updated to accept and persist this new field, with comprehensive test coverage including persistence verification across various operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
contracts/identity-registry-contract/src/contract.rs (1)
224-257: Consider adding documentation for the tuple field order.The tuple
(Address, String, u32, u32)represents(expert, uri, status, category_id). While the code is correct, the field order isn't immediately obvious to callers.📝 Suggested documentation improvement
/// Batch update expert profiles (Admin only) /// Allows admins to update multiple expert metadata URIs in a single transaction +/// Each tuple is (expert_address, new_uri, status_u32, category_id) pub fn batch_update_profiles( env: &Env, updates: Vec<(Address, String, u32, u32)>, ) -> Result<(), RegistryError> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/identity-registry-contract/src/contract.rs` around lines 224 - 257, The tuple parameter in batch_update_profiles is ambiguous to callers—document the expected field order (Address, String, u32, u32) as (expert, uri, status, category_id) by adding a doc comment above the batch_update_profiles function (or replace the tuple with a named type like ProfileUpdate or a type alias) so callers clearly understand the parameter ordering; reference the function name batch_update_profiles and the tuple used in its signature so reviewers can locate and update the API docs or refactor to a named struct for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@contracts/identity-registry-contract/src/contract.rs`:
- Around line 224-257: The tuple parameter in batch_update_profiles is ambiguous
to callers—document the expected field order (Address, String, u32, u32) as
(expert, uri, status, category_id) by adding a doc comment above the
batch_update_profiles function (or replace the tuple with a named type like
ProfileUpdate or a type alias) so callers clearly understand the parameter
ordering; reference the function name batch_update_profiles and the tuple used
in its signature so reviewers can locate and update the API docs or refactor to
a named struct for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4dfa7123-361e-447c-b9da-847c141b06fc
📒 Files selected for processing (5)
contracts/identity-registry-contract/src/contract.rscontracts/identity-registry-contract/src/lib.rscontracts/identity-registry-contract/src/storage.rscontracts/identity-registry-contract/src/test.rscontracts/identity-registry-contract/src/types.rs
Summary
Closes #41
Test plan
Summary by CodeRabbit