Skip to content

fix: override name_from_body in user resource to use Username#252

Merged
kferrone merged 6 commits intomainfrom
fix/user-apply-name-from-body
Apr 17, 2026
Merged

fix: override name_from_body in user resource to use Username#252
kferrone merged 6 commits intomainfrom
fix/user-apply-name-from-body

Conversation

@amaechiabuah
Copy link
Copy Markdown
Collaborator

Describe Changes

duploctl user apply fails with KeyError: 'Name' because the inherited DuploResourceV2.apply() calls name_from_body(body) which expects a Name key, but the user resource uses Username as its identifier field.

Overrides name_from_body() in DuploUser to return body["Username"].

Link to Issues

https://app.clickup.com/t/8655600/DUPLO-41892

PR Review Checklist

  • Thoroughly reviewed on local machine.
  • Have you added any tests
  • Make sure to note changes in Changelog

@zafarabbas
Copy link
Copy Markdown
Contributor

Task linked: DUPLO-41892 User: apply & wait fails

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix user apply by overriding name_from_body to use Username

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Override name_from_body() in DuploUser to use Username field
• Fixes duploctl user apply KeyError when identifying users
• User resource uses Username as identifier, not Name
Diagram
flowchart LR
  A["DuploResourceV2.apply()"] -->|calls| B["name_from_body()"]
  B -->|expects| C["Name key"]
  D["DuploUser override"] -->|returns| E["Username key"]
  E -->|resolves| F["KeyError fix"]
Loading

Grey Divider

File Changes

1. src/duplo_resource/user.py 🐞 Bug fix +3/-0

Override name_from_body to use Username field

• Added name_from_body() method override in DuploUser class
• Returns body["Username"] instead of inherited Name field
• Resolves KeyError when applying user resources

src/duplo_resource/user.py


2. CHANGELOG.md 📝 Documentation +1/-0

Document user apply KeyError fix

• Added entry documenting the fix for user apply KeyError
• Clarifies that user resource identifies by Username not Name

CHANGELOG.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 3, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Indexes body['Username'] 📘 Rule violation ≡ Correctness
Description
DuploUser.name_from_body() directly indexes body["Username"] without validating that body is a
dict and contains the required key. This can still raise TypeError/KeyError and violates the
requirement to validate optional bodies and required fields before use.
Code

src/duplo_resource/user.py[R13-14]

+  def name_from_body(self, body):
+    return body["Username"]
Evidence
PR Compliance ID 1 requires validating request bodies (non-None, dict-like) and required keys before
indexing. The added implementation returns body["Username"] with no checks, so missing/invalid
body input will raise implicit exceptions instead of a user-friendly domain error.

src/duplo_resource/user.py[13-14]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DuploUser.name_from_body()` indexes `body["Username"]` without validating `body` is a dict and that `Username` exists, which can raise `TypeError`/`KeyError` and violates required-field validation expectations.
## Issue Context
`args.BODY` is loaded from YAML and may be omitted or empty (YAML loader can return `None`). Compliance requires raising a clear domain error before accessing required fields.
## Fix Focus Areas
- src/duplo_resource/user.py[13-14]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. User apply missing update🐞 Bug ≡ Correctness
Description
With the new DuploUser.name_from_body() override, DuploResourceV2.apply() will successfully
resolve the username and then call self.update(name, body) when the user already exists, but
DuploUser has no update() method. This will raise AttributeError for existing users, so
duploctl user apply remains broken for the update path.
Code

src/duplo_resource/user.py[R13-15]

+  def name_from_body(self, body):
+    return body["Username"]
+
Evidence
DuploResourceV2.apply() routes to update() when find() succeeds, and after this PR the user
name can be extracted (so find() can succeed); however DuploUser defines
create()/delete()/find() but no update(), so the call will fail at runtime for existing
users.

src/duplocloud/resource.py[92-157]
src/duplo_resource/user.py[7-77]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DuploResourceV2.apply()` calls `self.update(name, body)` when the resource already exists. After overriding `DuploUser.name_from_body()` to return `Username`, `user apply` will now reach that update branch, but `DuploUser` does not implement `update()`, causing an `AttributeError` for existing users.
## Issue Context
For users, the API endpoint used in `create()` (`admin/UpdateUserRole`) appears to be the endpoint that should also handle updates.
## Fix Focus Areas
- src/duplo_resource/user.py[7-77]
## Suggested change
Either:
1) Add `update(self, name: args.NAME, body: args.BODY) -> dict` to `DuploUser` that performs the same POST to `admin/UpdateUserRole` (and any required defaulting like `State`), OR
2) Override `apply()` in `DuploUser` to call the POST endpoint directly (create-or-update) without delegating to the base `DuploResourceV2.apply()` update branch.
Ensure the method signature is compatible with the base call `update(name, body)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@duploctl
Copy link
Copy Markdown
Contributor

duploctl Bot commented Apr 3, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
3701 1433 39% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/duplo_resource/user.py 0% 🟢
TOTAL 0% 🟢

updated for commit: 38f1fff by action🐍

Comment thread src/duplo_resource/user.py
Comment thread src/duplo_resource/user.py
@amaechiabuah amaechiabuah requested a review from kferrone April 6, 2026 12:20
Comment thread src/duplo_resource/user.py Outdated
Copy link
Copy Markdown
Collaborator

@kferrone kferrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@kferrone kferrone merged commit 8135fb2 into main Apr 17, 2026
7 checks passed
@kferrone kferrone deleted the fix/user-apply-name-from-body branch April 17, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants