644435 - Ability to filter results on machine's name

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Working on the Fennec mobile front-end side, I'm mostly interested by some particular tests (browser-chrome in this case) and by the related failures since those are the tests of the browser UI.

Having all the machine/tests results make tbpl unuseful to track that and so the patch add a filter functionality on machine's name to handle that.

Comment 1

[Mounir Lamouri (:mounir)]

Comment on attachment 521376 [details] [diff] [review]
Patch

>diff --git a/js/UserInterface.js b/js/UserInterface.js
>--- a/js/UserInterface.js
>+++ b/js/UserInterface.js
>@@ -5,23 +5,25 @@ var UserInterface = {
> 
>   _controller: null,
>   _treeName: "",
>   _data: null,
>   _activeResult: "",
>   _storage: null,
>   _onlyUnstarred: false,
>   _pusher: "",
>+  _machine: null,

"" seems more appropriate.

>       if (!state) {
>         var params = self._controller.getParams();
>         self._updatePusherFilter(params.pusher ? params.pusher : "");
>         self._updateUnstarredFilter(params.onlyunstarred == '1');
>         return;
>       }
> 
>       self._updatePusherFilter(state['pusher'] ? state['pusher'] : "");
>+      self._updateMachineFilter(state['machine'] ? state['machine'] : null);
>       self._updateUnstarredFilter(state['onlyUnstarred']);
>     }

You need to add:
self.updateMachineFilter(params.machine ? params.machine : "");
inside the !state block so we revert to no filter when going back to the first history item.

In addition, I wonder if 'machine' is an appropriate name. Why not "jobs" or "job name"?

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

> >       if (!state) {
> >         var params = self._controller.getParams();
> >         self._updatePusherFilter(params.pusher ? params.pusher : "");
> >         self._updateUnstarredFilter(params.onlyunstarred == '1');
> >         return;
> >       }
> > 
> >       self._updatePusherFilter(state['pusher'] ? state['pusher'] : "");
> >+      self._updateMachineFilter(state['machine'] ? state['machine'] : null);
> >       self._updateUnstarredFilter(state['onlyUnstarred']);
> >     }
> 
> You need to add:
> self.updateMachineFilter(params.machine ? params.machine : "");
> inside the !state block so we revert to no filter when going back to the first
> history item.

Thanks, I've missed that.

> 
> In addition, I wonder if 'machine' is an appropriate name. Why not "jobs" or
> "job name"?

I agree that machine does not seems appropriate here but this is to stay in sync with property name coming from the json data.
But if it make things unclear for users I can rename the url parameter as well as the label in the filter menu?

Comment 3

[Arpad Borsos [:Swatinem]]

Comment on attachment 521376 [details] [diff] [review]
Patch

>   _filterDisplayedResults: function UserInterface__filterDisplayedResults(results) {
>-    if (!this._onlyUnstarred)
>+    if (!this._onlyUnstarred && !this._machine)
>       return results;
> 
>     var self = this;
>-    return results.filter(function (result) {
>-      return self._isUnstarredFailure(result);
>-    });
>+    var filteredResults = results;
>+
>+    if (this._onlyUnstarred) {
>+     filteredResults = results.filter(function (result) {
>+        return self._isUnstarredFailure(result);
>+      });
>+    }
>+
>+    if (this._machine) {
>+     var machineFilter = new RegExp(this._machine, "i");
>+     filteredResults = results.filter(function (result) {
>+       return result.machine.name.match(machineFilter) != null;
>+     });
>+    }
>+
>+    return filteredResults;

Make sure both filters work together (The second overwrites the first result). You can maybe push both conditions into one .filter() call?

+ the things Mounir mentioned

Comment 4

[Mounir Lamouri (:mounir)]

(In reply to comment #2)
> > In addition, I wonder if 'machine' is an appropriate name. Why not "jobs" or
> > "job name"?
> 
> I agree that machine does not seems appropriate here but this is to stay in
> sync with property name coming from the json data.
> But if it make things unclear for users I can rename the url parameter as well
> as the label in the filter menu?

Seems a good plan to me.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Address both comments.

Comment 6

[Arpad Borsos [:Swatinem]]

Comment on attachment 521458 [details] [diff] [review]
Patch v0.2

>+     filteredResults = filteredResults.filter(function (result) {
>+       return result.machine.name.match(machineFilter) != null;
>+     });

Make that 
return machineFilter.test(result.machine.name);

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/3210bca26228