SONAR-14214 - Unassign Security Hotspot

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/Assignee.tsx

@@ -18,7 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 import * as React from 'react';
-import { translateWithParameters } from 'sonar-ui-common/helpers/l10n';
+import { translate, translateWithParameters } from 'sonar-ui-common/helpers/l10n';
 import { assignSecurityHotspot } from '../../../../api/security-hotspots';
 import addGlobalSuccessMessage from '../../../../app/utils/addGlobalSuccessMessage';
 import { withCurrentUser } from '../../../../components/hoc/withCurrentUser';
@@ -61,25 +61,25 @@ export class Assignee extends React.PureComponent<Props, State> {
     this.setState({ editing: false });
   };
 
-  handleAssign = (newAssignee?: T.UserActive) => {
-    if (newAssignee && newAssignee.login) {
-      this.setState({ loading: true });
-      assignSecurityHotspot(this.props.hotspot.key, {
-        assignee: newAssignee.login
+  handleAssign = (newAssignee: T.UserActive) => {
+    this.setState({ loading: true });
+    assignSecurityHotspot(this.props.hotspot.key, {
+      assignee: newAssignee?.login
+    })
+      .then(() => {
+        if (this.mounted) {
+          this.setState({ editing: false, loading: false });
+          this.props.onAssigneeChange();
+        }
       })
-        .then(() => {
-          if (this.mounted) {
-            this.setState({ editing: false, loading: false });
-            this.props.onAssigneeChange();
-          }
-        })
-        .then(() =>
-          addGlobalSuccessMessage(
-            translateWithParameters('hotspots.assign.success', newAssignee.name)
-          )
+      .then(() =>
+        addGlobalSuccessMessage(
+          newAssignee.login
+            ? translateWithParameters('hotspots.assign.success', newAssignee.name)
+            : translate('hotspots.assign.unassign.success')
         )
-        .catch(() => this.setState({ loading: false }));
-    }
+      )
+      .catch(() => this.setState({ loading: false }));
   };
 
   render() {

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/AssigneeRenderer.tsx

@@ -33,7 +33,7 @@ export interface AssigneeRendererProps {
   assignee?: T.UserBase;
   loggedInUser?: T.LoggedInUser;
 
-  onAssign: (user?: T.UserActive) => void;
+  onAssign: (user: T.UserActive) => void;
   onEnterEditionMode: () => void;
   onExitEditionMode: () => void;
 }

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/AssigneeSelection.tsx

@@ -20,6 +20,7 @@
 import { debounce } from 'lodash';
 import * as React from 'react';
 import { KeyCodes } from 'sonar-ui-common/helpers/keycodes';
+import { translate } from 'sonar-ui-common/helpers/l10n';
 import { searchUsers } from '../../../../api/users';
 import { isUserActive } from '../../../../helpers/users';
 import AssigneeSelectionRenderer from './AssigneeSelectionRenderer';
@@ -27,17 +28,18 @@ import AssigneeSelectionRenderer from './AssigneeSelectionRenderer';
 interface Props {
   allowCurrentUserSelection: boolean;
   loggedInUser: T.LoggedInUser;
-  onSelect: (user?: T.UserActive) => void;
+  onSelect: (user: T.UserActive) => void;
 }
 
 interface State {
   highlighted?: T.UserActive;
   loading: boolean;
-  open: boolean;
   query?: string;
   suggestedUsers: T.UserActive[];
 }
 
+const UNASSIGNED: T.UserActive = { login: '', name: translate('unassigned') };
+
 export default class AssigneeSelection extends React.PureComponent<Props, State> {
   mounted = false;
 
@@ -46,8 +48,9 @@ export default class AssigneeSelection extends React.PureComponent<Props, State>
 
     this.state = {
       loading: false,
-      open: props.allowCurrentUserSelection,
-      suggestedUsers: props.allowCurrentUserSelection ? [props.loggedInUser] : []
+      suggestedUsers: props.allowCurrentUserSelection
+        ? [props.loggedInUser, UNASSIGNED]
+        : [UNASSIGNED]
     };
 
     this.handleSearch = debounce(this.handleSearch, 250);
@@ -76,9 +79,8 @@ export default class AssigneeSelection extends React.PureComponent<Props, State>
 
     this.setState({
       loading: false,
-      open: allowCurrentUserSelection,
       query,
-      suggestedUsers: allowCurrentUserSelection ? [loggedInUser] : []
+      suggestedUsers: allowCurrentUserSelection ? [loggedInUser, UNASSIGNED] : [UNASSIGNED]
     });
   };
 
@@ -90,8 +92,7 @@ export default class AssigneeSelection extends React.PureComponent<Props, State>
           this.setState({
             loading: false,
             query,
-            open: true,
-            suggestedUsers: result.users.filter(isUserActive) as T.UserActive[]
+            suggestedUsers: (result.users.filter(isUserActive) as T.UserActive[]).concat(UNASSIGNED)
           });
         }
       })
@@ -158,7 +159,7 @@ export default class AssigneeSelection extends React.PureComponent<Props, State>
   };
 
   render() {
-    const { highlighted, loading, open, query, suggestedUsers } = this.state;
+    const { highlighted, loading, query, suggestedUsers } = this.state;
 
     return (
       <AssigneeSelectionRenderer
@@ -167,7 +168,6 @@ export default class AssigneeSelection extends React.PureComponent<Props, State>
         onKeyDown={this.handleKeyDown}
         onSearch={this.handleSearch}
         onSelect={this.props.onSelect}
-        open={open}
         query={query}
         suggestedUsers={suggestedUsers}
       />

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/AssigneeSelectionRenderer.tsx

@@ -32,14 +32,14 @@ export interface HotspotAssigneeSelectRendererProps {
   loading: boolean;
   onKeyDown: (event: React.KeyboardEvent) => void;
   onSearch: (query: string) => void;
-  onSelect: (user: T.UserActive) => void;
-  open: boolean;
+  onSelect: (user?: T.UserActive) => void;
   query?: string;
   suggestedUsers?: T.UserActive[];
 }
 
 export default function AssigneeSelectionRenderer(props: HotspotAssigneeSelectRendererProps) {
-  const { highlighted, loading, open, query, suggestedUsers } = props;
+  const { highlighted, loading, query, suggestedUsers } = props;
+
   return (
     <>
       <div className="display-flex-center">
@@ -50,35 +50,33 @@ export default function AssigneeSelectionRenderer(props: HotspotAssigneeSelectRe
           placeholder={translate('hotspots.assignee.select_user')}
           value={query}
         />
-
         {loading && <DeferredSpinner className="spacer-left" />}
       </div>
 
-      {!loading && open && (
+      {!loading && (
         <div className="position-relative">
           <DropdownOverlay noPadding={true} placement={PopupPlacement.BottomLeft}>
-            {suggestedUsers && suggestedUsers.length > 0 ? (
-              <ul className="hotspot-assignee-search-results">
-                {suggestedUsers.map(suggestion => (
+            <ul className="hotspot-assignee-search-results">
+              {suggestedUsers &&
+                suggestedUsers.map(suggestion => (
                   <li
                     className={classNames('padded', {
                       active: highlighted && highlighted.login === suggestion.login
                     })}
                     key={suggestion.login}
                     onClick={() => props.onSelect(suggestion)}>
-                    <Avatar
-                      className="spacer-right"
-                      hash={suggestion.avatar}
-                      name={suggestion.name}
-                      size={16}
-                    />
+                    {suggestion.login && (
+                      <Avatar
+                        className="spacer-right"
+                        hash={suggestion.avatar}
+                        name={suggestion.name}
+                        size={16}
+                      />
+                    )}
                     {suggestion.name}
                   </li>
                 ))}
-              </ul>
-            ) : (
-              <div className="padded">{translate('no_results')}</div>
-            )}
+            </ul>
           </DropdownOverlay>
         </div>
       )}

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/__tests__/Assignee-test.tsx

@@ -59,10 +59,12 @@ it('should handle edition event correctly', () => {
   expect(wrapper.state().editing).toBe(false);
 });
 
-it('should handle assign event correctly', async () => {
+it.each([
+  ['assign to user', mockUser() as T.UserActive],
+  ['unassign', { login: '', name: 'unassigned' } as T.UserActive]
+])('should handle %s event', async (_, user: T.UserActive) => {
   const hotspot = mockHotspot();
   const onAssigneeChange = jest.fn();
-  const user = mockUser() as T.UserActive;
 
   const wrapper = shallowRender({ hotspot, onAssigneeChange });
 
@@ -73,7 +75,7 @@ it('should handle assign event correctly', async () => {
     .onAssign(user);
 
   expect(wrapper.state().loading).toBe(true);
-  expect(assignSecurityHotspot).toHaveBeenCalledWith(hotspot.key, { assignee: user.login });
+  expect(assignSecurityHotspot).toHaveBeenCalledWith(hotspot.key, { assignee: user?.login });
 
   await waitAndUpdate(wrapper);
 

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/__tests__/AssigneeSelection-test.tsx

@@ -45,7 +45,7 @@ it('should handle keydown', () => {
   const wrapper = shallowRender({ onSelect });
 
   wrapper.instance().handleKeyDown(mockEvent(KeyCodes.UpArrow) as any);
-  expect(wrapper.state().highlighted).toBeUndefined();
+  expect(wrapper.state().highlighted).toEqual({ login: '', name: 'unassigned' });
 
   wrapper.setState({ suggestedUsers });
 
@@ -77,11 +77,10 @@ it('should handle search', async () => {
   const onSelect = jest.fn();
 
   const wrapper = shallowRender({ onSelect });
-  expect(wrapper.state().suggestedUsers.length).toBe(0);
+  expect(wrapper.state().suggestedUsers.length).toBe(1);
   wrapper.instance().handleSearch('j');
 
   expect(searchUsers).not.toBeCalled();
-  expect(wrapper.state().open).toBe(false);
 
   wrapper.instance().handleSearch('jo');
   expect(wrapper.state().loading).toBe(true);
@@ -90,14 +89,13 @@ it('should handle search', async () => {
   await waitAndUpdate(wrapper);
 
   expect(wrapper.state().loading).toBe(false);
-  expect(wrapper.state().open).toBe(true);
-  expect(wrapper.state().suggestedUsers).toHaveLength(3);
+  expect(wrapper.state().suggestedUsers).toHaveLength(4);
 
   jest.clearAllMocks();
 
   wrapper.instance().handleSearch('');
   expect(searchUsers).not.toBeCalled();
-  expect(wrapper.state().suggestedUsers.length).toBe(0);
+  expect(wrapper.state().suggestedUsers.length).toBe(1);
 });
 
 it('should allow current user selection', async () => {
@@ -110,7 +108,7 @@ it('should allow current user selection', async () => {
 
   wrapper.instance().handleSearch('jo');
   await waitAndUpdate(wrapper);
-  expect(wrapper.state().suggestedUsers).toHaveLength(3);
+  expect(wrapper.state().suggestedUsers).toHaveLength(4);
 
   wrapper.instance().handleSearch('');
   expect(wrapper.state().suggestedUsers[0]).toBe(loggedInUser);

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/__tests__/AssigneeSelectionRenderer-test.tsx

@@ -27,13 +27,11 @@ import AssigneeSelectionRenderer, {
 it('should render correctly', () => {
   expect(shallowRender()).toMatchSnapshot();
   expect(shallowRender({ loading: true })).toMatchSnapshot('loading');
-  expect(shallowRender({ open: true })).toMatchSnapshot('open');
 
   const highlightedUser = mockUser({ login: 'highlighted' }) as T.UserActive;
   expect(
     shallowRender({
       highlighted: highlightedUser,
-      open: true,
       suggestedUsers: [mockUser() as T.UserActive, highlightedUser]
     })
   ).toMatchSnapshot('open with results');
@@ -43,7 +41,6 @@ it('should call onSelect when clicked', () => {
   const user = mockUser() as T.UserActive;
   const onSelect = jest.fn();
   const wrapper = shallowRender({
-    open: true,
     onSelect,
     suggestedUsers: [user]
   });
@@ -63,7 +60,6 @@ function shallowRender(props?: Partial<HotspotAssigneeSelectRendererProps>) {
       onKeyDown={jest.fn()}
       onSearch={jest.fn()}
       onSelect={jest.fn()}
-      open={false}
       {...props}
     />
   );

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/__tests__/__snapshots__/AssigneeSelection-test.tsx.snap

@@ -6,7 +6,13 @@ exports[`should render correctly 1`] = `
   onKeyDown={[Function]}
   onSearch={[Function]}
   onSelect={[MockFunction]}
-  open={false}
-  suggestedUsers={Array []}
+  suggestedUsers={
+    Array [
+      Object {
+        "login": "",
+        "name": "unassigned",
+      },
+    ]
+  }
 />
 `;

server/sonar-web/src/main/js/apps/security-hotspots/components/assignee/__tests__/__snapshots__/AssigneeSelectionRenderer-test.tsx.snap

@@ -12,6 +12,18 @@ exports[`should render correctly 1`] = `
       placeholder="hotspots.assignee.select_user"
     />
   </div>
+  <div
+    className="position-relative"
+  >
+    <DropdownOverlay
+      noPadding={true}
+      placement="bottom-left"
+    >
+      <ul
+        className="hotspot-assignee-search-results"
+      />
+    </DropdownOverlay>
+  </div>
 </Fragment>
 `;
 
@@ -33,35 +45,6 @@ exports[`should render correctly: loading 1`] = `
 </Fragment>
 `;
 
-exports[`should render correctly: open 1`] = `
-<Fragment>
-  <div
-    className="display-flex-center"
-  >
-    <SearchBox
-      autoFocus={true}
-      onChange={[MockFunction]}
-      onKeyDown={[MockFunction]}
-      placeholder="hotspots.assignee.select_user"
-    />
-  </div>
-  <div
-    className="position-relative"
-  >
-    <DropdownOverlay
-      noPadding={true}
-      placement="bottom-left"
-    >
-      <div
-        className="padded"
-      >
-        no_results
-      </div>
-    </DropdownOverlay>
-  </div>
-</Fragment>
-`;
-
 exports[`should render correctly: open with results 1`] = `
 <Fragment>
   <div

server/sonar-web/src/main/js/types/security-hotspots.ts

@@ -150,6 +150,6 @@ export interface HotspotSetStatusRequest {
 }
 
 export interface HotspotAssignRequest {
-  assignee: string;
+  assignee?: string;
   comment?: string;
 }

server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/AssignAction.java

@@ -20,6 +20,7 @@
 package org.sonar.server.hotspot.ws;
 
 import javax.annotation.Nullable;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -35,6 +36,7 @@ import org.sonar.db.user.UserDto;
 import org.sonar.server.issue.IssueFieldsSetter;
 import org.sonar.server.issue.ws.IssueUpdater;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
 import static org.sonar.server.exceptions.NotFoundException.checkFound;
 import static org.sonar.server.exceptions.NotFoundException.checkFoundWithOptional;
@@ -66,7 +68,9 @@ public class AssignAction implements HotspotsWsAction {
       .setSince("8.2")
       .setHandler(this)
       .setInternal(true)
-      .setPost(true);
+      .setPost(true)
+      .setChangelog(
+        new Change("8.9", "Parameter 'assignee' is no longer mandatory"));
 
     action.createParam(PARAM_HOTSPOT_KEY)
       .setDescription("Hotspot key")
@@ -75,7 +79,6 @@ public class AssignAction implements HotspotsWsAction {
 
     action.createParam(PARAM_ASSIGNEE)
       .setDescription("Login of the assignee with 'Browse' project permission")
-      .setRequired(true)
       .setExampleValue("admin");
 
     action.createParam(PARAM_COMMENT)
@@ -85,7 +88,7 @@ public class AssignAction implements HotspotsWsAction {
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    String assignee = request.mandatoryParam(PARAM_ASSIGNEE);
+    String assignee = request.param(PARAM_ASSIGNEE);
     String key = request.mandatoryParam(PARAM_HOTSPOT_KEY);
     String comment = request.param(PARAM_COMMENT);
 
@@ -101,7 +104,7 @@ public class AssignAction implements HotspotsWsAction {
 
       checkIfHotspotToReview(hotspotDto);
       hotspotWsSupport.loadAndCheckProject(dbSession, hotspotDto, UserRole.USER);
-      UserDto assignee = getAssignee(dbSession, login);
+      UserDto assignee = isNullOrEmpty(login) ? null :getAssignee(dbSession, login);
 
       IssueChangeContext context = hotspotWsSupport.newIssueChangeContext();
 
@@ -111,7 +114,9 @@ public class AssignAction implements HotspotsWsAction {
         issueFieldsSetter.addComment(defaultIssue, comment, context);
       }
 
-      checkAssigneeProjectPermission(dbSession, assignee, hotspotDto.getProjectUuid());
+      if (assignee != null) {
+        checkAssigneeProjectPermission(dbSession, assignee, hotspotDto.getProjectUuid());
+      }
 
       if (issueFieldsSetter.assign(defaultIssue, assignee, context)) {
         issueUpdater.saveIssueAndPreloadSearchResponseData(dbSession, defaultIssue, context, false);

server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AssignActionTest.java

@@ -34,6 +34,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.sonar.api.rules.RuleType;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
 import org.sonar.api.web.UserRole;
@@ -58,9 +59,11 @@ import org.sonar.server.ws.WsActionTester;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.tuple;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -100,11 +103,14 @@ public class AssignActionTest {
     assertThat(hotspotParam.isRequired()).isTrue();
     WebService.Param assigneeParam = wsDefinition.param("assignee");
     assertThat(assigneeParam).isNotNull();
-    assertThat(assigneeParam.isRequired()).isTrue();
+    assertThat(assigneeParam.isRequired()).isFalse();
     WebService.Param commentParam = wsDefinition.param("comment");
     assertThat(commentParam).isNotNull();
     assertThat(commentParam.isRequired()).isFalse();
     assertThat(wsDefinition.since()).isEqualTo("8.2");
+    assertThat(wsDefinition.changelog())
+      .extracting(Change::getVersion, Change::getDescription)
+      .contains(tuple("8.9", "Parameter 'assignee' is no longer mandatory"));
   }
 
   @Test
@@ -124,6 +130,23 @@ public class AssignActionTest {
     verifyFieldSetters(assignee, null);
   }
 
+  @Test
+  public void unassign_hotspot_for_public_project() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    UserDto assignee = insertUser(randomAlphanumeric(15));
+
+    IssueDto hotspot = dbTester.issues().insertHotspot(project, file, h -> h.setAssigneeUuid(assignee.getUuid()));
+
+    UserDto userDto = insertUser(randomAlphanumeric(10));
+    userSessionRule.logIn(userDto).registerComponents(project);
+    when(issueFieldsSetter.assign(eq(hotspot.toDefaultIssue()), isNull(), any(IssueChangeContext.class))).thenReturn(true);
+
+    executeRequest(hotspot, null, null);
+
+    verifyFieldSetters(null, null);
+  }
+
   @Test
   public void assign_hotspot_to_me_for_public_project() {
     ComponentDto project = dbTester.components().insertPublicProject();
@@ -140,6 +163,21 @@ public class AssignActionTest {
     verifyFieldSetters(me, null);
   }
 
+  @Test
+  public void unassign_hotspot_myself_for_public_project() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    UserDto me = insertUser(randomAlphanumeric(10));
+    userSessionRule.logIn(me).registerComponents(project);
+    IssueDto hotspot = dbTester.issues().insertHotspot(project, file, h -> h.setAssigneeUuid(me.getUuid()));
+
+    when(issueFieldsSetter.assign(eq(hotspot.toDefaultIssue()), isNull(), any(IssueChangeContext.class))).thenReturn(true);
+
+    executeRequest(hotspot, null, null);
+
+    verifyFieldSetters(null, null);
+  }
+
   @Test
   public void assign_hotspot_to_someone_for_private_project() {
     ComponentDto project = dbTester.components().insertPrivateProject();
@@ -156,6 +194,22 @@ public class AssignActionTest {
     verifyFieldSetters(assignee, null);
   }
 
+  @Test
+  public void unassign_hotspot_for_private_project() {
+    ComponentDto project = dbTester.components().insertPrivateProject();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    UserDto assignee = insertUser(randomAlphanumeric(15));
+    IssueDto hotspot = dbTester.issues().insertHotspot(project, file, h -> h.setAssigneeUuid(assignee.getUuid()));
+
+    insertAndLoginAsUserWithProjectUserPermission(randomAlphanumeric(10), project, UserRole.USER);
+
+    when(issueFieldsSetter.assign(eq(hotspot.toDefaultIssue()), isNull(), any(IssueChangeContext.class))).thenReturn(true);
+
+    executeRequest(hotspot, null, null);
+
+    verifyFieldSetters(null, null);
+  }
+
   @Test
   public void assign_hotspot_to_someone_for_private_project_branch() {
     ComponentDto project = dbTester.components().insertPrivateProject();
@@ -494,8 +548,12 @@ public class AssignActionTest {
   }
 
   private static UserDto userMatcher(UserDto user) {
-    return argThat(argument -> argument.getLogin().equals(user.getLogin()) &&
-      argument.getUuid().equals(user.getUuid()));
+    if (user == null) {
+      return isNull();
+    } else {
+      return argThat(argument -> argument.getLogin().equals(user.getLogin()) &&
+        argument.getUuid().equals(user.getUuid()));
+    }
   }
 
 }

sonar-core/src/main/resources/org/sonar/l10n/core.properties

@@ -754,6 +754,7 @@ hotspots.reviewed.tooltip=Percentage of Security Hotspots reviewed (fixed or saf
 hotspots.review_hotspot=Review Hotspot
 
 hotspots.assign.success=Security Hotspot was successfully assigned to {0}
+hotspots.assign.unassign.success=Security Hotspot was successfully unassigned
 hotspots.update.success=Security Hotspot status was successfully changed to {0}
 
 #------------------------------------------------------------------------------