Merge lp:~blake-rouse/maas/fix-1443959 into lp:~maas-committers/maas/trunk
- fix-1443959
- Merge into trunk
Proposed by
Blake Rouse
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 3986 |
Proposed branch: | lp:~blake-rouse/maas/fix-1443959 |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
289 lines (+112/-30) 4 files modified
src/maasserver/static/js/angular/controllers/nodes_list.js (+20/-4) src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js (+35/-4) src/maasserver/static/js/angular/services/search.js (+32/-20) src/maasserver/static/js/angular/services/tests/test_search.js (+25/-2) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/fix-1443959 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Approve | ||
Review via email:
|
Commit message
Save search filters for nodes and devices when changing the current view in angular.
This will keep the current search when going back and forth between the node listing and node details view. Now if you refresh the page the filters will reset.
Description of the change
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Blake Rouse (blake-rouse) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/maasserver/static/js/angular/controllers/nodes_list.js' | |||
2 | --- src/maasserver/static/js/angular/controllers/nodes_list.js 2015-05-26 13:52:53 +0000 | |||
3 | +++ src/maasserver/static/js/angular/controllers/nodes_list.js 2015-06-03 20:11:57 +0000 | |||
4 | @@ -45,7 +45,7 @@ | |||
5 | 45 | $scope.tabs.nodes.predicate = 'fqdn'; | 45 | $scope.tabs.nodes.predicate = 'fqdn'; |
6 | 46 | $scope.tabs.nodes.allViewableChecked = false; | 46 | $scope.tabs.nodes.allViewableChecked = false; |
7 | 47 | $scope.tabs.nodes.metadata = NodesManager.getMetadata(); | 47 | $scope.tabs.nodes.metadata = NodesManager.getMetadata(); |
9 | 48 | $scope.tabs.nodes.filters = SearchService.emptyFilter; | 48 | $scope.tabs.nodes.filters = SearchService.getEmptyFilter(); |
10 | 49 | $scope.tabs.nodes.column = 'fqdn'; | 49 | $scope.tabs.nodes.column = 'fqdn'; |
11 | 50 | $scope.tabs.nodes.actionOption = null; | 50 | $scope.tabs.nodes.actionOption = null; |
12 | 51 | $scope.tabs.nodes.takeActionOptions = GeneralManager.getData( | 51 | $scope.tabs.nodes.takeActionOptions = GeneralManager.getData( |
13 | @@ -75,7 +75,7 @@ | |||
14 | 75 | $scope.tabs.devices.predicate = 'fqdn'; | 75 | $scope.tabs.devices.predicate = 'fqdn'; |
15 | 76 | $scope.tabs.devices.allViewableChecked = false; | 76 | $scope.tabs.devices.allViewableChecked = false; |
16 | 77 | $scope.tabs.devices.metadata = DevicesManager.getMetadata(); | 77 | $scope.tabs.devices.metadata = DevicesManager.getMetadata(); |
18 | 78 | $scope.tabs.devices.filters = SearchService.emptyFilter; | 78 | $scope.tabs.devices.filters = SearchService.getEmptyFilter(); |
19 | 79 | $scope.tabs.devices.column = 'fqdn'; | 79 | $scope.tabs.devices.column = 'fqdn'; |
20 | 80 | $scope.tabs.devices.actionOption = null; | 80 | $scope.tabs.devices.actionOption = null; |
21 | 81 | $scope.tabs.devices.takeActionOptions = GeneralManager.getData( | 81 | $scope.tabs.devices.takeActionOptions = GeneralManager.getData( |
22 | @@ -289,7 +289,7 @@ | |||
23 | 289 | var filters = SearchService.getCurrentFilters( | 289 | var filters = SearchService.getCurrentFilters( |
24 | 290 | $scope.tabs[tab].search); | 290 | $scope.tabs[tab].search); |
25 | 291 | if(filters === null) { | 291 | if(filters === null) { |
27 | 292 | $scope.tabs[tab].filters = SearchService.emptyFilter; | 292 | $scope.tabs[tab].filters = SearchService.getEmptyFilter(); |
28 | 293 | $scope.tabs[tab].searchValid = false; | 293 | $scope.tabs[tab].searchValid = false; |
29 | 294 | } else { | 294 | } else { |
30 | 295 | $scope.tabs[tab].filters = filters; | 295 | $scope.tabs[tab].filters = filters; |
31 | @@ -477,11 +477,27 @@ | |||
32 | 477 | $scope.loading = false; | 477 | $scope.loading = false; |
33 | 478 | }); | 478 | }); |
34 | 479 | 479 | ||
36 | 480 | // Stop polling when the scope is destroyed. | 480 | // Stop polling and save the current filter when the scope is destroyed. |
37 | 481 | $scope.$on("$destroy", function() { | 481 | $scope.$on("$destroy", function() { |
38 | 482 | GeneralManager.stopPolling("osinfo"); | 482 | GeneralManager.stopPolling("osinfo"); |
39 | 483 | SearchService.storeFilters("nodes", $scope.tabs.nodes.filters); | ||
40 | 484 | SearchService.storeFilters("devices", $scope.tabs.devices.filters); | ||
41 | 483 | }); | 485 | }); |
42 | 484 | 486 | ||
43 | 487 | // Restore the filters if any saved. | ||
44 | 488 | var nodesFilter = SearchService.retrieveFilters("nodes"); | ||
45 | 489 | if(angular.isObject(nodesFilter)) { | ||
46 | 490 | $scope.tabs.nodes.search = SearchService.filtersToString( | ||
47 | 491 | nodesFilter); | ||
48 | 492 | $scope.updateFilters("nodes"); | ||
49 | 493 | } | ||
50 | 494 | var devicesFilter = SearchService.retrieveFilters("devices"); | ||
51 | 495 | if(angular.isObject(devicesFilter)) { | ||
52 | 496 | $scope.tabs.devices.search = SearchService.filtersToString( | ||
53 | 497 | devicesFilter); | ||
54 | 498 | $scope.updateFilters("devices"); | ||
55 | 499 | } | ||
56 | 500 | |||
57 | 485 | // Switch to the specified tab, if specified. | 501 | // Switch to the specified tab, if specified. |
58 | 486 | if($routeParams.tab === "nodes" || $routeParams.tab === "devices") { | 502 | if($routeParams.tab === "nodes" || $routeParams.tab === "devices") { |
59 | 487 | $scope.toggleTab($routeParams.tab); | 503 | $scope.toggleTab($routeParams.tab); |
60 | 488 | 504 | ||
61 | === modified file 'src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js' | |||
62 | --- src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js 2015-05-26 13:52:53 +0000 | |||
63 | +++ src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js 2015-06-03 20:11:57 +0000 | |||
64 | @@ -149,6 +149,18 @@ | |||
65 | 149 | "osinfo"); | 149 | "osinfo"); |
66 | 150 | }); | 150 | }); |
67 | 151 | 151 | ||
68 | 152 | it("saves current filters for nodes and devices when scope destroyed", | ||
69 | 153 | function() { | ||
70 | 154 | var controller = makeController(); | ||
71 | 155 | var nodesFilters = {}, devicesFilters = {}; | ||
72 | 156 | $scope.tabs.nodes.filters = nodesFilters; | ||
73 | 157 | $scope.tabs.devices.filters = devicesFilters; | ||
74 | 158 | $scope.$destroy(); | ||
75 | 159 | expect(SearchService.retrieveFilters("nodes")).toBe(nodesFilters); | ||
76 | 160 | expect(SearchService.retrieveFilters("devices")).toBe( | ||
77 | 161 | devicesFilters); | ||
78 | 162 | }); | ||
79 | 163 | |||
80 | 152 | it("calls loadManagers with NodesManager, DevicesManager," + | 164 | it("calls loadManagers with NodesManager, DevicesManager," + |
81 | 153 | "GeneralManager, UsersManager", | 165 | "GeneralManager, UsersManager", |
82 | 154 | function() { | 166 | function() { |
83 | @@ -166,6 +178,24 @@ | |||
84 | 166 | expect($scope.loading).toBe(false); | 178 | expect($scope.loading).toBe(false); |
85 | 167 | }); | 179 | }); |
86 | 168 | 180 | ||
87 | 181 | it("sets nodes search from SearchService", | ||
88 | 182 | function() { | ||
89 | 183 | var query = makeName("query"); | ||
90 | 184 | SearchService.storeFilters( | ||
91 | 185 | "nodes", SearchService.getCurrentFilters(query)); | ||
92 | 186 | var controller = makeController(); | ||
93 | 187 | expect($scope.tabs.nodes.search).toBe(query); | ||
94 | 188 | }); | ||
95 | 189 | |||
96 | 190 | it("sets devices search from SearchService", | ||
97 | 191 | function() { | ||
98 | 192 | var query = makeName("query"); | ||
99 | 193 | SearchService.storeFilters( | ||
100 | 194 | "devices", SearchService.getCurrentFilters(query)); | ||
101 | 195 | var controller = makeController(); | ||
102 | 196 | expect($scope.tabs.devices.search).toBe(query); | ||
103 | 197 | }); | ||
104 | 198 | |||
105 | 169 | it("sets nodes search from $routeParams.query", | 199 | it("sets nodes search from $routeParams.query", |
106 | 170 | function() { | 200 | function() { |
107 | 171 | var query = makeName("query"); | 201 | var query = makeName("query"); |
108 | @@ -228,7 +258,8 @@ | |||
109 | 228 | expect(tabScope.selectedItems).toBe( | 258 | expect(tabScope.selectedItems).toBe( |
110 | 229 | tabScope.manager.getSelectedItems()); | 259 | tabScope.manager.getSelectedItems()); |
111 | 230 | expect(tabScope.metadata).toBe(tabScope.manager.getMetadata()); | 260 | expect(tabScope.metadata).toBe(tabScope.manager.getMetadata()); |
113 | 231 | expect(tabScope.filters).toBe(SearchService.emptyFilter); | 261 | expect(tabScope.filters).toEqual( |
114 | 262 | SearchService.getEmptyFilter()); | ||
115 | 232 | expect(tabScope.column).toBe("fqdn"); | 263 | expect(tabScope.column).toBe("fqdn"); |
116 | 233 | expect(tabScope.actionOption).toBeNull(); | 264 | expect(tabScope.actionOption).toBeNull(); |
117 | 234 | expect(tabScope.takeActionOptions).toEqual([]); | 265 | expect(tabScope.takeActionOptions).toEqual([]); |
118 | @@ -440,7 +471,7 @@ | |||
119 | 440 | it("calls SearchService.toggleFilter", function() { | 471 | it("calls SearchService.toggleFilter", function() { |
120 | 441 | var controller = makeController(); | 472 | var controller = makeController(); |
121 | 442 | spyOn(SearchService, "toggleFilter").and.returnValue( | 473 | spyOn(SearchService, "toggleFilter").and.returnValue( |
123 | 443 | SearchService.emptyFilter); | 474 | SearchService.getEmptyFilter()); |
124 | 444 | $scope.toggleFilter("hostname", "test", tab); | 475 | $scope.toggleFilter("hostname", "test", tab); |
125 | 445 | expect(SearchService.toggleFilter).toHaveBeenCalled(); | 476 | expect(SearchService.toggleFilter).toHaveBeenCalled(); |
126 | 446 | }); | 477 | }); |
127 | @@ -507,8 +538,8 @@ | |||
128 | 507 | $scope.tabs[tab].search = "test hostname:(name"; | 538 | $scope.tabs[tab].search = "test hostname:(name"; |
129 | 508 | $scope.updateFilters(tab); | 539 | $scope.updateFilters(tab); |
130 | 509 | expect( | 540 | expect( |
133 | 510 | $scope.tabs[tab].filters).toBe( | 541 | $scope.tabs[tab].filters).toEqual( |
134 | 511 | SearchService.emptyFilter); | 542 | SearchService.getEmptyFilter()); |
135 | 512 | expect($scope.tabs[tab].searchValid).toBe(false); | 543 | expect($scope.tabs[tab].searchValid).toBe(false); |
136 | 513 | }); | 544 | }); |
137 | 514 | }); | 545 | }); |
138 | 515 | 546 | ||
139 | === modified file 'src/maasserver/static/js/angular/services/search.js' | |||
140 | --- src/maasserver/static/js/angular/services/search.js 2015-01-29 21:35:52 +0000 | |||
141 | +++ src/maasserver/static/js/angular/services/search.js 2015-06-03 20:11:57 +0000 | |||
142 | @@ -6,13 +6,21 @@ | |||
143 | 6 | 6 | ||
144 | 7 | angular.module('MAAS').service('SearchService', function() { | 7 | angular.module('MAAS').service('SearchService', function() { |
145 | 8 | 8 | ||
146 | 9 | // Holds an empty filter object. | ||
147 | 10 | var emptyFilter = { _: [] }; | ||
148 | 11 | |||
149 | 12 | // Return a new empty filter; | ||
150 | 13 | this.getEmptyFilter = function() { | ||
151 | 14 | return angular.copy(emptyFilter); | ||
152 | 15 | }; | ||
153 | 16 | |||
154 | 9 | // Splits the search string into different terms based on white space. | 17 | // Splits the search string into different terms based on white space. |
155 | 10 | // This handles the ability for whitespace to be inside of '(', ')'. | 18 | // This handles the ability for whitespace to be inside of '(', ')'. |
156 | 11 | // | 19 | // |
157 | 12 | // XXX blake_r 28-01-15: This could be improved with a regex, but was | 20 | // XXX blake_r 28-01-15: This could be improved with a regex, but was |
158 | 13 | // unable to come up with one that would allow me to validate the end | 21 | // unable to come up with one that would allow me to validate the end |
159 | 14 | // ')' in the string. | 22 | // ')' in the string. |
161 | 15 | function getSplitSearch(search) { | 23 | this.getSplitSearch = function(search) { |
162 | 16 | var terms = search.split(' '); | 24 | var terms = search.split(' '); |
163 | 17 | var fixedTerms = []; | 25 | var fixedTerms = []; |
164 | 18 | var spanningParentheses = false; | 26 | var spanningParentheses = false; |
165 | @@ -46,15 +54,15 @@ | |||
166 | 46 | return null; | 54 | return null; |
167 | 47 | } | 55 | } |
168 | 48 | return fixedTerms; | 56 | return fixedTerms; |
170 | 49 | } | 57 | }; |
171 | 50 | 58 | ||
172 | 51 | // Return all of the currently active filters for the given search. | 59 | // Return all of the currently active filters for the given search. |
175 | 52 | function getCurrentFilters(search) { | 60 | this.getCurrentFilters = function(search) { |
176 | 53 | var filters = { _: [] }; | 61 | var filters = this.getEmptyFilter(); |
177 | 54 | if(search.length === 0) { | 62 | if(search.length === 0) { |
178 | 55 | return filters; | 63 | return filters; |
179 | 56 | } | 64 | } |
181 | 57 | var searchTerms = getSplitSearch(search); | 65 | var searchTerms = this.getSplitSearch(search); |
182 | 58 | if(!searchTerms) { | 66 | if(!searchTerms) { |
183 | 59 | return null; | 67 | return null; |
184 | 60 | } | 68 | } |
185 | @@ -94,10 +102,10 @@ | |||
186 | 94 | } | 102 | } |
187 | 95 | }); | 103 | }); |
188 | 96 | return filters; | 104 | return filters; |
190 | 97 | } | 105 | }; |
191 | 98 | 106 | ||
192 | 99 | // Convert "filters" into a search string. | 107 | // Convert "filters" into a search string. |
194 | 100 | function filtersToString(filters) { | 108 | this.filtersToString = function(filters) { |
195 | 101 | var search = ""; | 109 | var search = ""; |
196 | 102 | angular.forEach(filters, function(terms, type) { | 110 | angular.forEach(filters, function(terms, type) { |
197 | 103 | // Skip empty and skip "_" as it gets appended at the | 111 | // Skip empty and skip "_" as it gets appended at the |
198 | @@ -111,19 +119,19 @@ | |||
199 | 111 | search = filters._.join(" ") + " " + search; | 119 | search = filters._.join(" ") + " " + search; |
200 | 112 | } | 120 | } |
201 | 113 | return search.trim(); | 121 | return search.trim(); |
203 | 114 | } | 122 | }; |
204 | 115 | 123 | ||
205 | 116 | // Return true if the type and value are in the filters. | 124 | // Return true if the type and value are in the filters. |
207 | 117 | function isFilterActive(filters, type, value) { | 125 | this.isFilterActive = function(filters, type, value) { |
208 | 118 | var values = filters[type]; | 126 | var values = filters[type]; |
209 | 119 | if(angular.isUndefined(values)) { | 127 | if(angular.isUndefined(values)) { |
210 | 120 | return false; | 128 | return false; |
211 | 121 | } | 129 | } |
212 | 122 | return values.indexOf(value) !== -1; | 130 | return values.indexOf(value) !== -1; |
214 | 123 | } | 131 | }; |
215 | 124 | 132 | ||
216 | 125 | // Toggles a filter on or off based on type and value. | 133 | // Toggles a filter on or off based on type and value. |
218 | 126 | function toggleFilter(filters, type, value) { | 134 | this.toggleFilter = function(filters, type, value) { |
219 | 127 | if(angular.isUndefined(filters[type])) { | 135 | if(angular.isUndefined(filters[type])) { |
220 | 128 | filters[type] = []; | 136 | filters[type] = []; |
221 | 129 | } | 137 | } |
222 | @@ -134,14 +142,18 @@ | |||
223 | 134 | filters[type].splice(idx, 1); | 142 | filters[type].splice(idx, 1); |
224 | 135 | } | 143 | } |
225 | 136 | return filters; | 144 | return filters; |
235 | 137 | } | 145 | }; |
236 | 138 | 146 | ||
237 | 139 | return { | 147 | // Holds all stored filters. |
238 | 140 | emptyFilter: { _: [] }, | 148 | var storedFilters = {}; |
239 | 141 | getSplitSearch: getSplitSearch, | 149 | |
240 | 142 | getCurrentFilters: getCurrentFilters, | 150 | // Store a filter for later. |
241 | 143 | filtersToString: filtersToString, | 151 | this.storeFilters = function(name, filters) { |
242 | 144 | isFilterActive: isFilterActive, | 152 | storedFilters[name] = filters; |
243 | 145 | toggleFilter: toggleFilter | 153 | }; |
244 | 154 | |||
245 | 155 | // Retrieve a stored fitler. | ||
246 | 156 | this.retrieveFilters = function(name) { | ||
247 | 157 | return storedFilters[name]; | ||
248 | 146 | }; | 158 | }; |
249 | 147 | }); | 159 | }); |
250 | 148 | 160 | ||
251 | === modified file 'src/maasserver/static/js/angular/services/tests/test_search.js' | |||
252 | --- src/maasserver/static/js/angular/services/tests/test_search.js 2015-01-29 21:35:52 +0000 | |||
253 | +++ src/maasserver/static/js/angular/services/tests/test_search.js 2015-06-03 20:11:57 +0000 | |||
254 | @@ -146,10 +146,33 @@ | |||
255 | 146 | }); | 146 | }); |
256 | 147 | }); | 147 | }); |
257 | 148 | 148 | ||
259 | 149 | describe("emptyFilter", function() { | 149 | describe("getEmptyFilter", function() { |
260 | 150 | 150 | ||
261 | 151 | it("includes _ empty list", function() { | 151 | it("includes _ empty list", function() { |
263 | 152 | expect(SearchService.emptyFilter).toEqual({ _: [] }); | 152 | expect(SearchService.getEmptyFilter()).toEqual({ _: [] }); |
264 | 153 | }); | ||
265 | 154 | |||
266 | 155 | it("returns different object on each call", function() { | ||
267 | 156 | var one = SearchService.getEmptyFilter(); | ||
268 | 157 | var two = SearchService.getEmptyFilter(); | ||
269 | 158 | expect(one).not.toBe(two); | ||
270 | 159 | }); | ||
271 | 160 | }); | ||
272 | 161 | |||
273 | 162 | describe("storeFilters/retrieveFilters", function() { | ||
274 | 163 | |||
275 | 164 | it("stores and retrieves the same object", function() { | ||
276 | 165 | var i, names = [], objects = []; | ||
277 | 166 | for(i = 0; i < 3; i++) { | ||
278 | 167 | names.push(makeName("name")); | ||
279 | 168 | objects.push({}); | ||
280 | 169 | } | ||
281 | 170 | angular.forEach(names, function(name, idx) { | ||
282 | 171 | SearchService.storeFilters(name, objects[idx]); | ||
283 | 172 | }); | ||
284 | 173 | angular.forEach(names, function(name, idx) { | ||
285 | 174 | expect(SearchService.retrieveFilters(name)).toBe(objects[idx]); | ||
286 | 175 | }); | ||
287 | 153 | }); | 176 | }); |
288 | 154 | }); | 177 | }); |
289 | 155 | }); | 178 | }); |
Looks generally good. I have a minor comment below about some code that looks to bit a bit boilerplate. I think we might be able to do better with some slight refactoring; right now it seems like we have objects and constants scattered about for the nodes and devices; maybe if a single object knew about all these, it would be cleaner?