Skip to content

refactor(provisioning_api): consolidate user-edit field logic#61065

Open
pringelmann wants to merge 1 commit into
masterfrom
refactor/40903/edit-user-controller-consolidation
Open

refactor(provisioning_api): consolidate user-edit field logic#61065
pringelmann wants to merge 1 commit into
masterfrom
refactor/40903/edit-user-controller-consolidation

Conversation

@pringelmann

@pringelmann pringelmann commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Stage 1 of consolidating the two user-update endpoints:

  • editUser (single-field PUT)
  • editUserMultiField (multi-field PATCH)

These duplicated the per-field validate/apply rules. This extracts them into
private helpers and routes both endpoints through them, so each rule lives in
one place.

Mostly a refactor, but unifying the two endpoints' rules changes behavior in
three places. All intended:

  • PATCH now gates the primary email on canEditProperty('email'), same as PUT
    already did. A self user whose email is locked (allow_user_to_change_email=false,
    or an LDAP-mapped account) can no longer change it via PATCH.
  • PUT now applies the force_language policy in the language branch, so a
    subadmin editing another user is blocked when force_language is set
    (previously it only checked the language exists).
  • Email validation goes through IEmailValidator, so the accepted/rejected set
    shifts vs filter_var (e.g. user@localhost is accepted on non-strict instances).

Request/response shapes and the OCS error codes / HTTP statuses are otherwise
unchanged. Existing tests still pass (a few updated slightly); new unit tests cover each helper plus the three
changes above.

Stage 2 will expand PATCH to full field parity and make editUser a thin wrapper
over it. Left out here so this stays scoped, since rerouting the long-standing
editUser path is a bigger behavioral change that warrants its own review imo.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Screenshots before/after for front-end changes
  • Documentation (manuals or wiki) has been updated or is not required
  • Backports requested where applicable (ex: critical bugfixes)
  • Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
  • Milestone added for target branch/version (ex: 32.x for stable32)

AI

  • The content of this PR was partly generated using AI (mainly testing and reviewing)

@pringelmann pringelmann self-assigned this Jun 8, 2026
@susnux susnux added this to the Nextcloud 35 milestone Jun 9, 2026
@pringelmann pringelmann force-pushed the refactor/40903/edit-user-controller-consolidation branch 2 times, most recently from 8b98773 to 8d5bbab Compare June 22, 2026 13:45
@pringelmann pringelmann added feature: users and groups technical debt 🧱 🤔🚀 ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jun 22, 2026
@pringelmann pringelmann marked this pull request as ready for review June 22, 2026 13:51
@pringelmann pringelmann requested a review from a team as a code owner June 22, 2026 13:51
@pringelmann pringelmann requested review from Altahrim, artonge, leftybournes, provokateurin and salmart-dev and removed request for a team June 22, 2026 13:51
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
@pringelmann pringelmann force-pushed the refactor/40903/edit-user-controller-consolidation branch from 8d5bbab to d55e147 Compare June 22, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: users and groups ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants