Skip to content
Snippets Groups Projects
New issue look: Off

Can't assign previously assigned MR

    • Closed Issue created by username-removed-408881 @asumaran

    Reproduced locally on master as well.

    A developer (without permission to merge) can't assign an existing MR by himself.

    /cc @jschatz1 @JobV @DouweM @rspeicher

    Screencap

    Designs

      Child items ...

      2. Activity

      • Jacob Schatz
        Jacob Schatz @jschatz1 ·
        Maintainer

        @alfredo1 It would be good if

        1. You can reproduce this locally.
        2. You can provide some data from the logs.
      • username-removed-408881
        username-removed-408881 @asumaran ·
        Author Contributor

        @jschatz1 Yes, I can reproduce this issue locally.

        Here's the development.log when I chose my username on the dropdown.

        Started PATCH "/root/anotherproject/merge_requests/1" for 127.0.0.1 at 2016-02-26 16:16:45 -0500
Processing by Projects::MergeRequestsController#update as JS
  Parameters: {"utf8"=>"✓", "merge_request"=>{"assignee_id"=>"31", "milestone_id"=>""}, "issuable_context"=>"", "namespace_id"=>"root", "project_id"=>"anotherproject", "id"=>"1"}
  User Load (0.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1  ORDER BY "users"."id" DESC LIMIT 1  [["id", 31]]
  ActiveRecord::SchemaMigration Load (0.2ms)  SELECT "schema_migrations".* FROM "schema_migrations"
   (0.1ms)  BEGIN
   (0.1ms)  ROLLBACK
  Project Load (0.6ms)  SELECT  "projects".* FROM "projects" INNER JOIN "namespaces" ON "namespaces"."id" = "projects"."namespace_id" WHERE (LOWER("namespaces"."path") = LOWER('root')) AND "projects"."path" = 'anotherproject' LIMIT 1
  ProjectMember Load (0.2ms)  SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3  ORDER BY "members"."id" DESC  [["source_type", "Project"], ["source_id", 20], ["source_type", "Project"]]
  Group Load (0.1ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."type" IN ('Group') AND "namespaces"."id" = $1 AND "namespaces"."type" IN ('Group') AND "namespaces"."type" = 'Group'  ORDER BY "namespaces"."id" DESC LIMIT 1  [["id", 1]]
  Namespace Load (0.2ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1  ORDER BY "namespaces"."id" DESC LIMIT 1  [["id", 1]]
  User Load (0.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1  ORDER BY "users"."id" DESC LIMIT 1  [["id", 1]]
  MergeRequest Load (0.2ms)  SELECT  "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = $1 AND "merge_requests"."iid" = $2  ORDER BY "merge_requests"."id" DESC LIMIT 1  [["target_project_id", 20], ["iid", 1]]
  CACHE (0.0ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1  ORDER BY "users"."id" DESC LIMIT 1  [["id", 1]]
  Project Load (0.2ms)  SELECT  "projects".* FROM "projects" WHERE "projects"."id" = $1  ORDER BY "projects"."id" DESC LIMIT 1  [["id", 20]]
  CACHE (0.0ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1  ORDER BY "namespaces"."id" DESC LIMIT 1  [["id", 1]]
  MergeRequestDiff Load (0.2ms)  SELECT  "merge_request_diffs".* FROM "merge_request_diffs" WHERE "merge_request_diffs"."merge_request_id" = $1  ORDER BY "merge_request_diffs"."id" DESC LIMIT 1  [["merge_request_id", 84]]
  CACHE (0.0ms)  SELECT  "projects".* FROM "projects" WHERE "projects"."id" = $1  ORDER BY "projects"."id" DESC LIMIT 1  [["id", 20]]
  Service Load (0.2ms)  SELECT  "services".* FROM "services" WHERE "services"."project_id" = $1 AND "services"."category" = $2 AND "services"."active" = $3 AND "services"."default" = $4  ORDER BY "services"."id" DESC LIMIT 1  [["project_id", 20], ["category", "issue_tracker"], ["active", "t"], ["default", "f"]]
  CACHE (0.0ms)  SELECT  "services".* FROM "services" WHERE "services"."project_id" = $1 AND "services"."category" = $2 AND "services"."active" = $3 AND "services"."default" = $4  ORDER BY "services"."id" DESC LIMIT 1  [["project_id", 20], ["category", "issue_tracker"], ["active", true], ["default", false]]
  GitlabIssueTrackerService Load (0.2ms)  SELECT  "services".* FROM "services" WHERE "services"."type" IN ('GitlabIssueTrackerService') AND "services"."project_id" = $1  ORDER BY "services"."id" DESC LIMIT 1  [["project_id", 20]]
  CACHE (0.0ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1  ORDER BY "users"."id" DESC LIMIT 1  [["id", 1]]
  Rendered public/404.html (0.1ms)
Filter chain halted as :authorize_update_merge_request! rendered or redirected
Completed 404 Not Found in 90ms (Views: 24.3ms | ActiveRecord: 3.4ms)


Started GET "/root/anotherproject/notes?target_id=84&target_type=merge_request&last_fetched_at=1456521402" for 127.0.0.1 at 2016-02-26 16:16:57 -0500
Processing by Projects::NotesController#index as JSON
  Parameters: {"target_id"=>"84", "target_type"=>"merge_request", "last_fetched_at"=>"1456521402", "namespace_id"=>"root", "project_id"=>"anotherproject"}
  User Load (0.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1  ORDER BY "users"."id" DESC LIMIT 1  [["id", 31]]
  ActiveRecord::SchemaMigration Load (0.2ms)  SELECT "schema_migrations".* FROM "schema_migrations"
   (0.1ms)  BEGIN
   (0.1ms)  ROLLBACK
  Project Load (0.5ms)  SELECT  "projects".* FROM "projects" INNER JOIN "namespaces" ON "namespaces"."id" = "projects"."namespace_id" WHERE (LOWER("namespaces"."path") = LOWER('root')) AND "projects"."path" = 'anotherproject' LIMIT 1
  ProjectMember Load (0.3ms)  SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3  ORDER BY "members"."id" DESC  [["source_type", "Project"], ["source_id", 20], ["source_type", "Project"]]
  Group Load (0.2ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."type" IN ('Group') AND "namespaces"."id" = $1 AND "namespaces"."type" IN ('Group') AND "namespaces"."type" = 'Group'  ORDER BY "namespaces"."id" DESC LIMIT 1  [["id", 1]]
  Namespace Load (0.2ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1  ORDER BY "namespaces"."id" DESC LIMIT 1  [["id", 1]]
  User Load (0.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1  ORDER BY "users"."id" DESC LIMIT 1  [["id", 1]]
  MergeRequest Load (0.2ms)  SELECT  "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = $1 AND "merge_requests"."id" = $2  ORDER BY "merge_requests"."id" DESC LIMIT 1  [["target_project_id", 20], ["id", 84]]
      • Douwe Maan Milestone changed to 8.6

        Milestone changed to 8.6

      • Douwe Maan Added ~12980 label

        Added ~12980 label

      • Douwe Maan Added ~14281 label

        Added ~14281 label

      • Rubén Dávila
        Rubén Dávila @rdavila ·
        Maintainer

        @DouweM @rspeicher I'm not sure if this can be considered a bug, what happened here is that the user was trying to change the assignee of a MR that was not owned by him neither was assigned to him.

      • Rubén Dávila Reassigned to @rdavila

        Reassigned to @rdavila

      • username-removed-408881
        username-removed-408881 @asumaran ·
        Author Contributor

        @rdavila If this is by design. We might disable the option and/or show a message explaining this. Because It's confusing if no feedback is provided for a user when the assignment fails or it's not allowed.

        /cc @jschatz1

        Edited by username-removed-408881
      • Jacob Schatz
        Jacob Schatz @jschatz1 ·
        Maintainer

        @rdavila It is certainly a bug in UX/UI design. The user shouldn't have been allowed to choose an assignee from the list if wasn't allowed to assign an assignee in the first place. That is very confusing.

        I say we remove the edit button if the user doesn't have the rights to edit the assignee. @creamzy @skyruler @JobV.

        I say we remove the Edit button unless anyone else says otherwise.

      • Rubén Dávila
        Rubén Dávila @rdavila ·
        Maintainer

        @alfredo1 @jschatz1 makes sense to me the removal of the Edit button if the user is not allowed to change the assignee, I'll work on that change at least someone has a better solution for this.

      • username-removed-408881
        username-removed-408881 @asumaran ·
        Author Contributor

        @jschatz1 @rdavila I agree. We should hide the Edit button for this case.

      • Rubén Dávila mentioned in merge request !3100 (closed)

        mentioned in merge request !3100 (closed)

      • Douwe Maan
        Douwe Maan @DouweM ·
        Maintainer

        The issue is actually the other way around, the 'Edit' button should be there, but the backend is currently failing while it shouldn't. Commented on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3100.

      • Rubén Dávila
        Rubén Dávila @rdavila ·
        Maintainer

        @DouweM I've commented too, right now the developer can update the Issue/MR as long as he is who created it or he is current assignee. Do you think the developer should always have the update_merge_request and update_issue permissions?

      • Douwe Maan
        Douwe Maan @DouweM ·
        Maintainer

        This is, confusingly, a duplicate of https://gitlab.com/gitlab-org/gitlab-ce/issues/13765 for which https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3099 is a fix. I'll close this :)

      • Douwe Maan Status changed to closed

        Status changed to closed

      • username-removed-127938
        username-removed-127938 @skyruler ·
        Contributor

        @jschatz1 yeah I completely agree about removing the button if user have no rights, but I mean it depends on a rights not sure if it's UI/UX though

      • Douwe Maan
        Douwe Maan @DouweM ·
        Maintainer

        @skyruler It's fixed already!

      • username-removed-127938
        username-removed-127938 @skyruler ·
        Contributor

        @DouweM good

      Please register or sign in to reply