Refactoring report filter UI #1401

csordasmarton · 2018-02-26T12:44:08Z

Closes #1347

Create separate files for each base filters.
Create new filter interface.
Extend API with tag filter.
Remove CountFilter type. The client should set the correct filter set instead of checking the filter type.
Do not show default values in URL query parameter for is-unique and difftype.
Notify other filters on filter change when users changed the filter and the tooltip is closed.
Get items from the server only if some filter is changed or if the filter uses server side search.

bruntib · 2018-02-28T09:09:07Z

api/v6/report_server.thrift

+ * Between the list elements there is "OR" relation.
+ * If exactMatch field is True it will use exact match for run names.
+ */
+struct TagFilter {


Is it worth being a struct instead of a typedef? If it will have more members later, then it's ok.

Later this way we can add an option easily to filter run history data by author or date. I have also renamed it to RunHistoryFilter because it is much more expressive.

bruntib · 2018-02-28T09:20:08Z

libcodechecker/server/api/report_server.py

@@ -110,7 +100,7 @@ def conv(text):
    return text.replace('*', '%')


-def process_report_filter_v2(session, report_filter, count_filter=None):
+def process_report_filter(session, report_filter):
    """
    Process the new report filter.
    If the count_filter parameter is set it will ignore that field type of


Update the comment as well: there is no count_filter now.

bruntib · 2018-02-28T10:17:52Z

www/scripts/codecheckerviewer/ListOfBugs.js

-      });
+     this._runHistoryTopic = topic.subscribe('subtab/runHistory', function () {
+       that.selectChild(that._runHistory);
+     });


Indentation

bruntib · 2018-02-28T10:18:18Z

www/scripts/codecheckerviewer/ListOfBugs.js


      this._bugOverview.addChild(this._bugFilterView);

      this._bugOverview.addChild(this._grid);
      this.addChild(this._bugOverview);

-      this.addChild(this._runHistory);
+     this.addChild(this._runHistory);


Indentation

bruntib · 2018-02-28T10:28:38Z

www/scripts/codecheckerviewer/ListOfBugs.js

-      if (!this.initalized) {
-        this.initalized = true;
+      // If the filter is not initalized before hen we should initalize it
+      // first by using an empty filter set.


Maybe this comment should be checked. I don't understand what's happening here.

bruntib · 2018-02-28T10:33:03Z

www/scripts/codecheckerviewer/hashHelper.js

+    * This function pushes the currently stored state to the browser history.
+    * During the update process, the "hashSetProgress" variable of the
+    * urlHandler object is set to true. This signs that the url update in the
+   * browser was done by urlHandler, not by the "browser back" or "browser


Indentation

whisperity · 2018-03-05T13:09:50Z

I'm not entirely sure what this patch is supposed to do. I understand and like splitting up the filters into separate files.

However, I don't see this closing #1347. If I click a particular checker name in the statistics view, the following happens:

I clicked deadcode.DeadStores, but no checker name filters are selected. There is no error output in the browser console. The URL generated by clicking on a checker name is:

http://localhost:8001/Default/#tab=allReports&review-status=unreviewed&review-status=confirmed&review-status=false_positive&review-status=intentional&detection-status=new&detection-status=resolved&detection-status=unresolved&detection-status=reopened&checker-name=deadcode.DeadStores

There is deadcode.DeadStores in the URL but it doesn't seem to work. If I reload the page, the filter is applied to the result set but in the left-hand filter view under Checker name it still says No filter..

csordasmarton · 2018-03-05T13:32:46Z

@whisperity I can not reproduce your problem. For me it works fine.
I can not even see that your Checker name filter is not set properly on the left hand side filter because it is on the bottom of the left hand filter which is not seen on your screenshot.

whisperity · 2018-03-05T13:42:02Z

Yeah, small resolution when not on external screen. 🙁
Uh... okay. Restarting the browser and trying again made it work, even though I was using Private browsing. Weird.

whisperity

Alright, the code looks nice and now works. (Dojo is weird I guess...)

However, I really don't like the fact that when I click on a checker name itself (so without filtering for a detection or review status) I get a filter where every possible status is selected, causing, IMHO, a bloat in the filter list. However, if I specifically click on the number in the All reports column, I don't get these "status" filters set.

whisperity · 2018-03-05T13:45:53Z

api/v6/report_server.thrift

@@ -253,7 +263,8 @@ service codeCheckerDBAccess {
  // PERMISSION: PRODUCT_ACCESS
  RunHistoryDataList getRunHistory(1: list<i64> runIds,
                                   2: i64       limit,
-                                   3: i64       offset)
+                                   3: i64       offset,
+                                   4: RunHistoryFilter runHistoryFilter)


Is extending a function signature with a new variable backwards compatible? (I know we have learnt extending records are.)

Yes. E.g.: https://github.com/Ericsson/codechecker/pull/1094/files

csordasmarton · 2018-03-05T13:53:04Z

@whisperity I removed that piece of code which selected all Review status and Detection status by clicking on a Checker name.

gyorb · 2018-03-06T15:30:44Z

@csordasmarton if I click on the detection date filter and select today the report list and the filters are updated, shouldn't they be updated only when I click out of the filter box?

gyorb · 2018-03-06T15:34:57Z

I for the first click the filter boxes can be viewed only partially because they go out of the screen at the bottom if there is a long list. On the second click they will be rendered above the filter section so I can view the list.

csordasmarton · 2018-03-06T16:41:35Z

@gyorb Done.

gyorb · 2018-03-07T11:37:12Z

@csordasmarton I used bzip as a test project.

filter on a file by regex bzlib (there are low and medium results)
filter by severity (the count is 0 for all of the severity levels)
select one severity level and the count in the severity filter is still 0

* Create separate files for each base filters. * Create new filter interface. * Extend API with tag filter. * Remove `CountFilter` type. The client should set the correct filter set instead of checking the filter type. * Do not show default values in URL query parameter for `is-unique` and `difftype`. * Notify other filters on filter change when users changed the filter and the tooltip is closed. * Get items from the server only if some filter is changed or if the filter uses server side search.

csordasmarton · 2018-03-07T12:56:27Z

@gyorb Fixed

I used bzip as a test project.

filter on a file by regex bzlib (there are low and medium results)

filter by severity (the count is 0 for all of the severity levels)

select one severity level and the count in the severity filter is still 0

csordasmarton added enhancement 🌟 GUI 🎨 bugfix 🔨 labels Feb 26, 2018

csordasmarton added this to the release 6.6 milestone Feb 26, 2018

csordasmarton requested review from whisperity, gyorb and bruntib February 26, 2018 12:44

csordasmarton force-pushed the refactoring-ui-filter branch from eaf39f4 to f83c320 Compare February 26, 2018 16:13

bruntib reviewed Feb 28, 2018

View reviewed changes

csordasmarton force-pushed the refactoring-ui-filter branch 2 times, most recently from 2eb8d3a to c105dbd Compare March 2, 2018 09:16

csordasmarton mentioned this pull request Mar 2, 2018

Filter cmd line checker statistics #1416

Merged

csordasmarton force-pushed the refactoring-ui-filter branch from c105dbd to db4f070 Compare March 5, 2018 13:25

whisperity requested changes Mar 5, 2018

View reviewed changes

whisperity reviewed Mar 5, 2018

View reviewed changes

csordasmarton force-pushed the refactoring-ui-filter branch from db4f070 to d4c525e Compare March 5, 2018 13:51

whisperity approved these changes Mar 5, 2018

View reviewed changes

csordasmarton force-pushed the refactoring-ui-filter branch from d4c525e to f0708f5 Compare March 6, 2018 16:40

csordasmarton force-pushed the refactoring-ui-filter branch from f0708f5 to b369faf Compare March 7, 2018 12:52

gyorb approved these changes Mar 8, 2018

View reviewed changes

gyorb merged commit a300c69 into Ericsson:master Mar 8, 2018

csordasmarton deleted the refactoring-ui-filter branch March 8, 2018 14:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring report filter UI #1401

Refactoring report filter UI #1401

csordasmarton commented Feb 26, 2018

bruntib Feb 28, 2018

csordasmarton Mar 2, 2018

bruntib Feb 28, 2018

bruntib Feb 28, 2018

bruntib Feb 28, 2018

bruntib Feb 28, 2018

bruntib Feb 28, 2018

whisperity commented Mar 5, 2018

csordasmarton commented Mar 5, 2018

whisperity commented Mar 5, 2018

whisperity left a comment

whisperity Mar 5, 2018

csordasmarton Mar 5, 2018

csordasmarton commented Mar 5, 2018

gyorb commented Mar 6, 2018

gyorb commented Mar 6, 2018

csordasmarton commented Mar 6, 2018

gyorb commented Mar 7, 2018

csordasmarton commented Mar 7, 2018

Refactoring report filter UI #1401

Refactoring report filter UI #1401

Conversation

csordasmarton commented Feb 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whisperity commented Mar 5, 2018

csordasmarton commented Mar 5, 2018

whisperity commented Mar 5, 2018

whisperity left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csordasmarton commented Mar 5, 2018

gyorb commented Mar 6, 2018

gyorb commented Mar 6, 2018

csordasmarton commented Mar 6, 2018

gyorb commented Mar 7, 2018

csordasmarton commented Mar 7, 2018