Skip to content

[Fixes #14473] Fix payload handling in the users transfer_resources endpoint - #14474

Open
christianbraun wants to merge 1 commit into
GeoNode:masterfrom
christianbraun:fix/transfer-resources-payload
Open

[Fixes #14473] Fix payload handling in the users transfer_resources endpoint#14474
christianbraun wants to merge 1 commit into
GeoNode:masterfrom
christianbraun:fix/transfer-resources-payload

Conversation

@christianbraun

Copy link
Copy Markdown

Fixes #14473.

POST /api/v2/users/{pk}/transfer_resources could not be called with the minimal payload, and one of its guards was unreachable.

  • if not target_user and previous_owner only rejected a missing newOwner when currentOwner was present. Without either, the request fell through to get_object_or_404(User, id=None) and answered 404 instead of 400.
  • currentOwner is optional as far as the filter is concerned (filter_payload = dict(owner=previous_owner or user)), but the loop resolved it unconditionally with get_object_or_404, so {"newOwner": <id>} on its own always returned 404. It is now resolved once, up front, defaulting to the user the resources are read from - which also takes the query out of the loop.
  • With a JSON body and no resources key the subset is None, so len(transfer_resource_subset) raised TypeError whenever the source user is not a superuser. Skipping the subset check when there is no subset keeps the whole-account transfer working.

Everything that already worked keeps its status code. The one behaviour change is the empty-body case, which now answers the 400 the guard was clearly written for: test_transfer_resources_nopayload asserted the 404 from the first bug and now asserts 400.

test_transfer_resources_without_current_owner is new and covers the other two - it sends a JSON body, so resources is genuinely absent rather than the empty list a form-encoded QueryDict.getlist returns. That difference is why the existing tests pass today.

I have not been able to run the full Django suite locally; black and flake8 are clean on both files.

`POST /api/v2/users/{pk}/transfer_resources` could not be called with the
minimal documented payload, and one of its guards was unreachable:

- `if not target_user and previous_owner` only rejected a missing `newOwner`
  when `currentOwner` was present. Without either, the request fell through to
  `get_object_or_404(User, id=None)` and answered 404 instead of 400.
- `currentOwner` is optional (`filter_payload = dict(owner=previous_owner or
  user)`), but the loop resolved it unconditionally with `get_object_or_404`,
  so `{"newOwner": <id>}` on its own always returned 404. It is now resolved
  once, up front, defaulting to the user the resources are read from.
- With a JSON body and no `resources` key the subset is `None`, so
  `len(transfer_resource_subset)` raised TypeError when the source user is not
  a superuser. Skipping the subset check when there is no subset keeps the
  whole-account transfer working.

`test_transfer_resources_nopayload` asserted the 404 from the first bug and now
asserts 400. Added `test_transfer_resources_without_current_owner`, which sends
a JSON body and covers the other two.
@cla-bot

cla-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @christianbraun on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

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.

transfer_resources rejects the minimal payload and raises TypeError on a JSON body without resources

3 participants