ipauser: Fix query state error handling and per-field conversion errors

[t-woerner]

query_convert_result now reports the failing field name per-key instead of aborting the whole loop with an unrelated try/except scope. main() also rejects state=query with action=member and query is excluded from rename's generated args and PARAM_MAPPING query fields.

Summary by Sourcery

Improve ipauser query handling by reporting per-field conversion errors and tightening invalid state/action and parameter combinations for query operations.

Bug Fixes:

• Ensure query_convert_result reports conversion failures for the specific field instead of aborting processing with a generic error.
• Reject use of state=query with action=member and disallow users parameter for query state to prevent invalid query invocations.
• Exclude rename from query-related parameter mapping and generated query arguments to avoid improper query behavior.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In check_parameters(), the new state == "query" fail_json makes the subsequent state == "query" branch that appends "users" to invalid unreachable; consider consolidating these into a single block or removing the dead code.
• The error message "users can not be used with state=query" in main() is still raised when state != "query" (same as before), which can be confusing for users; consider updating the message to reflect the actual condition under which it fires.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In check_parameters(), the new state == "query" fail_json makes the subsequent state == "query" branch that appends "users" to invalid unreachable; consider consolidating these into a single block or removing the dead code.
- The error message "users can not be used with state=query" in main() is still raised when state != "query" (same as before), which can be confusing for users; consider updating the message to reflect the actual condition under which it fires.

## Individual Comments

### Comment 1
<location path="plugins/modules/ipauser.py" line_range="887" />
<code_context>
                 invalid.extend(
                     ["principal", "manager", "certificate", "certmapdata"])

+        if state == "query":
+            module.fail_json(
+                msg="check_parameters can not be used with action query.")
         if state == "query":
             invalid.append("users")

</code_context>
<issue_to_address>
**issue (bug_risk):** The second `if state == "query"` branch is unreachable after `fail_json`, which suggests either redundant or misstructured logic.

Because `module.fail_json(...)` aborts execution, the second `if state == "query": invalid.append("users")` is now unreachable. Decide whether you want an immediate hard-fail for `check_parameters`+`query` (in which case the `invalid.append("users")` branch should be removed), or to keep the previous behavior of populating `invalid` and handling it later (in which case the first `if` likely needs to be an `elif` or merged into the branch that examines `invalid`).
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): The second if state == "query" branch is unreachable after fail_json, which suggests either redundant or misstructured logic.

Because module.fail_json(...) aborts execution, the second if state == "query": invalid.append("users") is now unreachable. Decide whether you want an immediate hard-fail for check_parameters+query (in which case the invalid.append("users") branch should be removed), or to keep the previous behavior of populating invalid and handling it later (in which case the first if likely needs to be an elif or merged into the branch that examines invalid).

[rjeffman]

LGTM.