Merge lp:~allenap/maas/notifications-creation into lp:~maas-committers/maas/trunk
- notifications-creation
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 5702 |
Proposed branch: | lp:~allenap/maas/notifications-creation |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
544 lines (+100/-133) 19 files modified
src/maasserver/bootsources.py (+3/-1) src/maasserver/components.py (+30/-20) src/maasserver/context_processors.py (+0/-2) src/maasserver/migrations/builtin/maasserver/0111_remove_component_error.py (+14/-0) src/maasserver/models/__init__.py (+0/-2) src/maasserver/models/component_error.py (+0/-30) src/maasserver/models/notification.py (+9/-2) src/maasserver/models/tests/test_notification.py (+7/-0) src/maasserver/static/js/angular/directives/notifications.js (+1/-1) src/maasserver/static/js/angular/directives/tests/test_notifications.js (+13/-0) src/maasserver/static/js/angular/maas.js (+1/-1) src/maasserver/templates/maasserver/base.html (+0/-6) src/maasserver/templates/maasserver/index.html (+0/-6) src/maasserver/tests/test_bootsources.py (+10/-6) src/maasserver/tests/test_components.py (+9/-20) src/maasserver/views/combo.py (+1/-0) src/maasserver/views/tests/test_general.py (+0/-36) src/maastesting/karma.conf.js (+1/-0) utilities/check-imports (+1/-0) |
To merge this branch: | bzr merge lp:~allenap/maas/notifications-creation |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Blake Rouse (community) | Approve | ||
Review via email: mp+316718@code.launchpad.net |
Commit message
Switch the persistent error mechanism to use notifications as its back-end.
Previously it was using ComponentError, which this change also removes.
Description of the change
Gavin Panella (allenap) wrote : | # |
> Looks good. Other than the one question I have. Which actually might
> allow you to remove that new dependency you added.
The new dependency is something we already pull in — python3-sphinx
depends on it, indirectly — so the net effect is zero. It'll only matter
if the dependencies of python3-sphinx change or if we stop depending on
python3-sphinx.
> We need to do some work to get the notifications to show on the none
> angular pages as well, since this branch removes notifications from
> those pages. Its not to hard to enable.
I'm interested to know how to do that.
Thanks!
MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~allenap/maas/notifications-creation into lp:maas failed. Below is the output from the failed tests.
Hit:1 http://
Get:2 http://
Get:3 http://
Get:4 http://
Get:5 http://
Get:6 http://
Get:7 http://
Get:8 http://
Fetched 1,588 kB in 0s (2,706 kB/s)
Reading package lists...
sudo DEBIAN_
--no-
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubun
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5....
Gavin Panella (allenap) wrote : | # |
Unrelated test failure, as far as I can tell :-/ Nothing obviously wrong with maasserver.
=======
ERROR: maasserver.
-------
Traceback (most recent call last):
testtools.
Twisted logs
Traceback (most recent call last):
File "/tmp/tarmac/
result = function(*args, **kwargs)
File "/home/
return self._get_
File "/tmp/tarmac/
response = self.client.
File "/usr/lib/
secure=secure, **extra)
File "/usr/lib/
secure=secure, **extra)
File "/usr/lib/
return self.request(**r)
File "/tmp/tarmac/
return super(MAASSensi
File "/tmp/tarmac/
return upcall(**request)
File "/tmp/tarmac/
self.fire()
File "/tmp/tarmac/
result = func(*args, **kwargs)
File "/tmp/tarmac/
self.
File "/usr/lib/
result.
File "/usr/lib/
raise self.value.
maasserver.
-------
maas.node: info: I5NT0fKaf40Y9kt
maas.node: error: I5NT0fKaf40Y9kt
-------
=======
ERROR: maasserver.
-------
Traceback (most recent call last):
testtools.
Twisted logs
Traceback (most recent call last):
File "/tmp/tarmac/
Preview Diff
1 | === modified file 'src/maasserver/bootsources.py' |
2 | --- src/maasserver/bootsources.py 2016-09-30 13:56:04 +0000 |
3 | +++ src/maasserver/bootsources.py 2017-02-09 09:06:11 +0000 |
4 | @@ -10,6 +10,7 @@ |
5 | "get_os_info_from_boot_sources", |
6 | ] |
7 | |
8 | +import html |
9 | import os |
10 | |
11 | from maasserver.components import ( |
12 | @@ -208,7 +209,8 @@ |
13 | component = COMPONENT.REGION_IMAGE_IMPORT |
14 | if len(errors) > 0: |
15 | yield deferToDatabase( |
16 | - register_persistent_error, component, "\n".join(errors)) |
17 | + register_persistent_error, component, |
18 | + "<br>".join(map(html.escape, errors))) |
19 | else: |
20 | yield deferToDatabase( |
21 | discard_persistent_error, component) |
22 | |
23 | === modified file 'src/maasserver/components.py' |
24 | --- src/maasserver/components.py 2016-06-30 16:49:02 +0000 |
25 | +++ src/maasserver/components.py 2017-02-09 09:06:11 +0000 |
26 | @@ -1,21 +1,25 @@ |
27 | # Copyright 2012-2016 Canonical Ltd. This software is licensed under the |
28 | # GNU Affero General Public License version 3 (see the file LICENSE). |
29 | |
30 | -"""MAAS components management.""" |
31 | +"""MAAS components notifications. |
32 | + |
33 | +This is a legacy compatibility shim, to use the new notifications feature to |
34 | +display the old components messages. In time this will go away, but it's |
35 | +simple enough that there's no rush until it doesn't actually do what we want |
36 | +it to do. |
37 | +""" |
38 | |
39 | __all__ = [ |
40 | "discard_persistent_error", |
41 | "get_persistent_error", |
42 | "get_persistent_errors", |
43 | "register_persistent_error", |
44 | - ] |
45 | +] |
46 | |
47 | -from django.utils.safestring import mark_safe |
48 | -from maasserver.models import ComponentError |
49 | -from maasserver.utils.orm import ( |
50 | - get_one, |
51 | - transactional, |
52 | -) |
53 | +from maasserver.enum import COMPONENT |
54 | +from maasserver.models import Notification |
55 | +from maasserver.utils.orm import transactional |
56 | +from provisioningserver.utils.enum import map_enum |
57 | |
58 | |
59 | @transactional |
60 | @@ -24,7 +28,7 @@ |
61 | |
62 | :param component: An enum value of :class:`COMPONENT`. |
63 | """ |
64 | - ComponentError.objects.filter(component=component).delete() |
65 | + Notification.objects.filter(ident=component).delete() |
66 | |
67 | |
68 | @transactional |
69 | @@ -34,25 +38,31 @@ |
70 | :param component: An enum value of :class:`COMPONENT`. |
71 | :param error_message: Human-readable error text. |
72 | """ |
73 | - component_error, created = ComponentError.objects.get_or_create( |
74 | - component=component, defaults={'error': error_message}) |
75 | - # If we didn't create a new object, we may need to update it if the error |
76 | - # message is different. |
77 | - if not created and component_error.error != error_message: |
78 | - component_error.error = error_message |
79 | - component_error.save() |
80 | + try: |
81 | + notification = Notification.objects.get(ident=component) |
82 | + except Notification.DoesNotExist: |
83 | + notification = Notification.objects.create_error_for_admins( |
84 | + error_message, ident=component) |
85 | + else: |
86 | + if notification.message != error_message: |
87 | + notification.message = error_message |
88 | + notification.save() |
89 | |
90 | |
91 | def get_persistent_error(component): |
92 | """Return persistent error for `component`, or None.""" |
93 | - err = get_one(ComponentError.objects.filter(component=component)) |
94 | - if err is None: |
95 | + try: |
96 | + notification = Notification.objects.get(ident=component) |
97 | + except Notification.DoesNotExist: |
98 | return None |
99 | else: |
100 | - return err.error |
101 | + return notification.render() |
102 | |
103 | |
104 | def get_persistent_errors(): |
105 | """Return list of current persistent error messages.""" |
106 | + components = map_enum(COMPONENT).values() |
107 | return sorted( |
108 | - mark_safe(err.error) for err in ComponentError.objects.all()) |
109 | + notification.render() for notification in |
110 | + Notification.objects.filter(ident__in=components) |
111 | + ) |
112 | |
113 | === modified file 'src/maasserver/context_processors.py' |
114 | --- src/maasserver/context_processors.py 2016-09-22 02:53:33 +0000 |
115 | +++ src/maasserver/context_processors.py 2017-02-09 09:06:11 +0000 |
116 | @@ -9,7 +9,6 @@ |
117 | ] |
118 | |
119 | from django.conf import settings |
120 | -from maasserver.components import get_persistent_errors |
121 | from maasserver.config import RegionConfiguration |
122 | from maasserver.models import Config |
123 | from maasserver.utils.version import ( |
124 | @@ -32,7 +31,6 @@ |
125 | if hasattr(request.user, 'userprofile'): |
126 | user_completed_intro = request.user.userprofile.completed_intro |
127 | return { |
128 | - 'persistent_errors': get_persistent_errors(), |
129 | 'global_options': { |
130 | 'site_name': Config.objects.get_config('maas_name'), |
131 | 'enable_analytics': Config.objects.get_config('enable_analytics'), |
132 | |
133 | === added file 'src/maasserver/migrations/builtin/maasserver/0111_remove_component_error.py' |
134 | --- src/maasserver/migrations/builtin/maasserver/0111_remove_component_error.py 1970-01-01 00:00:00 +0000 |
135 | +++ src/maasserver/migrations/builtin/maasserver/0111_remove_component_error.py 2017-02-09 09:06:11 +0000 |
136 | @@ -0,0 +1,14 @@ |
137 | +from django.db import migrations |
138 | + |
139 | + |
140 | +class Migration(migrations.Migration): |
141 | + |
142 | + dependencies = [ |
143 | + ('maasserver', '0110_notification_category'), |
144 | + ] |
145 | + |
146 | + operations = [ |
147 | + migrations.DeleteModel( |
148 | + name='ComponentError', |
149 | + ), |
150 | + ] |
151 | |
152 | === modified file 'src/maasserver/models/__init__.py' |
153 | --- src/maasserver/models/__init__.py 2017-01-23 19:56:16 +0000 |
154 | +++ src/maasserver/models/__init__.py 2017-02-09 09:06:11 +0000 |
155 | @@ -17,7 +17,6 @@ |
156 | 'BootSourceSelection', |
157 | 'BridgeInterface', |
158 | 'CacheSet', |
159 | - 'ComponentError', |
160 | 'Config', |
161 | 'Controller', |
162 | 'Device', |
163 | @@ -108,7 +107,6 @@ |
164 | from maasserver.models.bootsourcecache import BootSourceCache |
165 | from maasserver.models.bootsourceselection import BootSourceSelection |
166 | from maasserver.models.cacheset import CacheSet |
167 | -from maasserver.models.component_error import ComponentError |
168 | from maasserver.models.config import Config |
169 | from maasserver.models.dhcpsnippet import DHCPSnippet |
170 | from maasserver.models.discovery import Discovery |
171 | |
172 | === removed file 'src/maasserver/models/component_error.py' |
173 | --- src/maasserver/models/component_error.py 2015-12-01 18:12:59 +0000 |
174 | +++ src/maasserver/models/component_error.py 1970-01-01 00:00:00 +0000 |
175 | @@ -1,30 +0,0 @@ |
176 | -# Copyright 2012-2015 Canonical Ltd. This software is licensed under the |
177 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
178 | - |
179 | -"""Persistent component errors.""" |
180 | - |
181 | -__all__ = [ |
182 | - 'ComponentError', |
183 | - ] |
184 | - |
185 | - |
186 | -from django.db.models import CharField |
187 | -from maasserver import DefaultMeta |
188 | -from maasserver.models.cleansave import CleanSave |
189 | -from maasserver.models.timestampedmodel import TimestampedModel |
190 | - |
191 | - |
192 | -class ComponentError(CleanSave, TimestampedModel): |
193 | - """Error state of a major component of the system.""" |
194 | - |
195 | - class Meta(DefaultMeta): |
196 | - """Needed for South to recognize this model.""" |
197 | - |
198 | - # A descriptor for the failing component, as in the COMPONENT enum. |
199 | - # This is a failure state for an out-of-process component. We won't |
200 | - # know much about what's wrong, and we don't support multiple errors |
201 | - # for a single component. |
202 | - component = CharField(max_length=40, unique=True, blank=False) |
203 | - |
204 | - # Human-readable description of what's wrong. |
205 | - error = CharField(max_length=1000, blank=False) |
206 | |
207 | === modified file 'src/maasserver/models/notification.py' |
208 | --- src/maasserver/models/notification.py 2017-02-02 17:10:32 +0000 |
209 | +++ src/maasserver/models/notification.py 2017-02-09 09:06:11 +0000 |
210 | @@ -23,6 +23,7 @@ |
211 | from maasserver.fields import JSONObjectField |
212 | from maasserver.models.cleansave import CleanSave |
213 | from maasserver.models.timestampedmodel import TimestampedModel |
214 | +from markupsafe import Markup |
215 | |
216 | |
217 | def _create(method, category): |
218 | @@ -172,8 +173,14 @@ |
219 | ) |
220 | |
221 | def render(self): |
222 | - """Render this notification's message using its context.""" |
223 | - return self.message.format(**self.context) |
224 | + """Render this notification's message using its context. |
225 | + |
226 | + The message can contain HTML markup. Values from the context are |
227 | + escaped. |
228 | + """ |
229 | + markup = Markup(self.message) |
230 | + markup = markup.format(**self.context) |
231 | + return str(markup) |
232 | |
233 | def is_relevant_to(self, user): |
234 | """Is this notification relevant to the given user?""" |
235 | |
236 | === modified file 'src/maasserver/models/tests/test_notification.py' |
237 | --- src/maasserver/models/tests/test_notification.py 2017-02-02 17:10:32 +0000 |
238 | +++ src/maasserver/models/tests/test_notification.py 2017-02-09 09:06:11 +0000 |
239 | @@ -167,6 +167,13 @@ |
240 | "There are " + str(thing_b) + " of " + |
241 | thing_a + " in my suitcase.")) |
242 | |
243 | + def test_render_allows_markup_in_message_but_escapes_context(self): |
244 | + message = "<foo>{bar}</foo>" |
245 | + context = {"bar": "<BAR>"} |
246 | + notification = Notification(message=message, context=context) |
247 | + self.assertThat( |
248 | + notification.render(), Equals("<foo><BAR></foo>")) |
249 | + |
250 | def test_save_checks_that_rendering_works(self): |
251 | message = "Dude, where's my {thing}?" |
252 | notification = Notification(message=message) |
253 | |
254 | === modified file 'src/maasserver/static/js/angular/directives/notifications.js' |
255 | --- src/maasserver/static/js/angular/directives/notifications.js 2017-02-03 17:03:51 +0000 |
256 | +++ src/maasserver/static/js/angular/directives/notifications.js 2017-02-09 09:06:11 +0000 |
257 | @@ -10,7 +10,7 @@ |
258 | '<div ng-repeat="n in notifications" ng-class="classes[n.category]">', |
259 | '<p class="p-notification__response">', |
260 | '<span class="p-notification__status"></span>', |
261 | - '<span>{$ n.message $}</span> — ', |
262 | + '<span ng-bind-html="n.message"></span> — ', |
263 | '<a ng-click="dismiss(n)">Dismiss</a>', |
264 | '<br><small>(id: {$ n.id $}, ', |
265 | 'ident: {$ n.ident || "-" $}, user: {$ n.user || "-" $}, ', |
266 | |
267 | === modified file 'src/maasserver/static/js/angular/directives/tests/test_notifications.js' |
268 | --- src/maasserver/static/js/angular/directives/tests/test_notifications.js 2017-02-03 17:19:44 +0000 |
269 | +++ src/maasserver/static/js/angular/directives/tests/test_notifications.js 2017-02-09 09:06:11 +0000 |
270 | @@ -115,6 +115,19 @@ |
271 | ]); |
272 | }); |
273 | |
274 | + it("sanitizes messages", function() { |
275 | + var harmfulNotification = angular.copy(exampleNotifications[0]); |
276 | + harmfulNotification.message = |
277 | + "Hello <script>alert('Gotcha');</script><em>World</em>!"; |
278 | + theNotificationsManager._items = [harmfulNotification]; |
279 | + var directive = compileDirective(); |
280 | + var messages = directive.find("div > p"); |
281 | + expect(messages.html()).not.toContain("<script>"); |
282 | + expect(messages.html()).not.toContain("Gotcha"); |
283 | + expect(messages.html()).toContain("<em>World</em>"); |
284 | + expect(messages.text()).toContain("Hello World!"); |
285 | + }); |
286 | + |
287 | }); |
288 | |
289 | }); |
290 | |
291 | === modified file 'src/maasserver/static/js/angular/maas.js' |
292 | --- src/maasserver/static/js/angular/maas.js 2016-12-14 11:07:08 +0000 |
293 | +++ src/maasserver/static/js/angular/maas.js 2017-02-09 09:06:11 +0000 |
294 | @@ -9,7 +9,7 @@ |
295 | */ |
296 | |
297 | angular.module('MAAS', |
298 | - ['ngRoute', 'ngCookies', 'ngTagsInput', 'sticky']).config( |
299 | + ['ngRoute', 'ngCookies', 'ngSanitize', 'ngTagsInput', 'sticky']).config( |
300 | function($interpolateProvider, $routeProvider, $httpProvider) { |
301 | $interpolateProvider.startSymbol('{$'); |
302 | $interpolateProvider.endSymbol('$}'); |
303 | |
304 | === modified file 'src/maasserver/templates/maasserver/base.html' |
305 | --- src/maasserver/templates/maasserver/base.html 2017-01-27 11:49:08 +0000 |
306 | +++ src/maasserver/templates/maasserver/base.html 2017-02-09 09:06:11 +0000 |
307 | @@ -150,12 +150,6 @@ |
308 | {% if user.is_authenticated %} |
309 | <div class="row u-padding--top-none u-padding--bottom-none {% block notifications-class %}{% endblock %}"> |
310 | <div class="wrapper--inner"> |
311 | - {% for persistent_error in persistent_errors %} |
312 | - <div class="p-notification--error"> |
313 | - <p class="p-notification__response"> |
314 | - <span class="p-notification__status">Error:</span>{{ persistent_error }}</p> |
315 | - </div> |
316 | - {% endfor %} |
317 | {% if messages %} |
318 | {% for message in messages %} |
319 | <div{% if message.tags %} class="p-notification p-notification--{{ message.tags }}" {% endif %}> |
320 | |
321 | === modified file 'src/maasserver/templates/maasserver/index.html' |
322 | --- src/maasserver/templates/maasserver/index.html 2017-01-13 15:42:46 +0000 |
323 | +++ src/maasserver/templates/maasserver/index.html 2017-02-09 09:06:11 +0000 |
324 | @@ -136,12 +136,6 @@ |
325 | {% if user.is_authenticated %} |
326 | <div class="wrapper--inner"> |
327 | <div class="ng-hide"> |
328 | - {% for persistent_error in persistent_errors %} |
329 | - <div class="p-notification--error"> |
330 | - <p class="p-notification__response"> |
331 | - <span class="p-notification__status">Error:</span>{{ persistent_error }}</p> |
332 | - </div> |
333 | - {% endfor %} |
334 | {% if messages %} |
335 | {% for message in messages %} |
336 | <div{% if message.tags %} class="p-notification--{{ message.tags }}" {% endif %}> |
337 | |
338 | === modified file 'src/maasserver/tests/test_bootsources.py' |
339 | --- src/maasserver/tests/test_bootsources.py 2016-09-30 13:56:04 +0000 |
340 | +++ src/maasserver/tests/test_bootsources.py 2017-02-09 09:06:11 +0000 |
341 | @@ -5,6 +5,7 @@ |
342 | |
343 | __all__ = [] |
344 | |
345 | +import html |
346 | from os import environ |
347 | from unittest import skip |
348 | from unittest.mock import ( |
349 | @@ -319,18 +320,21 @@ |
350 | factory.make_BootSource(keyring_data=b'1234') for _ in range(3)] |
351 | download_image_descriptions = self.patch( |
352 | download_descriptions_module, 'download_image_descriptions') |
353 | - error_text = factory.make_name("error_text") |
354 | + error_text_one = factory.make_name("<error1>") |
355 | + error_text_two = factory.make_name("<error2>") |
356 | # Make two of the downloads fail. |
357 | download_image_descriptions.side_effect = [ |
358 | - ConnectionError(error_text), |
359 | + ConnectionError(error_text_one), |
360 | BootImageMapping(), |
361 | - IOError(error_text), |
362 | + IOError(error_text_two), |
363 | ] |
364 | cache_boot_sources() |
365 | base_error = "Failed to import images from boot source {url}: {err}" |
366 | - error_part_one = base_error.format(url=sources[0].url, err=error_text) |
367 | - error_part_two = base_error.format(url=sources[2].url, err=error_text) |
368 | - expected_error = error_part_one + '\n' + error_part_two |
369 | + error_part_one = base_error.format( |
370 | + url=sources[0].url, err=html.escape(error_text_one)) |
371 | + error_part_two = base_error.format( |
372 | + url=sources[2].url, err=html.escape(error_text_two)) |
373 | + expected_error = error_part_one + '<br>' + error_part_two |
374 | actual_error = get_persistent_error(COMPONENT.REGION_IMAGE_IMPORT) |
375 | self.assertEqual(expected_error, actual_error) |
376 | |
377 | |
378 | === renamed file 'src/maasserver/models/tests/test_components.py' => 'src/maasserver/tests/test_components.py' |
379 | --- src/maasserver/models/tests/test_components.py 2016-06-30 16:49:02 +0000 |
380 | +++ src/maasserver/tests/test_components.py 2017-02-09 09:06:11 +0000 |
381 | @@ -10,12 +10,11 @@ |
382 | |
383 | from maasserver.components import ( |
384 | discard_persistent_error, |
385 | - get_persistent_error, |
386 | get_persistent_errors, |
387 | register_persistent_error, |
388 | ) |
389 | from maasserver.enum import COMPONENT |
390 | -from maasserver.models.component_error import ComponentError |
391 | +from maasserver.models import Notification |
392 | from maasserver.testing.factory import factory |
393 | from maasserver.testing.testcase import MAASServerTestCase |
394 | from provisioningserver.utils.enum import map_enum |
395 | @@ -27,9 +26,6 @@ |
396 | |
397 | class PersistentErrorsUtilitiesTest(MAASServerTestCase): |
398 | |
399 | - def setUp(self): |
400 | - super(PersistentErrorsUtilitiesTest, self).setUp() |
401 | - |
402 | def test_register_persistent_error_registers_error(self): |
403 | error_message = factory.make_string() |
404 | component = get_random_component() |
405 | @@ -70,15 +66,6 @@ |
406 | components.append(component) |
407 | self.assertItemsEqual(errors, get_persistent_errors()) |
408 | |
409 | - def test_get_persistent_error_returns_None_if_no_error(self): |
410 | - self.assertIsNone(get_persistent_error(factory.make_name('component'))) |
411 | - |
412 | - def test_get_persistent_error_returns_component_error(self): |
413 | - component = factory.make_name('component') |
414 | - error = factory.make_name('error') |
415 | - register_persistent_error(component, error) |
416 | - self.assertEqual(error, get_persistent_error(component)) |
417 | - |
418 | def test_register_persistent_error_reuses_component_errors(self): |
419 | """When registering a persistent error that already has an error |
420 | recorded for that component, reuse the error instead of deleting and |
421 | @@ -87,10 +74,12 @@ |
422 | error1 = factory.make_name('error') |
423 | error2 = factory.make_name('error') |
424 | register_persistent_error(component, error1) |
425 | - error = ComponentError.objects.get(component=component) |
426 | - self.assertEqual(error.error, error1) # Should be our error |
427 | - error_id = error.id |
428 | + notification = Notification.objects.get(ident=component) |
429 | + self.assertEqual(notification.render(), error1) |
430 | + notification_id = notification.id |
431 | register_persistent_error(component, error2) |
432 | - error = ComponentError.objects.get(component=component) |
433 | - self.assertEqual(error.error, error2) # Should update the message |
434 | - self.assertEqual(error.id, error_id) # Should reuse the same id |
435 | + notification = Notification.objects.get(ident=component) |
436 | + # The message is updated. |
437 | + self.assertEqual(notification.render(), error2) |
438 | + # The same notification row is used. |
439 | + self.assertEqual(notification.id, notification_id) |
440 | |
441 | === modified file 'src/maasserver/views/combo.py' |
442 | --- src/maasserver/views/combo.py 2017-02-01 22:18:19 +0000 |
443 | +++ src/maasserver/views/combo.py 2017-02-09 09:06:11 +0000 |
444 | @@ -39,6 +39,7 @@ |
445 | "angular.min.js", |
446 | "angular-route.min.js", |
447 | "angular-cookies.min.js", |
448 | + "angular-sanitize.min.js", |
449 | ] |
450 | }, |
451 | "ng-tags-input.js": { |
452 | |
453 | === modified file 'src/maasserver/views/tests/test_general.py' |
454 | --- src/maasserver/views/tests/test_general.py 2016-12-12 21:40:28 +0000 |
455 | +++ src/maasserver/views/tests/test_general.py 2017-02-09 09:06:11 +0000 |
456 | @@ -6,22 +6,17 @@ |
457 | __all__ = [] |
458 | |
459 | import http.client |
460 | -from random import randint |
461 | from urllib.parse import ( |
462 | parse_qs, |
463 | urlparse, |
464 | ) |
465 | -from xmlrpc.client import Fault |
466 | |
467 | from django.conf import settings |
468 | from django.conf.urls import patterns |
469 | from django.core.exceptions import PermissionDenied |
470 | -from django.core.urlresolvers import reverse |
471 | from django.http import Http404 |
472 | from django.test.client import RequestFactory |
473 | -from django.utils.html import escape |
474 | from lxml.html import fromstring |
475 | -from maasserver.components import register_persistent_error |
476 | from maasserver.testing import extract_redirect |
477 | from maasserver.testing.factory import factory |
478 | from maasserver.testing.testcase import MAASServerTestCase |
479 | @@ -29,7 +24,6 @@ |
480 | HelpfulDeleteView, |
481 | PaginatedListView, |
482 | ) |
483 | -from testtools.matchers import ContainsAll |
484 | |
485 | |
486 | class Test404500(MAASServerTestCase): |
487 | @@ -313,33 +307,3 @@ |
488 | "page": ["4"], |
489 | }, |
490 | parse_qs(urlparse(context["last_page_link"]).query)) |
491 | - |
492 | - |
493 | -class PermanentErrorDisplayTest(MAASServerTestCase): |
494 | - |
495 | - def test_permanent_error_displayed(self): |
496 | - self.client_log_in() |
497 | - fault_codes = [ |
498 | - randint(1, 100), |
499 | - randint(101, 200), |
500 | - ] |
501 | - errors = [] |
502 | - for fault in fault_codes: |
503 | - # Create component with make_string to be sure to display all |
504 | - # the errors. |
505 | - component = factory.make_name('component') |
506 | - error_message = factory.make_name('error') |
507 | - errors.append(Fault(fault, error_message)) |
508 | - register_persistent_error(component, error_message) |
509 | - links = [ |
510 | - reverse('index'), |
511 | - reverse('prefs'), |
512 | - ] |
513 | - for link in links: |
514 | - response = self.client.get(link) |
515 | - self.assertThat( |
516 | - response.content, |
517 | - ContainsAll([ |
518 | - escape(error.faultString).encode(settings.DEFAULT_CHARSET) |
519 | - for error in errors |
520 | - ])) |
521 | |
522 | === modified file 'src/maastesting/karma.conf.js' |
523 | --- src/maastesting/karma.conf.js 2015-03-26 17:55:20 +0000 |
524 | +++ src/maastesting/karma.conf.js 2017-02-09 09:06:11 +0000 |
525 | @@ -20,6 +20,7 @@ |
526 | '/usr/share/javascript/angular.js/angular-route.js', |
527 | '/usr/share/javascript/angular.js/angular-mocks.js', |
528 | '/usr/share/javascript/angular.js/angular-cookies.js', |
529 | + '/usr/share/javascript/angular.js/angular-sanitize.js', |
530 | '../../src/maasserver/static/js/angular/maas.js', |
531 | '../../src/maasserver/static/js/angular/testing/*.js', |
532 | '../../src/maasserver/static/js/angular/*/*.js', |
533 | |
534 | === modified file 'utilities/check-imports' |
535 | --- utilities/check-imports 2017-02-04 21:59:38 +0000 |
536 | +++ utilities/check-imports 2017-02-09 09:06:11 +0000 |
537 | @@ -245,6 +245,7 @@ |
538 | Allow("lxml|lxml.**"), |
539 | Allow("maascli.utils.parse_docstring"), |
540 | Allow("maasserver|maasserver.**"), |
541 | + Allow("markupsafe|markupsafe.**"), |
542 | Allow("metadataserver|metadataserver.**"), |
543 | Allow("netaddr|netaddr.**"), |
544 | Allow("oauth|oauth.**"), |
Looks good. Other than the one question I have. Which actually might allow you to remove that new dependency you added.
We need to do some work to get the notifications to show on the none angular pages as well, since this branch removes notifications from those pages. Its not to hard to enable.