Merge lp:~newell-jensen/maas/bulk-node-actions-messages-bug-1387968 into lp:~maas-committers/maas/trunk

[Newell Jensen]

This branch gives the bulk node action message(s) the correct color for the given message(s).

Currently, some action messages should be yellow for warnings or red for errors, but instead are blue. This branch takes the highest severity (precedence) out of all the concatenated messages and uses this color. If we were going to make it so each message was going to have its own color instead of concatenating the messages as `message_from_form_stats` currently does, we would need to have specific node information (such as hostnames), to distinguish them, instead of just the number of nodes as how it is done now. If further distinguishing is desired, this branch could potentially be modified to do that.

[Blake Rouse (blake-rouse)]

No tests?

Also the commit message is a little long. I would give exactly what this does or fixes. Explaining the better way in the description.

review: Needs Fixing

[Newell Jensen (newell-jensen)]

No new separate test but I did update a test that was already there for the
functionality that I introduced.

On Mon, Nov 3, 2014 at 10:44 AM, Blake Rouse <email address hidden>
wrote:

> Review: Needs Fixing
>
> No tests?
>
> Also the commit message is a little long. I would give exactly what this
> does or fixes. Explaining the better way in the description.
> --
>
> https://code.launchpad.net/~newell-jensen/maas/bulk-node-actions-messages-bug-1387968/+merge/240217
> You are the owner of
> lp:~newell-jensen/maas/bulk-node-actions-messages-bug-1387968.
>

[Blake Rouse (blake-rouse)]

Okay. Looks good.

review: Approve

[Julian Edwards (julian-edwards)]

On Monday 03 November 2014 18:44:02 you wrote:
> Also the commit message is a little long.

There is no such thing as a commit message that is too long, we're not using
CVS. This format is fine:

"""
Short sentence describing overall change.

Longer sentence
possibly going over
multiple lines to
explain why this change
is needed.
"""

=== modified file 'src/maasserver/views/nodes.py'
--- src/maasserver/views/nodes.py	2014-10-24 23:45:33 +0000
+++ src/maasserver/views/nodes.py	2014-10-31 07:22:03 +0000
@@ -146,7 +146,7 @@
         (not_actionable, not_actionable_templates),
         (not_permitted, not_permitted_templates)]
     message = []
-    for number, message_templates in number_message:
+    for index, (number, message_templates) in enumerate(number_message):
         singular, plural = message_templates
         if number != 0:
             message_template = singular if number == 1 else plural
@@ -154,7 +154,8 @@
             # Override the action name so that only the first sentence will
             # contain the full name of the action.
             action_name = 'It'
-    return ' '.join(message)
+            level = index
+    return ' '.join(message), ('info', 'warning', 'error')[level]
 
 
 def prefetch_nodes_listing(nodes_query):
@@ -293,8 +294,8 @@
             action_class = SetZoneBulkAction
         else:
             action_class = ACTIONS_DICT[form.cleaned_data['action']]
-        message = message_from_form_stats(action_class, *stats)
-        messages.info(self.request, message)
+        message, level = message_from_form_stats(action_class, *stats)
+        getattr(messages, level)(self.request, message)
         return super(NodeListView, self).form_valid(form)
 
     def _compose_sort_order(self):

=== modified file 'src/maasserver/views/tests/test_nodes.py'
--- src/maasserver/views/tests/test_nodes.py	2014-10-29 04:34:42 +0000
+++ src/maasserver/views/tests/test_nodes.py	2014-10-31 07:22:03 +0000
@@ -1827,10 +1827,13 @@
                  % Delete.display_bulk,),
             ),
         ]
-        for params, snippets in params_and_snippets:
-            message = message_from_form_stats(*params)
+        # level precedence is worst-case for the concantenation of messages
+        levels = ['error', 'info', 'error', 'error']
+        for index, (params, snippets) in enumerate(params_and_snippets):
+            message, level = message_from_form_stats(*params)
             for snippet in snippets:
                 self.assertIn(snippet, message)
+            self.assertEqual(level, levels[index])
 
 
 class NodeEnlistmentPreseedViewTest(MAASServerTestCase):