Browse Source

feat(ui): Remove issues from issue stream when action taken (#31701)

* feat(ui): Remove issues from issue stream when action taken

Remove issues from the issue stream when an action like Resolve, Ignore, or Delete is taken on the issue. This will remove the issue from the stream and bring in an issue from the next page (if applicable) and will not refresh the page.

FIXES WOR-1588

* refactor so resolved, ignored, and deleted issues are removed immediately

* refactor marked reviewed group id removal

* add acceptance tests
Kelly Carino 3 years ago
parent
commit
36196bd717

+ 43 - 22
static/app/views/issueList/overview.tsx

@@ -100,8 +100,8 @@ type Props = {
 } & RouteComponentProps<{searchId?: string}, {}>;
 
 type State = {
+  actionTaken: boolean;
   error: string | null;
-  forReview: boolean;
   groupIds: string[];
   isSidebarVisible: boolean;
   issuesLoading: boolean;
@@ -118,7 +118,6 @@ type State = {
   queryCounts: QueryCounts;
   queryMaxCount: number;
   realtimeActive: boolean;
-  reviewedIds: string[];
   selectAllActive: boolean;
   tagsLoading: boolean;
   // Will be set to true if there is valid session data from issue-stats api call
@@ -158,8 +157,7 @@ class IssueListOverview extends React.Component<Props, State> {
 
     return {
       groupIds: [],
-      reviewedIds: [],
-      forReview: false,
+      actionTaken: false,
       selectAllActive: false,
       realtimeActive,
       pageLinks: '',
@@ -212,10 +210,6 @@ class IssueListOverview extends React.Component<Props, State> {
       }
     }
 
-    if (prevState.forReview !== this.state.forReview) {
-      this.fetchData();
-    }
-
     // Wait for saved searches to load before we attempt to fetch stream data
     if (this.props.savedSearchLoading) {
       return;
@@ -518,9 +512,7 @@ class IssueListOverview extends React.Component<Props, State> {
   };
 
   fetchData = (fetchAllCounts = false) => {
-    const query = this.getQuery();
-
-    if (!this.state.reviewedIds.length || !isForReviewQuery(query)) {
+    if (!this.state.actionTaken) {
       GroupStore.loadInitialData([]);
       this._streamManager.reset();
 
@@ -528,7 +520,6 @@ class IssueListOverview extends React.Component<Props, State> {
         issuesLoading: true,
         queryCount: 0,
         itemsRemoved: 0,
-        reviewedIds: [],
         error: null,
       });
     }
@@ -599,10 +590,6 @@ class IssueListOverview extends React.Component<Props, State> {
 
         this._streamManager.push(data);
 
-        if (isForReviewQuery(query)) {
-          GroupStore.remove(this.state.reviewedIds);
-        }
-
         this.fetchStats(data.map((group: BaseGroup) => group.id));
 
         const hits = resp.getResponseHeader('X-Hits');
@@ -613,9 +600,7 @@ class IssueListOverview extends React.Component<Props, State> {
           typeof maxHits !== 'undefined' && maxHits ? parseInt(maxHits, 10) || 0 : 0;
         const pageLinks = resp.getResponseHeader('Link');
 
-        if (!this.state.forReview) {
-          this.fetchCounts(queryCount, fetchAllCounts);
-        }
+        this.fetchCounts(queryCount, fetchAllCounts);
 
         this.setState({
           error: null,
@@ -643,7 +628,7 @@ class IssueListOverview extends React.Component<Props, State> {
 
         this.resumePolling();
 
-        this.setState({forReview: false});
+        this.setState({actionTaken: false});
       },
     });
   };
@@ -706,6 +691,35 @@ class IssueListOverview extends React.Component<Props, State> {
   listener = GroupStore.listen(() => this.onGroupChange(), undefined);
 
   onGroupChange() {
+    const query = this.getQuery();
+
+    const resolvedIds = this._streamManager
+      .getAllItems()
+      .filter(id => id.status === 'resolved')
+      .map(item => item.id);
+    const ignoredIds = this._streamManager
+      .getAllItems()
+      .filter(id => id.status === 'ignored')
+      .map(item => item.id);
+    const reviewedIds = this._streamManager
+      .getAllItems()
+      .filter(id => !id.inbox)
+      .map(item => item.id);
+    // Remove Ignored and Resolved group ids from the issue stream, but if you have a query
+    // that includes these statuses or there's no query/you want to see ALL issues,
+    // don't trigger these group ids to be removed from the issue stream.
+    if (resolvedIds.length > 0 && !query.includes('is:resolved') && !!query) {
+      this.onIssueAction(resolvedIds);
+    }
+    if (ignoredIds.length > 0 && !query.includes('is:ignored') && !!query) {
+      this.onIssueAction(ignoredIds);
+    }
+    // Remove issues that are marked as Reviewed from the For Review tab, but still include the
+    // issues if not on the For Review tab, or no query for ALL issues.
+    if (reviewedIds.length > 0 && isForReviewQuery(query) && !!query) {
+      this.onIssueAction(reviewedIds);
+    }
+
     const groupIds = this._streamManager.getAllItems().map(item => item.id) ?? [];
     if (!isEqual(groupIds, this.state.groupIds)) {
       this.setState({groupIds});
@@ -948,6 +962,7 @@ class IssueListOverview extends React.Component<Props, State> {
   };
 
   onDelete = () => {
+    this.setState({actionTaken: true});
     this.fetchData(true);
   };
 
@@ -969,12 +984,18 @@ class IssueListOverview extends React.Component<Props, State> {
           [query as Query]: currentQueryCount,
         },
         itemsRemoved: itemsRemoved + inInboxCount,
-        reviewedIds: itemIds,
-        forReview: true,
       });
     }
   };
 
+  onIssueAction = (itemIds: string[]) => {
+    GroupStore.remove(itemIds);
+    this.setState({
+      actionTaken: true,
+    });
+    this.fetchData(true);
+  };
+
   tagValueLoader = (key: string, search: string) => {
     const {orgId} = this.props.params;
     const projectIds = this.getGlobalSearchProjectIds().map(id => id.toString());

+ 15 - 1
tests/acceptance/page_objects/issue_list.py

@@ -26,16 +26,30 @@ class IssueListPage(BasePage):
 
     def resolve_issues(self):
         self.browser.click('[aria-label="Resolve"]')
-        self.browser.click('[data-test-id="confirm-button"]')
 
     def wait_for_resolved_issue(self):
         self.browser.wait_until('[data-test-id="resolved-issue"]')
 
+    def wait_for_issue_removal(self):
+        self.browser.wait_until_not('[data-test-id="toast-loading"]')
+
     def wait_for_issue(self):
         self.browser.wait_until('[data-test-id="group"]')
 
     def find_resolved_issues(self):
         return self.browser.elements('[data-test-id="resolved-issue"]')
 
+    def ignore_issues(self):
+        self.browser.click('[aria-label="Ignore"]')
+
+    def delete_issues(self):
+        self.browser.click('[aria-label="Open more issue actions"]')
+        self.browser.click('[aria-label="Delete Issues"]')
+        self.browser.click('[data-test-id="confirm-button"]')
+
+    def merge_issues(self):
+        self.browser.click('[aria-label="Merge Selected Issues"]')
+        self.browser.click('[data-test-id="confirm-button"]')
+
     def mark_reviewed_issues(self):
         self.browser.click('[aria-label="Mark Reviewed"]')

+ 151 - 13
tests/acceptance/test_organization_group_index.py

@@ -4,7 +4,7 @@ from unittest.mock import patch
 import pytz
 from django.utils import timezone
 
-from sentry.models import AssistantActivity, GroupInboxReason
+from sentry.models import AssistantActivity, GroupInboxReason, GroupStatus
 from sentry.models.groupinbox import add_group_to_inbox
 from sentry.testutils import AcceptanceTestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
@@ -30,7 +30,7 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
         self.dismiss_assistant()
 
     def create_issues(self):
-        event_a = self.store_event(
+        self.event_a = self.store_event(
             data={
                 "event_id": "a" * 32,
                 "message": "oh no",
@@ -39,8 +39,8 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
             },
             project_id=self.project.id,
         )
-        add_group_to_inbox(event_a.group, GroupInboxReason.NEW)
-        event_b = self.store_event(
+        add_group_to_inbox(self.event_a.group, GroupInboxReason.NEW)
+        self.event_b = self.store_event(
             data={
                 "event_id": "b" * 32,
                 "message": "oh snap",
@@ -49,7 +49,7 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
             },
             project_id=self.project.id,
         )
-        add_group_to_inbox(event_b.group, GroupInboxReason.NEW)
+        add_group_to_inbox(self.event_b.group, GroupInboxReason.NEW)
 
     def test_with_onboarding(self):
         self.project.update(first_event=None)
@@ -80,33 +80,171 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
     def test_resolve_issues(self, mock_now):
         mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
         self.create_issues()
+
+        group1 = self.event_a.group
+
         self.page.visit_issue_list(self.org.slug)
         self.page.wait_for_stream()
 
         self.page.select_issue(1)
-        self.page.select_issue(2)
         self.page.resolve_issues()
-        self.page.wait_for_resolved_issue()
-        resolved_groups = self.page.find_resolved_issues()
 
-        assert len(resolved_groups) == 2
+        group1.update(status=GroupStatus.RESOLVED)
+
+        self.page.wait_for_issue_removal()
+        groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+        assert len(groups) == 1
 
     @patch("django.utils.timezone.now")
     def test_resolve_issues_multi_projects(self, mock_now):
         mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
         self.create_issues()
 
+        group1 = self.event_a.group
+
         with self.feature("organizations:global-views"):
             self.page.visit_issue_list(self.org.slug)
             self.page.wait_for_stream()
 
             self.page.select_issue(1)
-            self.page.select_issue(2)
             self.page.resolve_issues()
-            self.page.wait_for_resolved_issue()
-            resolved_groups = self.page.find_resolved_issues()
 
-            assert len(resolved_groups) == 2
+            group1.update(status=GroupStatus.RESOLVED)
+
+            self.page.wait_for_issue_removal()
+            groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+            assert len(groups) == 1
+
+    @patch("django.utils.timezone.now")
+    def test_ignore_issues(self, mock_now):
+        mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
+        self.create_issues()
+
+        group1 = self.event_a.group
+
+        self.page.visit_issue_list(self.org.slug)
+        self.page.wait_for_stream()
+
+        self.page.select_issue(1)
+        self.page.ignore_issues()
+
+        group1.update(status=GroupStatus.IGNORED)
+
+        self.page.wait_for_issue_removal()
+        groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+        assert len(groups) == 1
+
+    @patch("django.utils.timezone.now")
+    def test_ignore_issues_multi_projects(self, mock_now):
+        mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
+        self.create_issues()
+
+        group1 = self.event_a.group
+
+        with self.feature("organizations:global-views"):
+            self.page.visit_issue_list(self.org.slug)
+            self.page.wait_for_stream()
+
+            self.page.select_issue(1)
+            self.page.ignore_issues()
+
+            group1.update(status=GroupStatus.IGNORED)
+
+            self.page.wait_for_issue_removal()
+            groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+            assert len(groups) == 1
+
+    @patch("django.utils.timezone.now")
+    def test_delete_issues(self, mock_now):
+        mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
+        self.create_issues()
+
+        group1 = self.event_a.group
+
+        self.page.visit_issue_list(self.org.slug)
+        self.page.wait_for_stream()
+
+        self.page.select_issue(1)
+        self.page.delete_issues()
+
+        group1.update(status=GroupStatus.PENDING_DELETION)
+
+        self.page.wait_for_issue_removal()
+        groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+        assert len(groups) == 1
+
+    @patch("django.utils.timezone.now")
+    def test_delete_issues_multi_projects(self, mock_now):
+        mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
+        self.create_issues()
+
+        group1 = self.event_a.group
+
+        with self.feature("organizations:global-views"):
+            self.page.visit_issue_list(self.org.slug)
+            self.page.wait_for_stream()
+
+            self.page.select_issue(1)
+            self.page.delete_issues()
+
+            group1.update(status=GroupStatus.PENDING_DELETION)
+
+            self.page.wait_for_issue_removal()
+            groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+            assert len(groups) == 1
+
+    @patch("django.utils.timezone.now")
+    def test_merge_issues(self, mock_now):
+        mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
+        self.create_issues()
+
+        group1 = self.event_a.group
+        group2 = self.event_b.group
+
+        self.page.visit_issue_list(self.org.slug)
+        self.page.wait_for_stream()
+
+        self.page.select_issue(1)
+        self.page.select_issue(2)
+        self.page.merge_issues()
+
+        group1.update(status=GroupStatus.PENDING_MERGE)
+        group2.update(status=GroupStatus.PENDING_MERGE)
+
+        self.page.wait_for_issue_removal()
+        groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+        assert len(groups) == 1
+
+    @patch("django.utils.timezone.now")
+    def test_merge_issues_multi_projects(self, mock_now):
+        mock_now.return_value = datetime.utcnow().replace(tzinfo=pytz.utc)
+        self.create_issues()
+
+        group1 = self.event_a.group
+        group2 = self.event_b.group
+
+        with self.feature("organizations:global-views"):
+            self.page.visit_issue_list(self.org.slug)
+            self.page.wait_for_stream()
+
+            self.page.select_issue(1)
+            self.page.select_issue(2)
+            self.page.merge_issues()
+
+            group1.update(status=GroupStatus.PENDING_MERGE)
+            group2.update(status=GroupStatus.PENDING_MERGE)
+
+            self.page.wait_for_issue_removal()
+            groups = self.browser.elements('[data-test-id="event-issue-header"]')
+
+            assert len(groups) == 1
 
     @patch("django.utils.timezone.now")
     def test_inbox_results(self, mock_now):