Project

General

Profile

Actions

Defect #19363

closed

"User is invalid" error is returned when adding user to group twice

Added by Alex Last about 9 years ago. Updated about 9 years ago.

Status:
Closed
Priority:
Normal
Category:
REST API
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Invalid
Affected version:

Description

this operation used to be safe (idempotent) for Redmine 2.6.x.
now this fails for Redmine 3.0.0.

I am using this test:
https://github.com/taskadapter/redmine-java-api/blob/master/src/test/java/com/taskadapter/redmineapi/UserIntegrationTest.java

method addingUserToGroupTwiceDoesNotGiveErrors()

Actions #1

Updated by Jean-Philippe Lang about 9 years ago

  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Invalid

This behaviour (422 response) is present since 2.6.1, there's a test for this: source:/tags/2.6.1/test/integration/api_test/groups_test.rb#L192
Before that you'd get a 500 error (see #18665).

Actions #2

Updated by Alex Last about 9 years ago

I see I actually tested this with older version before - 2.6.0, not 2.6.2. So I missed that change in 2.6.1.
Maybe this REST operation should be idempotent ?

Actions #3

Updated by Jean-Philippe Lang about 9 years ago

Alex Last wrote:

I see I actually tested this with older version before - 2.6.0, not 2.6.2. So I missed that change in 2.6.1.

That was not really a change but a fix, this would trigger a 500 error before 2.6.1

Maybe this REST operation should be idempotent ?

POST requests are generally not. The few examples of REST API I was able to find respond with a 409 in this case, maybe we can change it to 409 instead of 422 for when the user already belongs to the group. Also, a 404 should be sent instead of a 422 for when the user does not exist. What do you think?

Actions #4

Updated by Alex Last about 9 years ago

I don't have a strong preference on 409 vs. 200 response code. but I don't see a harm in making this operation idempotent, so 200 would be more appropriate.
404 error code for non-existing user seems right as long as it is clear in the response what exactly is not found - user or group.

Actions

Also available in: Atom PDF