Merge lp:~rvb/maas/cluster-own-ui4 into lp:~maas-committers/maas/trunk
- cluster-own-ui4
- Merge into trunk
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Raphaël Badin | ||||||||
Approved revision: | no longer in the source branch. | ||||||||
Merged at revision: | 2223 | ||||||||
Proposed branch: | lp:~rvb/maas/cluster-own-ui4 | ||||||||
Merge into: | lp:~maas-committers/maas/trunk | ||||||||
Diff against target: |
959 lines (+412/-205) 18 files modified
docs/man/maas-import-pxe-files.8.rst (+1/-1) man/maas-import-pxe-files.8 (+2/-2) src/maasserver/api.py (+1/-2) src/maasserver/templates/maasserver/base.html (+5/-0) src/maasserver/templates/maasserver/cluster_listing.html (+54/-58) src/maasserver/templates/maasserver/cluster_listing_head.html (+10/-0) src/maasserver/templates/maasserver/cluster_listing_row.html (+16/-3) src/maasserver/templates/maasserver/no-bootimages-warning.html (+1/-1) src/maasserver/templates/maasserver/nodegroup_confirm_delete.html (+1/-1) src/maasserver/templates/maasserver/nodegroup_edit.html (+1/-1) src/maasserver/templates/maasserver/settings.html (+0/-5) src/maasserver/tests/test_api.py (+3/-1) src/maasserver/urls.py (+22/-9) src/maasserver/views/clusters.py (+108/-3) src/maasserver/views/settings.py (+1/-38) src/maasserver/views/tests/test_boot_image_list.py (+1/-1) src/maasserver/views/tests/test_clusters.py (+185/-6) src/maasserver/views/tests/test_settings.py (+0/-73) |
||||||||
To merge this branch: | bzr merge lp:~rvb/maas/cluster-own-ui4 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Review via email: mp+213848@code.launchpad.net |
Commit message
This branch makes the cluster listings deal gracefully with a large number of clusters. Additionally, it makes the cluster listing pages more accessible.
In details:
- Move the cluster listings to their own url (instead of displaying them on the 'settings' page).
- Split the cluster listings into 3 separate listings (one page for each possible cluster status).
- Add a link to the cluster listing page next to 'Nodes' at the top of the page.
- Add pagination to all the listings.
Description of the change
There is a live canonistack instance available with the change from this branch available at http://
On the face if it, this branch appears to be huge but please consider that a lot of it is tests/code being simply moved.
Things that where brought up by people during the design phase and that I left for later (because I wanted to get the main change landed first):
- Add a notification in the 'Clusters' tab when a MAAS instance contains clusters that need attention.
- Add a warning for pending/rejected clusters that are linked to nodes.
Jeroen T. Vermeulen (jtv) wrote : | # |
Raphaël Badin (rvb) wrote : | # |
> Wonderful! And of course we've gone over the design in depth already.
Thanks a lot for the review!.
> [...]
>
> Also in get_view_title, line 401 of the diff:
>
> title_chunk = '<a href=%s>%s</a>' % (link, title_chunk)
>
> Shouldn't the value for the href attribute be quoted? This may be a
> consequence of the method being slightly too big for comfortable detailed
> testing.
That's correct, I'm splitting that method into smaller bit now.
> I'm not too comfortable about the bulk accept/reject buttons applying to all
> pending clusters... somebody could attempt to register a malicious cluster
> after the admin loaded the page, but before they hit the “accept all” button.
> Or conversely, though less harmfully I guess, a good cluster could present
> itself after the admin loaded the page but just before they hit the “reject
> all” button.
It's a bit dangerous indeed. This is not something this branch changed though. Let's fix this later.
> [...]
All your other remarks: done.
Thanks again.
Raphaël Badin (rvb) wrote : | # |
> This may be a consequence of the method being slightly too big for comfortable detailed testing.
All done now. I've added a new utility method which is unit-tested.
Jeroen T. Vermeulen (jtv) wrote : | # |
The change in the man page is a good catch! I just wrote that, and I hadn't made the connection. In general, thanks for the changes you've made. I know I'm being difficult as always. :) I'll make up for that by letting you have your way with the en-masse accept/reject actions.
In no-bootimages-
Could you give make_title_entry a quick docstring? Doesn't have to be much, just enough to place it in context.
In the ‘post’ method, maybe move the comments about what case each ‘if’ block handles, into the block? You'd get something like:
if 'mass_accept_
# Accept clusters en masse.
number = NodeGroup.
return HttpResponseRed
This way the if/elif/else structure comes out more clearly, because those statements are the only thing at their indentation level. It also means that the reader doesn't have to add a mental “if applicable” proviso to each comment. Once you're past the ‘if,’ the situation is clear and unambiguous.
Finally, I don't actually see the unit tests for make_title_entry. Missing commit? Not pushed? Diff not updated..?
Raphaël Badin (rvb) wrote : | # |
> The change in the man page is a good catch! I just wrote that, and I hadn't
> made the connection. In general, thanks for the changes you've made. I know
> I'm being difficult as always. :) I'll make up for that by letting you have
> your way with the en-masse accept/reject actions.
>
> In no-bootimages-
> — should that apostrophe be there?
Right, changed to "Cluster listing page."
> Could you give make_title_entry a quick docstring? Doesn't have to be much,
> just enough to place it in context.
Done.
> In the ‘post’ method, maybe move the comments about what case each ‘if’ block
> handles, into the block? You'd get something like:
>
> if 'mass_accept_
> # Accept clusters en masse.
> number = NodeGroup.
> messages.
> return HttpResponseRed
Good point, done.
> Finally, I don't actually see the unit tests for make_title_entry. Missing
> commit? Not pushed? Diff not updated..?
I guess I forgot to push the revision, done now.
Thanks again for the review.
Preview Diff
1 | === modified file 'docs/man/maas-import-pxe-files.8.rst' | |||
2 | --- docs/man/maas-import-pxe-files.8.rst 2014-04-01 07:59:47 +0000 | |||
3 | +++ docs/man/maas-import-pxe-files.8.rst 2014-04-03 11:20:25 +0000 | |||
4 | @@ -20,7 +20,7 @@ | |||
5 | 20 | An easier way to run the script is to trigger it from the MAAS web user | 20 | An easier way to run the script is to trigger it from the MAAS web user |
6 | 21 | interface. To do that, log in to your MAAS as an administrator using a | 21 | interface. To do that, log in to your MAAS as an administrator using a |
7 | 22 | web browser, click the cogwheel icon in the top right of the page to go | 22 | web browser, click the cogwheel icon in the top right of the page to go |
9 | 23 | to the Settings page, and click "Import boot images." This will start | 23 | to the Clusters page, and click "Import boot images." This will start |
10 | 24 | imports on all cluster controllers simultaneously. The same thing can | 24 | imports on all cluster controllers simultaneously. The same thing can |
11 | 25 | also be done through the region-controller API, or through the | 25 | also be done through the region-controller API, or through the |
12 | 26 | command-line interface. | 26 | command-line interface. |
13 | 27 | 27 | ||
14 | === modified file 'man/maas-import-pxe-files.8' | |||
15 | --- man/maas-import-pxe-files.8 2014-04-01 07:59:47 +0000 | |||
16 | +++ man/maas-import-pxe-files.8 2014-04-03 11:20:25 +0000 | |||
17 | @@ -1,6 +1,6 @@ | |||
18 | 1 | .\" Man page generated from reStructuredText. | 1 | .\" Man page generated from reStructuredText. |
19 | 2 | . | 2 | . |
21 | 3 | .TH "MAAS-IMPORT-PXE-FILES" "8" "April 01, 2014" "1.5" "MAAS" | 3 | .TH "MAAS-IMPORT-PXE-FILES" "8" "April 03, 2014" "1.5" "MAAS" |
22 | 4 | .SH NAME | 4 | .SH NAME |
23 | 5 | maas-import-pxe-files \- MAAS helper script | 5 | maas-import-pxe-files \- MAAS helper script |
24 | 6 | . | 6 | . |
25 | @@ -46,7 +46,7 @@ | |||
26 | 46 | An easier way to run the script is to trigger it from the MAAS web user | 46 | An easier way to run the script is to trigger it from the MAAS web user |
27 | 47 | interface. To do that, log in to your MAAS as an administrator using a | 47 | interface. To do that, log in to your MAAS as an administrator using a |
28 | 48 | web browser, click the cogwheel icon in the top right of the page to go | 48 | web browser, click the cogwheel icon in the top right of the page to go |
30 | 49 | to the Settings page, and click "Import boot images." This will start | 49 | to the Clusters page, and click "Import boot images." This will start |
31 | 50 | imports on all cluster controllers simultaneously. The same thing can | 50 | imports on all cluster controllers simultaneously. The same thing can |
32 | 51 | also be done through the region\-controller API, or through the | 51 | also be done through the region\-controller API, or through the |
33 | 52 | command\-line interface. | 52 | command\-line interface. |
34 | 53 | 53 | ||
35 | === modified file 'src/maasserver/api.py' | |||
36 | --- src/maasserver/api.py 2014-03-27 17:24:02 +0000 | |||
37 | +++ src/maasserver/api.py 2014-04-03 11:20:25 +0000 | |||
38 | @@ -2492,8 +2492,7 @@ | |||
39 | 2492 | nodegroups_without_images = nodegroups_without_images.filter( | 2492 | nodegroups_without_images = nodegroups_without_images.filter( |
40 | 2493 | status=NODEGROUP_STATUS.ACCEPTED) | 2493 | status=NODEGROUP_STATUS.ACCEPTED) |
41 | 2494 | if nodegroups_without_images.exists(): | 2494 | if nodegroups_without_images.exists(): |
44 | 2495 | accepted_clusters_url = ( | 2495 | accepted_clusters_url = absolute_reverse("cluster-list") |
43 | 2496 | "%s#accepted-clusters" % absolute_reverse("settings")) | ||
45 | 2497 | warning = dedent("""\ | 2496 | warning = dedent("""\ |
46 | 2498 | Some cluster controllers are missing boot images. Either the | 2497 | Some cluster controllers are missing boot images. Either the |
47 | 2499 | import task has not been initiated (for each cluster, the task | 2498 | import task has not been initiated (for each cluster, the task |
48 | 2500 | 2499 | ||
49 | === modified file 'src/maasserver/templates/maasserver/base.html' | |||
50 | --- src/maasserver/templates/maasserver/base.html 2014-02-25 09:57:24 +0000 | |||
51 | +++ src/maasserver/templates/maasserver/base.html 2014-04-03 11:20:25 +0000 | |||
52 | @@ -72,6 +72,11 @@ | |||
53 | 72 | <li class="{% block nav-active-node-list %}{% endblock %}"> | 72 | <li class="{% block nav-active-node-list %}{% endblock %}"> |
54 | 73 | <a href="{% url 'node-list' %}">Nodes</a> | 73 | <a href="{% url 'node-list' %}">Nodes</a> |
55 | 74 | </li> | 74 | </li> |
56 | 75 | {% if user.is_superuser %} | ||
57 | 76 | <li class="{% block nav-active-cluster-list %}{% endblock %}"> | ||
58 | 77 | <a href="{% url 'cluster-list' %}">Clusters</a> | ||
59 | 78 | </li> | ||
60 | 79 | {% endif %} | ||
61 | 75 | <li class="{% block nav-active-zone-list %}{% endblock %}"> | 80 | <li class="{% block nav-active-zone-list %}{% endblock %}"> |
62 | 76 | <a href="{% url 'zone-list' %}">Zones</a> | 81 | <a href="{% url 'zone-list' %}">Zones</a> |
63 | 77 | </li> | 82 | </li> |
64 | 78 | 83 | ||
65 | === renamed file 'src/maasserver/templates/maasserver/settings_cluster_listing.html' => 'src/maasserver/templates/maasserver/cluster_listing.html' | |||
66 | --- src/maasserver/templates/maasserver/settings_cluster_listing.html 2012-11-07 10:39:19 +0000 | |||
67 | +++ src/maasserver/templates/maasserver/cluster_listing.html 2014-04-03 11:20:25 +0000 | |||
68 | @@ -1,58 +1,54 @@ | |||
127 | 1 | <h2 id="accepted-clusters">Cluster controllers</h2> | 1 | {% extends "maasserver/base.html" %} |
128 | 2 | <table class="list" id="accepted-clusters-list"> | 2 | |
129 | 3 | <tbody> | 3 | {% block nav-active-cluster-list %}active{% endblock %} |
130 | 4 | {% for cluster in accepted_clusters %} | 4 | {% block title %}{{ status_name }} clusters{% endblock %} |
131 | 5 | {% cycle 'even' 'odd' as cycle silent %} | 5 | {% block page-title %}{{ current_count }}{% if input_query %} matching{% endif %} {{ status_name|lower }} cluster{{ current_count|pluralize }} in {% include "maasserver/site_title.html" %}{% endblock %} |
132 | 6 | {% include "maasserver/settings_cluster_listing_row.html" with cycle=cycle %} | 6 | {% block site-switcher %}{% endblock %} |
133 | 7 | {% endfor %} | 7 | {% block header-search %}{% endblock %} |
134 | 8 | </tbody> | 8 | |
135 | 9 | </table> | 9 | {% block content %} |
136 | 10 | <div> | 10 | <div id="clusters" style="position: relative;"> |
137 | 11 | <form method="POST" | 11 | |
138 | 12 | action="{% url 'settings' %}"> | 12 | <h2 id="clusters">{{ title }}</h2> |
139 | 13 | {% csrf_token %} | 13 | <table class="list" id="clusters-list"> |
140 | 14 | <input type="hidden" name="import_all_boot_images" value="1" /> | 14 | {% include "maasserver/cluster_listing_head.html" %} |
141 | 15 | <input type="submit" class="button right" value="Import boot images" /> | 15 | <tbody> |
142 | 16 | </form> | 16 | {% for cluster in cluster_list %} |
143 | 17 | </div> | 17 | {% cycle 'even' 'odd' as cycle silent %} |
144 | 18 | <div class="clear"></div> | 18 | {% include "maasserver/cluster_listing_row.html" with cycle=cycle warning_no_images=warn_no_images %} |
145 | 19 | {% if pending_clusters %} | 19 | {% endfor %} |
146 | 20 | <h3 id="pending-clusters">Pending clusters</h3> | 20 | </tbody> |
147 | 21 | <table class="list" id="pending-clusters-list"> | 21 | </table> |
148 | 22 | <tbody> | 22 | {% include "maasserver/pagination.html" %} |
149 | 23 | {% for cluster in pending_clusters %} | 23 | <div> |
150 | 24 | {% cycle 'even' 'odd' as cycle silent %} | 24 | {% if status == statuses.ACCEPTED %} |
151 | 25 | {% include "maasserver/settings_cluster_listing_row.html" with cycle=cycle %} | 25 | <form method="POST" |
152 | 26 | {% endfor %} | 26 | action="{% url 'cluster-list' %}"> |
153 | 27 | </tbody> | 27 | {% csrf_token %} |
154 | 28 | </table> | 28 | <input type="hidden" name="import_all_boot_images" value="1" /> |
155 | 29 | <div> | 29 | <input type="submit" class="button right" value="Import boot images" /> |
156 | 30 | <form id="reject_all_pending_nodegroups" | 30 | </form> |
157 | 31 | method="POST" | 31 | {% endif %} |
158 | 32 | action="{% url 'settings' %}"> | 32 | {% if status == statuses.PENDING %} |
159 | 33 | {% csrf_token %} | 33 | <form id="reject_all_pending_nodegroups" |
160 | 34 | <input type="hidden" name="mass_accept_submit" value="1" /> | 34 | method="POST" |
161 | 35 | <input type="submit" class="button right" value="Accept all" /> | 35 | action="{% url 'cluster-list' %}"> |
162 | 36 | </form> | 36 | {% csrf_token %} |
163 | 37 | <form id="accept_all_pending_nodegroups" | 37 | <input type="hidden" name="mass_accept_submit" value="1" /> |
164 | 38 | method="POST" | 38 | <input type="submit" class="button right" value="Accept all" /> |
165 | 39 | action="{% url 'settings' %}"> | 39 | </form> |
166 | 40 | {% csrf_token %} | 40 | <form id="accept_all_pending_nodegroups" |
167 | 41 | <input type="hidden" name="mass_reject_submit" value="1" /> | 41 | method="POST" |
168 | 42 | <input type="submit" class="button right space-right-small" | 42 | action="{% url 'cluster-list' %}"> |
169 | 43 | value="Reject all" /> | 43 | {% csrf_token %} |
170 | 44 | </form> | 44 | <input type="hidden" name="mass_reject_submit" value="1" /> |
171 | 45 | </div> | 45 | <input type="submit" class="button right space-right-small" |
172 | 46 | <div class="clear"></div> | 46 | value="Reject all" /> |
173 | 47 | {% endif %} | 47 | </form> |
174 | 48 | {% if rejected_clusters %} | 48 | {% endif %} |
175 | 49 | <h3 id="rejected-clusters">Rejected clusters</h3> | 49 | |
176 | 50 | <table class="list" id="rejected-clusters-list"> | 50 | </div> |
177 | 51 | <tbody> | 51 | <div class="clear"></div> |
178 | 52 | {% for cluster in rejected_clusters %} | 52 | |
179 | 53 | {% cycle 'even' 'odd' as cycle silent %} | 53 | |
180 | 54 | {% include "maasserver/settings_cluster_listing_row.html" with cycle=cycle %} | 54 | {% endblock %} |
123 | 55 | {% endfor %} | ||
124 | 56 | </tbody> | ||
125 | 57 | </table> | ||
126 | 58 | {% endif %} | ||
181 | 59 | \ No newline at end of file | 55 | \ No newline at end of file |
182 | 60 | 56 | ||
183 | === added file 'src/maasserver/templates/maasserver/cluster_listing_head.html' | |||
184 | --- src/maasserver/templates/maasserver/cluster_listing_head.html 1970-01-01 00:00:00 +0000 | |||
185 | +++ src/maasserver/templates/maasserver/cluster_listing_head.html 2014-04-03 11:20:25 +0000 | |||
186 | @@ -0,0 +1,10 @@ | |||
187 | 1 | <thead> | ||
188 | 2 | <tr> | ||
189 | 3 | <th>Name</th> | ||
190 | 4 | <th>Managed interfaces</th> | ||
191 | 5 | <th>Boot images</th> | ||
192 | 6 | <th>Nodes</th> | ||
193 | 7 | {% comment %} Action buttons {% endcomment %} | ||
194 | 8 | <th></th> | ||
195 | 9 | </tr> | ||
196 | 10 | </thead> | ||
197 | 0 | \ No newline at end of file | 11 | \ No newline at end of file |
198 | 1 | 12 | ||
199 | === renamed file 'src/maasserver/templates/maasserver/settings_cluster_listing_row.html' => 'src/maasserver/templates/maasserver/cluster_listing_row.html' | |||
200 | --- src/maasserver/templates/maasserver/settings_cluster_listing_row.html 2012-12-06 04:22:51 +0000 | |||
201 | +++ src/maasserver/templates/maasserver/cluster_listing_row.html 2014-04-03 11:20:25 +0000 | |||
202 | @@ -1,11 +1,24 @@ | |||
204 | 1 | <tr class="cluster {{ cycle }}" | 1 | <tr class="cluster {{ cycle }} {% if warning_no_images and cluster.bootimage_set.count == 0 %}warning{% endif %}" |
205 | 2 | id="{{ cluster.uuid }}"> | 2 | id="{{ cluster.uuid }}"> |
206 | 3 | <td> | 3 | <td> |
208 | 4 | {{ cluster.cluster_name }} | 4 | <a href="{% url 'cluster-edit' cluster.uuid %}">{{ cluster.cluster_name }}</a> |
209 | 5 | </td> | ||
210 | 6 | <td> | ||
211 | 7 | {{ cluster.get_managed_interfaces|length }} | ||
212 | 8 | </td> | ||
213 | 9 | <td> | ||
214 | 10 | {% if warning_no_images and cluster.bootimage_set.count == 0 %} | ||
215 | 11 | 0 <img src="{{ STATIC_URL }}img/warning.png" title="Warning: this cluster has no boot images."/> | ||
216 | 12 | {% else %} | ||
217 | 5 | {% if cluster.bootimage_set.count == 0 %} | 13 | {% if cluster.bootimage_set.count == 0 %} |
219 | 6 | <span class="warning">(Warning: this cluster has no boot images.)</span> | 14 | 0 |
220 | 15 | {% else %} | ||
221 | 16 | <a title="View boot images" | ||
222 | 17 | href="{% url 'cluster-bootimages-list' cluster.uuid %}">{{ cluster.bootimage_set.count }}</a> | ||
223 | 7 | {% endif %} | 18 | {% endif %} |
224 | 19 | {% endif %} | ||
225 | 8 | </td> | 20 | </td> |
226 | 21 | <td>{{ cluster.node_set.count }}</td> | ||
227 | 9 | <td class="icon-controls"> | 22 | <td class="icon-controls"> |
228 | 10 | <a href="{% url 'cluster-edit' cluster.uuid %}" | 23 | <a href="{% url 'cluster-edit' cluster.uuid %}" |
229 | 11 | class="icon" | 24 | class="icon" |
230 | 12 | 25 | ||
231 | === modified file 'src/maasserver/templates/maasserver/no-bootimages-warning.html' | |||
232 | --- src/maasserver/templates/maasserver/no-bootimages-warning.html 2014-03-27 10:51:28 +0000 | |||
233 | +++ src/maasserver/templates/maasserver/no-bootimages-warning.html 2014-04-03 11:20:25 +0000 | |||
234 | @@ -5,5 +5,5 @@ | |||
235 | 5 | <span class="console">/etc/maas/bootresources.yaml</span> on the cluster's | 5 | <span class="console">/etc/maas/bootresources.yaml</span> on the cluster's |
236 | 6 | machine) suits the configuration of the machines this cluster will have to | 6 | machine) suits the configuration of the machines this cluster will have to |
237 | 7 | handle and then run the import script by clicking the "Import boot images" | 7 | handle and then run the import script by clicking the "Import boot images" |
239 | 8 | button on the <a href="{% url 'settings' %}#clusters">cluster's listing page</a>. | 8 | button on the <a href="{% url 'cluster-list' %}">cluster listing page</a>. |
240 | 9 | </p> | 9 | </p> |
241 | 10 | 10 | ||
242 | === modified file 'src/maasserver/templates/maasserver/nodegroup_confirm_delete.html' | |||
243 | --- src/maasserver/templates/maasserver/nodegroup_confirm_delete.html 2012-10-08 12:50:32 +0000 | |||
244 | +++ src/maasserver/templates/maasserver/nodegroup_confirm_delete.html 2014-04-03 11:20:25 +0000 | |||
245 | @@ -17,7 +17,7 @@ | |||
246 | 17 | <input type="hidden" name="post" value="yes" /> | 17 | <input type="hidden" name="post" value="yes" /> |
247 | 18 | <input type="submit" value="Delete cluster controller" | 18 | <input type="submit" value="Delete cluster controller" |
248 | 19 | class="right" /> | 19 | class="right" /> |
250 | 20 | <a href="{% url 'settings' %}">Cancel</a> | 20 | <a href="{% url 'cluster-list' %}">Cancel</a> |
251 | 21 | </form> | 21 | </form> |
252 | 22 | </p> | 22 | </p> |
253 | 23 | </div> | 23 | </div> |
254 | 24 | 24 | ||
255 | === modified file 'src/maasserver/templates/maasserver/nodegroup_edit.html' | |||
256 | --- src/maasserver/templates/maasserver/nodegroup_edit.html 2014-03-25 10:35:14 +0000 | |||
257 | +++ src/maasserver/templates/maasserver/nodegroup_edit.html 2014-04-03 11:20:25 +0000 | |||
258 | @@ -15,7 +15,7 @@ | |||
259 | 15 | </ul> | 15 | </ul> |
260 | 16 | <input type="submit" value="Save cluster controller" | 16 | <input type="submit" value="Save cluster controller" |
261 | 17 | class="button right" /> | 17 | class="button right" /> |
263 | 18 | <a class="link-button" href="{% url 'settings' %}">Cancel</a> | 18 | <a class="link-button" href="{% url 'cluster-list' %}">Cancel</a> |
264 | 19 | <h2>Interfaces</h2> | 19 | <h2>Interfaces</h2> |
265 | 20 | {% with nb_interfaces=cluster.nodegroupinterface_set.count %} | 20 | {% with nb_interfaces=cluster.nodegroupinterface_set.count %} |
266 | 21 | This cluster controller has {{ nb_interfaces }} | 21 | This cluster controller has {{ nb_interfaces }} |
267 | 22 | 22 | ||
268 | === modified file 'src/maasserver/templates/maasserver/settings.html' | |||
269 | --- src/maasserver/templates/maasserver/settings.html 2012-11-30 17:21:28 +0000 | |||
270 | +++ src/maasserver/templates/maasserver/settings.html 2014-04-03 11:20:25 +0000 | |||
271 | @@ -70,11 +70,6 @@ | |||
272 | 70 | <div class="clear"></div> | 70 | <div class="clear"></div> |
273 | 71 | </div> | 71 | </div> |
274 | 72 | <div class="divider"></div> | 72 | <div class="divider"></div> |
275 | 73 | <div id="clusters" class="block size11 first"> | ||
276 | 74 | {% include "maasserver/settings_cluster_listing.html" %} | ||
277 | 75 | <div class="clear"></div> | ||
278 | 76 | </div> | ||
279 | 77 | <div class="divider"></div> | ||
280 | 78 | <div id="commissioning_scripts" class="block size11 first"> | 73 | <div id="commissioning_scripts" class="block size11 first"> |
281 | 79 | {% include "maasserver/settings_commissioning_scripts.html" %} | 74 | {% include "maasserver/settings_commissioning_scripts.html" %} |
282 | 80 | <div class="clear"></div> | 75 | <div class="clear"></div> |
283 | 81 | 76 | ||
284 | === modified file 'src/maasserver/tests/test_api.py' | |||
285 | --- src/maasserver/tests/test_api.py 2014-03-19 13:54:20 +0000 | |||
286 | +++ src/maasserver/tests/test_api.py 2014-04-03 11:20:25 +0000 | |||
287 | @@ -53,6 +53,7 @@ | |||
288 | 53 | from maasserver.testing.oauthclient import OAuthAuthenticatedClient | 53 | from maasserver.testing.oauthclient import OAuthAuthenticatedClient |
289 | 54 | from maasserver.testing.testcase import MAASServerTestCase | 54 | from maasserver.testing.testcase import MAASServerTestCase |
290 | 55 | from maasserver.tests.test_forms import make_interface_settings | 55 | from maasserver.tests.test_forms import make_interface_settings |
291 | 56 | from maasserver.utils import absolute_reverse | ||
292 | 56 | from maasserver.utils.orm import get_one | 57 | from maasserver.utils.orm import get_one |
293 | 57 | from maastesting.djangotestcase import TransactionTestCase | 58 | from maastesting.djangotestcase import TransactionTestCase |
294 | 58 | from maastesting.matchers import MockCalledOnceWith | 59 | from maastesting.matchers import MockCalledOnceWith |
295 | @@ -754,7 +755,8 @@ | |||
296 | 754 | [args[0][0] for args in recorder.call_args_list]) | 755 | [args[0][0] for args in recorder.call_args_list]) |
297 | 755 | # The persistent error message links to the clusters listing. | 756 | # The persistent error message links to the clusters listing. |
298 | 756 | self.assertIn( | 757 | self.assertIn( |
300 | 757 | "/settings/#accepted-clusters", recorder.call_args_list[0][0][1]) | 758 | absolute_reverse("cluster-list"), |
301 | 759 | recorder.call_args_list[0][0][1]) | ||
302 | 758 | 760 | ||
303 | 759 | def test_warns_if_any_nodegroup_has_no_images(self): | 761 | def test_warns_if_any_nodegroup_has_no_images(self): |
304 | 760 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED) | 762 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED) |
305 | 761 | 763 | ||
306 | === modified file 'src/maasserver/urls.py' | |||
307 | --- src/maasserver/urls.py 2014-03-25 10:35:14 +0000 | |||
308 | +++ src/maasserver/urls.py 2014-04-03 11:20:25 +0000 | |||
309 | @@ -21,12 +21,22 @@ | |||
310 | 21 | url, | 21 | url, |
311 | 22 | ) | 22 | ) |
312 | 23 | from django.contrib.auth.decorators import user_passes_test | 23 | from django.contrib.auth.decorators import user_passes_test |
313 | 24 | from maasserver.enum import NODEGROUP_STATUS | ||
314 | 24 | from maasserver.models import Node | 25 | from maasserver.models import Node |
315 | 25 | from maasserver.views import TextTemplateView | 26 | from maasserver.views import TextTemplateView |
316 | 26 | from maasserver.views.account import ( | 27 | from maasserver.views.account import ( |
317 | 27 | login, | 28 | login, |
318 | 28 | logout, | 29 | logout, |
319 | 29 | ) | 30 | ) |
320 | 31 | from maasserver.views.clusters import ( | ||
321 | 32 | BootImagesListView, | ||
322 | 33 | ClusterDelete, | ||
323 | 34 | ClusterEdit, | ||
324 | 35 | ClusterInterfaceCreate, | ||
325 | 36 | ClusterInterfaceDelete, | ||
326 | 37 | ClusterInterfaceEdit, | ||
327 | 38 | ClusterListView, | ||
328 | 39 | ) | ||
329 | 30 | from maasserver.views.networks import ( | 40 | from maasserver.views.networks import ( |
330 | 31 | NetworkAdd, | 41 | NetworkAdd, |
331 | 32 | NetworkDelete, | 42 | NetworkDelete, |
332 | @@ -60,14 +70,6 @@ | |||
333 | 60 | AccountsView, | 70 | AccountsView, |
334 | 61 | settings, | 71 | settings, |
335 | 62 | ) | 72 | ) |
336 | 63 | from maasserver.views.settings_clusters import ( | ||
337 | 64 | BootImagesListView, | ||
338 | 65 | ClusterDelete, | ||
339 | 66 | ClusterEdit, | ||
340 | 67 | ClusterInterfaceCreate, | ||
341 | 68 | ClusterInterfaceDelete, | ||
342 | 69 | ClusterInterfaceEdit, | ||
343 | 70 | ) | ||
344 | 71 | from maasserver.views.settings_commissioning_scripts import ( | 73 | from maasserver.views.settings_commissioning_scripts import ( |
345 | 72 | CommissioningScriptCreate, | 74 | CommissioningScriptCreate, |
346 | 73 | CommissioningScriptDelete, | 75 | CommissioningScriptDelete, |
347 | @@ -116,7 +118,6 @@ | |||
348 | 116 | r'^account/prefs/sshkey/delete/(?P<keyid>\d*)/$', | 118 | r'^account/prefs/sshkey/delete/(?P<keyid>\d*)/$', |
349 | 117 | SSHKeyDeleteView.as_view(), name='prefs-delete-sshkey'), | 119 | SSHKeyDeleteView.as_view(), name='prefs-delete-sshkey'), |
350 | 118 | ) | 120 | ) |
351 | 119 | |||
352 | 120 | # Logout view. | 121 | # Logout view. |
353 | 121 | urlpatterns += patterns( | 122 | urlpatterns += patterns( |
354 | 122 | 'maasserver.views', | 123 | 'maasserver.views', |
355 | @@ -159,6 +160,18 @@ | |||
356 | 159 | urlpatterns += patterns( | 160 | urlpatterns += patterns( |
357 | 160 | 'maasserver.views', | 161 | 'maasserver.views', |
358 | 161 | adminurl( | 162 | adminurl( |
359 | 163 | r'^clusters/$', | ||
360 | 164 | ClusterListView.as_view(status=NODEGROUP_STATUS.ACCEPTED), | ||
361 | 165 | name='cluster-list'), | ||
362 | 166 | adminurl( | ||
363 | 167 | r'^clusters/pending/$', | ||
364 | 168 | ClusterListView.as_view(status=NODEGROUP_STATUS.PENDING), | ||
365 | 169 | name='cluster-list-pending'), | ||
366 | 170 | adminurl( | ||
367 | 171 | r'^clusters/rejected/$', | ||
368 | 172 | ClusterListView.as_view(status=NODEGROUP_STATUS.REJECTED), | ||
369 | 173 | name='cluster-list-rejected'), | ||
370 | 174 | adminurl( | ||
371 | 162 | r'^clusters/(?P<uuid>[\w\-]+)/edit/$', ClusterEdit.as_view(), | 175 | r'^clusters/(?P<uuid>[\w\-]+)/edit/$', ClusterEdit.as_view(), |
372 | 163 | name='cluster-edit'), | 176 | name='cluster-edit'), |
373 | 164 | adminurl( | 177 | adminurl( |
374 | 165 | 178 | ||
375 | === renamed file 'src/maasserver/views/settings_clusters.py' => 'src/maasserver/views/clusters.py' | |||
376 | --- src/maasserver/views/settings_clusters.py 2014-03-25 10:35:14 +0000 | |||
377 | +++ src/maasserver/views/clusters.py 2014-04-03 11:20:25 +0000 | |||
378 | @@ -1,7 +1,7 @@ | |||
379 | 1 | # Copyright 2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
380 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
381 | 3 | 3 | ||
383 | 4 | """Cluster Settings views.""" | 4 | """Cluster views.""" |
384 | 5 | 5 | ||
385 | 6 | from __future__ import ( | 6 | from __future__ import ( |
386 | 7 | absolute_import, | 7 | absolute_import, |
387 | @@ -19,17 +19,29 @@ | |||
388 | 19 | "ClusterInterfaceCreate", | 19 | "ClusterInterfaceCreate", |
389 | 20 | "ClusterInterfaceDelete", | 20 | "ClusterInterfaceDelete", |
390 | 21 | "ClusterInterfaceEdit", | 21 | "ClusterInterfaceEdit", |
391 | 22 | "ClusterListView", | ||
392 | 22 | ] | 23 | ] |
393 | 23 | 24 | ||
394 | 25 | from collections import OrderedDict | ||
395 | 26 | |||
396 | 24 | from django.contrib import messages | 27 | from django.contrib import messages |
397 | 25 | from django.core.urlresolvers import reverse | 28 | from django.core.urlresolvers import reverse |
398 | 26 | from django.http import HttpResponseRedirect | 29 | from django.http import HttpResponseRedirect |
399 | 27 | from django.shortcuts import get_object_or_404 | 30 | from django.shortcuts import get_object_or_404 |
400 | 31 | from django.utils.safestring import mark_safe | ||
401 | 28 | from django.views.generic import ( | 32 | from django.views.generic import ( |
402 | 29 | CreateView, | 33 | CreateView, |
403 | 30 | DeleteView, | 34 | DeleteView, |
404 | 31 | UpdateView, | 35 | UpdateView, |
405 | 32 | ) | 36 | ) |
406 | 37 | from django.views.generic.edit import ( | ||
407 | 38 | FormMixin, | ||
408 | 39 | ProcessFormView, | ||
409 | 40 | ) | ||
410 | 41 | from maasserver.enum import ( | ||
411 | 42 | NODEGROUP_STATUS, | ||
412 | 43 | NODEGROUP_STATUS_CHOICES, | ||
413 | 44 | ) | ||
414 | 33 | from maasserver.forms import ( | 45 | from maasserver.forms import ( |
415 | 34 | NodeGroupEdit, | 46 | NodeGroupEdit, |
416 | 35 | NodeGroupInterfaceForm, | 47 | NodeGroupInterfaceForm, |
417 | @@ -41,6 +53,99 @@ | |||
418 | 41 | from maasserver.views import PaginatedListView | 53 | from maasserver.views import PaginatedListView |
419 | 42 | 54 | ||
420 | 43 | 55 | ||
421 | 56 | class ClusterListView(PaginatedListView, FormMixin, ProcessFormView): | ||
422 | 57 | template_name = 'maasserver/cluster_listing.html' | ||
423 | 58 | context_object_name = "cluster_list" | ||
424 | 59 | status = None | ||
425 | 60 | |||
426 | 61 | def get_queryset(self): | ||
427 | 62 | return NodeGroup.objects.filter( | ||
428 | 63 | status=self.status).order_by('cluster_name') | ||
429 | 64 | |||
430 | 65 | # A record of the urls used to reach the clusters of different | ||
431 | 66 | # statuses. | ||
432 | 67 | status_links = OrderedDict(( | ||
433 | 68 | (NODEGROUP_STATUS.ACCEPTED, 'cluster-list'), | ||
434 | 69 | (NODEGROUP_STATUS.PENDING, 'cluster-list-pending'), | ||
435 | 70 | (NODEGROUP_STATUS.REJECTED, 'cluster-list-rejected'), | ||
436 | 71 | )) | ||
437 | 72 | |||
438 | 73 | def make_title_entry(self, status, link_name): | ||
439 | 74 | """Generate an entry as used by make_cluster_listing_title(). | ||
440 | 75 | |||
441 | 76 | This is a utility method only used by make_cluster_listing_title. | ||
442 | 77 | It is a separate method for clarity and to help testing.""" | ||
443 | 78 | link = reverse(link_name) | ||
444 | 79 | status_name = NODEGROUP_STATUS_CHOICES[status][1] | ||
445 | 80 | nb_clusters = NodeGroup.objects.filter( | ||
446 | 81 | status=status).count() | ||
447 | 82 | entry = "%d %s cluster%s" % ( | ||
448 | 83 | nb_clusters, | ||
449 | 84 | status_name.lower(), | ||
450 | 85 | 's' if nb_clusters != 1 else '') | ||
451 | 86 | if nb_clusters != 0 and status != self.status: | ||
452 | 87 | entry = '<a href="%s">%s</a>' % (link, entry) | ||
453 | 88 | return entry | ||
454 | 89 | |||
455 | 90 | def make_cluster_listing_title(self): | ||
456 | 91 | """Generate this view's title with "tabs" for each cluster status. | ||
457 | 92 | |||
458 | 93 | Generate a title for this view with the number of clusters for each | ||
459 | 94 | possible status. The title includes the links to the other listings | ||
460 | 95 | (i.e. if this is the listing for the accepted clusters, include links | ||
461 | 96 | to the listings of the pending/rejected clusters). The title will be | ||
462 | 97 | of the form: "3 accepted clusters / 1 pending cluster / 2 rejected | ||
463 | 98 | clusters". (This is simpler to do in the view rather than write this | ||
464 | 99 | using the template language.) | ||
465 | 100 | """ | ||
466 | 101 | return mark_safe( | ||
467 | 102 | ' / '.join( | ||
468 | 103 | self.make_title_entry(status, link_name) | ||
469 | 104 | for status, link_name in self.status_links.items())) | ||
470 | 105 | |||
471 | 106 | def get_context_data(self, **kwargs): | ||
472 | 107 | context = super(ClusterListView, self).get_context_data(**kwargs) | ||
473 | 108 | context['current_count'] = NodeGroup.objects.filter( | ||
474 | 109 | status=self.status).count() | ||
475 | 110 | context['title'] = self.make_cluster_listing_title() | ||
476 | 111 | # Display warnings for clusters that have no images, but only for the | ||
477 | 112 | # display of 'accepted' clusters. | ||
478 | 113 | context['warn_no_images'] = self.status == NODEGROUP_STATUS.ACCEPTED | ||
479 | 114 | context['status'] = self.status | ||
480 | 115 | context['statuses'] = NODEGROUP_STATUS | ||
481 | 116 | context['status_name'] = NODEGROUP_STATUS_CHOICES[self.status][1] | ||
482 | 117 | return context | ||
483 | 118 | |||
484 | 119 | def post(self, request, *args, **kwargs): | ||
485 | 120 | """Handle a POST request.""" | ||
486 | 121 | if 'mass_accept_submit' in request.POST: | ||
487 | 122 | # Process accept clusters en masse. | ||
488 | 123 | number = NodeGroup.objects.accept_all_pending() | ||
489 | 124 | messages.info(request, "Accepted %d cluster(s)." % number) | ||
490 | 125 | return HttpResponseRedirect(reverse('cluster-list')) | ||
491 | 126 | |||
492 | 127 | elif 'mass_reject_submit' in request.POST: | ||
493 | 128 | # Process reject clusters en masse. | ||
494 | 129 | number = NodeGroup.objects.reject_all_pending() | ||
495 | 130 | messages.info(request, "Rejected %d cluster(s)." % number) | ||
496 | 131 | return HttpResponseRedirect(reverse('cluster-list')) | ||
497 | 132 | |||
498 | 133 | elif 'import_all_boot_images' in request.POST: | ||
499 | 134 | # Import PXE files for all the accepted clusters. | ||
500 | 135 | NodeGroup.objects.import_boot_images_accepted_clusters() | ||
501 | 136 | message = ( | ||
502 | 137 | "Import of boot images started on all cluster controllers. " | ||
503 | 138 | "Importing the boot images can take a long time depending on " | ||
504 | 139 | "the available bandwidth.") | ||
505 | 140 | messages.info(request, message) | ||
506 | 141 | return HttpResponseRedirect(reverse('cluster-list')) | ||
507 | 142 | |||
508 | 143 | else: | ||
509 | 144 | # Unknown action: redirect to the cluster listing page (this | ||
510 | 145 | # shouldn't happen). | ||
511 | 146 | return HttpResponseRedirect(reverse('cluster-list')) | ||
512 | 147 | |||
513 | 148 | |||
514 | 44 | class ClusterEdit(UpdateView): | 149 | class ClusterEdit(UpdateView): |
515 | 45 | model = NodeGroup | 150 | model = NodeGroup |
516 | 46 | template_name = 'maasserver/nodegroup_edit.html' | 151 | template_name = 'maasserver/nodegroup_edit.html' |
517 | @@ -54,7 +159,7 @@ | |||
518 | 54 | return context | 159 | return context |
519 | 55 | 160 | ||
520 | 56 | def get_success_url(self): | 161 | def get_success_url(self): |
522 | 57 | return reverse('settings') | 162 | return reverse('cluster-list') |
523 | 58 | 163 | ||
524 | 59 | def get_object(self): | 164 | def get_object(self): |
525 | 60 | uuid = self.kwargs.get('uuid', None) | 165 | uuid = self.kwargs.get('uuid', None) |
526 | @@ -75,7 +180,7 @@ | |||
527 | 75 | return get_object_or_404(NodeGroup, uuid=uuid) | 180 | return get_object_or_404(NodeGroup, uuid=uuid) |
528 | 76 | 181 | ||
529 | 77 | def get_next_url(self): | 182 | def get_next_url(self): |
531 | 78 | return reverse('settings') | 183 | return reverse('cluster-list') |
532 | 79 | 184 | ||
533 | 80 | def delete(self, request, *args, **kwargs): | 185 | def delete(self, request, *args, **kwargs): |
534 | 81 | cluster = self.get_object() | 186 | cluster = self.get_object() |
535 | 82 | 187 | ||
536 | === modified file 'src/maasserver/views/settings.py' | |||
537 | --- src/maasserver/views/settings.py 2013-10-07 09:12:40 +0000 | |||
538 | +++ src/maasserver/views/settings.py 2014-04-03 11:20:25 +0000 | |||
539 | @@ -38,7 +38,6 @@ | |||
540 | 38 | from django.views.generic.base import TemplateView | 38 | from django.views.generic.base import TemplateView |
541 | 39 | from django.views.generic.detail import SingleObjectTemplateResponseMixin | 39 | from django.views.generic.detail import SingleObjectTemplateResponseMixin |
542 | 40 | from django.views.generic.edit import ModelFormMixin | 40 | from django.views.generic.edit import ModelFormMixin |
543 | 41 | from maasserver.enum import NODEGROUP_STATUS | ||
544 | 42 | from maasserver.exceptions import CannotDeleteUserException | 41 | from maasserver.exceptions import CannotDeleteUserException |
545 | 43 | from maasserver.forms import ( | 42 | from maasserver.forms import ( |
546 | 44 | CommissioningForm, | 43 | CommissioningForm, |
547 | @@ -48,10 +47,7 @@ | |||
548 | 48 | NewUserCreationForm, | 47 | NewUserCreationForm, |
549 | 49 | UbuntuForm, | 48 | UbuntuForm, |
550 | 50 | ) | 49 | ) |
555 | 51 | from maasserver.models import ( | 50 | from maasserver.models import UserProfile |
552 | 52 | NodeGroup, | ||
553 | 53 | UserProfile, | ||
554 | 54 | ) | ||
556 | 55 | from maasserver.views import process_form | 51 | from maasserver.views import process_form |
557 | 56 | from metadataserver.models import CommissioningScript | 52 | from metadataserver.models import CommissioningScript |
558 | 57 | 53 | ||
559 | @@ -186,36 +182,6 @@ | |||
560 | 186 | if response is not None: | 182 | if response is not None: |
561 | 187 | return response | 183 | return response |
562 | 188 | 184 | ||
563 | 189 | # Process accept clusters en masse. | ||
564 | 190 | if 'mass_accept_submit' in request.POST: | ||
565 | 191 | number = NodeGroup.objects.accept_all_pending() | ||
566 | 192 | messages.info(request, "Accepted %d cluster(s)." % number) | ||
567 | 193 | return HttpResponseRedirect(reverse('settings')) | ||
568 | 194 | |||
569 | 195 | # Process reject clusters en masse. | ||
570 | 196 | if 'mass_reject_submit' in request.POST: | ||
571 | 197 | number = NodeGroup.objects.reject_all_pending() | ||
572 | 198 | messages.info(request, "Rejected %d cluster(s)." % number) | ||
573 | 199 | return HttpResponseRedirect(reverse('settings')) | ||
574 | 200 | |||
575 | 201 | # Import PXE files for all the accepted clusters. | ||
576 | 202 | if 'import_all_boot_images' in request.POST: | ||
577 | 203 | NodeGroup.objects.import_boot_images_accepted_clusters() | ||
578 | 204 | message = ( | ||
579 | 205 | "Import of boot images started on all cluster controllers. " | ||
580 | 206 | "Importing the boot images can take a long time depending on " | ||
581 | 207 | "the available bandwidth.") | ||
582 | 208 | messages.info(request, message) | ||
583 | 209 | return HttpResponseRedirect(reverse('settings')) | ||
584 | 210 | |||
585 | 211 | # Cluster listings. | ||
586 | 212 | accepted_clusters = NodeGroup.objects.filter( | ||
587 | 213 | status=NODEGROUP_STATUS.ACCEPTED).order_by('cluster_name') | ||
588 | 214 | pending_clusters = NodeGroup.objects.filter( | ||
589 | 215 | status=NODEGROUP_STATUS.PENDING).order_by('cluster_name') | ||
590 | 216 | rejected_clusters = NodeGroup.objects.filter( | ||
591 | 217 | status=NODEGROUP_STATUS.REJECTED).order_by('cluster_name') | ||
592 | 218 | |||
593 | 219 | # Commissioning scripts. | 185 | # Commissioning scripts. |
594 | 220 | commissioning_scripts = CommissioningScript.objects.all() | 186 | commissioning_scripts = CommissioningScript.objects.all() |
595 | 221 | 187 | ||
596 | @@ -224,9 +190,6 @@ | |||
597 | 224 | { | 190 | { |
598 | 225 | 'user_list': user_list, | 191 | 'user_list': user_list, |
599 | 226 | 'commissioning_scripts': commissioning_scripts, | 192 | 'commissioning_scripts': commissioning_scripts, |
600 | 227 | 'accepted_clusters': accepted_clusters, | ||
601 | 228 | 'pending_clusters': pending_clusters, | ||
602 | 229 | 'rejected_clusters': rejected_clusters, | ||
603 | 230 | 'maas_and_network_form': maas_and_network_form, | 193 | 'maas_and_network_form': maas_and_network_form, |
604 | 231 | 'commissioning_form': commissioning_form, | 194 | 'commissioning_form': commissioning_form, |
605 | 232 | 'ubuntu_form': ubuntu_form, | 195 | 'ubuntu_form': ubuntu_form, |
606 | 233 | 196 | ||
607 | === modified file 'src/maasserver/views/tests/test_boot_image_list.py' | |||
608 | --- src/maasserver/views/tests/test_boot_image_list.py 2014-03-27 07:39:38 +0000 | |||
609 | +++ src/maasserver/views/tests/test_boot_image_list.py 2014-04-03 11:20:25 +0000 | |||
610 | @@ -21,7 +21,7 @@ | |||
611 | 21 | from lxml.html import fromstring | 21 | from lxml.html import fromstring |
612 | 22 | from maasserver.testing.factory import factory | 22 | from maasserver.testing.factory import factory |
613 | 23 | from maasserver.testing.testcase import MAASServerTestCase | 23 | from maasserver.testing.testcase import MAASServerTestCase |
615 | 24 | from maasserver.views.settings_clusters import BootImagesListView | 24 | from maasserver.views.clusters import BootImagesListView |
616 | 25 | from testtools.matchers import ContainsAll | 25 | from testtools.matchers import ContainsAll |
617 | 26 | 26 | ||
618 | 27 | 27 | ||
619 | 28 | 28 | ||
620 | === renamed file 'src/maasserver/views/tests/test_settings_clusters.py' => 'src/maasserver/views/tests/test_clusters.py' | |||
621 | --- src/maasserver/views/tests/test_settings_clusters.py 2014-03-25 12:53:32 +0000 | |||
622 | +++ src/maasserver/views/tests/test_clusters.py 2014-04-03 11:20:25 +0000 | |||
623 | @@ -20,10 +20,12 @@ | |||
624 | 20 | from lxml.html import fromstring | 20 | from lxml.html import fromstring |
625 | 21 | from maasserver.enum import ( | 21 | from maasserver.enum import ( |
626 | 22 | NODEGROUP_STATUS, | 22 | NODEGROUP_STATUS, |
627 | 23 | NODEGROUP_STATUS_CHOICES, | ||
628 | 23 | NODEGROUPINTERFACE_MANAGEMENT, | 24 | NODEGROUPINTERFACE_MANAGEMENT, |
629 | 24 | ) | 25 | ) |
630 | 25 | from maasserver.models import ( | 26 | from maasserver.models import ( |
631 | 26 | NodeGroup, | 27 | NodeGroup, |
632 | 28 | nodegroup as nodegroup_module, | ||
633 | 27 | NodeGroupInterface, | 29 | NodeGroupInterface, |
634 | 28 | ) | 30 | ) |
635 | 29 | from maasserver.testing import ( | 31 | from maasserver.testing import ( |
636 | @@ -33,25 +35,42 @@ | |||
637 | 33 | ) | 35 | ) |
638 | 34 | from maasserver.testing.factory import factory | 36 | from maasserver.testing.factory import factory |
639 | 35 | from maasserver.testing.testcase import MAASServerTestCase | 37 | from maasserver.testing.testcase import MAASServerTestCase |
640 | 38 | from maasserver.utils import map_enum | ||
641 | 39 | from maasserver.views.clusters import ClusterListView | ||
642 | 40 | from mock import ( | ||
643 | 41 | ANY, | ||
644 | 42 | call, | ||
645 | 43 | ) | ||
646 | 36 | from testtools.matchers import ( | 44 | from testtools.matchers import ( |
647 | 37 | AllMatch, | 45 | AllMatch, |
648 | 38 | Contains, | 46 | Contains, |
649 | 39 | ContainsAll, | 47 | ContainsAll, |
650 | 40 | Equals, | 48 | Equals, |
651 | 49 | HasLength, | ||
652 | 41 | MatchesStructure, | 50 | MatchesStructure, |
653 | 42 | ) | 51 | ) |
654 | 43 | 52 | ||
655 | 44 | 53 | ||
656 | 45 | class ClusterListingTest(MAASServerTestCase): | 54 | class ClusterListingTest(MAASServerTestCase): |
657 | 46 | 55 | ||
659 | 47 | def test_settings_contains_links_to_edit_and_delete_clusters(self): | 56 | scenarios = [ |
660 | 57 | ('accepted-clusters', {'status': NODEGROUP_STATUS.ACCEPTED}), | ||
661 | 58 | ('pending-clusters', {'status': NODEGROUP_STATUS.PENDING}), | ||
662 | 59 | ('rejected-clusters', {'status': NODEGROUP_STATUS.REJECTED}), | ||
663 | 60 | ] | ||
664 | 61 | |||
665 | 62 | def get_url(self): | ||
666 | 63 | """Return the listing url used in this scenario.""" | ||
667 | 64 | return reverse(ClusterListView.status_links[ | ||
668 | 65 | self.status]) | ||
669 | 66 | |||
670 | 67 | def test_cluster_listing_contains_links_to_manipulate_clusters(self): | ||
671 | 48 | self.client_log_in(as_admin=True) | 68 | self.client_log_in(as_admin=True) |
672 | 49 | nodegroups = { | 69 | nodegroups = { |
676 | 50 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED), | 70 | factory.make_node_group(status=self.status) |
677 | 51 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | 71 | for _ in range(3) |
675 | 52 | factory.make_node_group(status=NODEGROUP_STATUS.REJECTED), | ||
678 | 53 | } | 72 | } |
680 | 54 | links = get_content_links(self.client.get(reverse('settings'))) | 73 | links = get_content_links(self.client.get(self.get_url())) |
681 | 55 | nodegroup_edit_links = [ | 74 | nodegroup_edit_links = [ |
682 | 56 | reverse('cluster-edit', args=[nodegroup.uuid]) | 75 | reverse('cluster-edit', args=[nodegroup.uuid]) |
683 | 57 | for nodegroup in nodegroups] | 76 | for nodegroup in nodegroups] |
684 | @@ -62,6 +81,166 @@ | |||
685 | 62 | links, | 81 | links, |
686 | 63 | ContainsAll(nodegroup_edit_links + nodegroup_delete_links)) | 82 | ContainsAll(nodegroup_edit_links + nodegroup_delete_links)) |
687 | 64 | 83 | ||
688 | 84 | def make_listing_view(self, status): | ||
689 | 85 | view = ClusterListView() | ||
690 | 86 | view.status = status | ||
691 | 87 | return view | ||
692 | 88 | |||
693 | 89 | def test_make_title_entry_returns_link_for_other_status(self): | ||
694 | 90 | # If the entry's status is different from the view's status, | ||
695 | 91 | # the returned entry is a link. | ||
696 | 92 | other_status = factory.getRandomChoice( | ||
697 | 93 | NODEGROUP_STATUS_CHOICES, but_not=[self.status]) | ||
698 | 94 | factory.make_node_group(status=other_status) | ||
699 | 95 | link_name = ClusterListView.status_links[other_status] | ||
700 | 96 | view = self.make_listing_view(self.status) | ||
701 | 97 | entry = view.make_title_entry(other_status, link_name) | ||
702 | 98 | status_name = NODEGROUP_STATUS_CHOICES[other_status][1] | ||
703 | 99 | self.assertEqual( | ||
704 | 100 | '<a href="%s">1 %s cluster</a>' % ( | ||
705 | 101 | reverse(link_name), status_name.lower()), | ||
706 | 102 | entry) | ||
707 | 103 | |||
708 | 104 | def test_make_title_entry_returns_title_if_no_cluster(self): | ||
709 | 105 | # If no cluster correspond to the entry's status, the returned | ||
710 | 106 | # entry is not a link: it's a simple mention '0 <status> clusters'. | ||
711 | 107 | other_status = factory.getRandomChoice( | ||
712 | 108 | NODEGROUP_STATUS_CHOICES, but_not=[self.status]) | ||
713 | 109 | link_name = ClusterListView.status_links[other_status] | ||
714 | 110 | view = self.make_listing_view(self.status) | ||
715 | 111 | entry = view.make_title_entry(other_status, link_name) | ||
716 | 112 | status_name = NODEGROUP_STATUS_CHOICES[other_status][1] | ||
717 | 113 | self.assertEqual( | ||
718 | 114 | '0 %s clusters' % status_name.lower(), entry) | ||
719 | 115 | |||
720 | 116 | def test_title_displays_number_of_clusters(self): | ||
721 | 117 | for _ in range(3): | ||
722 | 118 | factory.make_node_group(status=self.status) | ||
723 | 119 | view = self.make_listing_view(self.status) | ||
724 | 120 | status_name = NODEGROUP_STATUS_CHOICES[self.status][1] | ||
725 | 121 | title = view.make_cluster_listing_title() | ||
726 | 122 | self.assertIn("3 %s clusters" % status_name.lower(), title) | ||
727 | 123 | |||
728 | 124 | def test_title_contains_links_to_other_listings(self): | ||
729 | 125 | view = self.make_listing_view(self.status) | ||
730 | 126 | other_statuses = [] | ||
731 | 127 | # Compute a list with the statuses of the clusters not being | ||
732 | 128 | # displayed by the 'view'. Create clusters with these statuses. | ||
733 | 129 | for status in map_enum(NODEGROUP_STATUS).values(): | ||
734 | 130 | if status != self.status: | ||
735 | 131 | other_statuses.append(status) | ||
736 | 132 | factory.make_node_group(status=status) | ||
737 | 133 | for status in other_statuses: | ||
738 | 134 | link_name = ClusterListView.status_links[status] | ||
739 | 135 | title = view.make_cluster_listing_title() | ||
740 | 136 | self.assertIn(reverse(link_name), title) | ||
741 | 137 | |||
742 | 138 | def test_listing_is_paginated(self): | ||
743 | 139 | self.patch(ClusterListView, "paginate_by", 2) | ||
744 | 140 | self.client_log_in(as_admin=True) | ||
745 | 141 | for _ in range(3): | ||
746 | 142 | factory.make_node_group(status=self.status) | ||
747 | 143 | response = self.client.get(self.get_url()) | ||
748 | 144 | self.assertEqual(httplib.OK, response.status_code) | ||
749 | 145 | doc = fromstring(response.content) | ||
750 | 146 | self.assertThat( | ||
751 | 147 | doc.cssselect('div.pagination'), | ||
752 | 148 | HasLength(1), | ||
753 | 149 | "Couldn't find pagination tag.") | ||
754 | 150 | |||
755 | 151 | |||
756 | 152 | class ClusterListingAccess(MAASServerTestCase): | ||
757 | 153 | |||
758 | 154 | def test_admin_sees_cluster_tab(self): | ||
759 | 155 | self.client_log_in(as_admin=True) | ||
760 | 156 | links = get_content_links( | ||
761 | 157 | self.client.get(reverse('index')), element='#main-nav') | ||
762 | 158 | self.assertIn(reverse('cluster-list'), links) | ||
763 | 159 | |||
764 | 160 | def test_non_admin_doesnt_see_cluster_tab(self): | ||
765 | 161 | self.client_log_in(as_admin=False) | ||
766 | 162 | links = get_content_links( | ||
767 | 163 | self.client.get(reverse('index')), element='#main-nav') | ||
768 | 164 | self.assertNotIn(reverse('cluster-list'), links) | ||
769 | 165 | |||
770 | 166 | |||
771 | 167 | class ClusterPendingListingTest(MAASServerTestCase): | ||
772 | 168 | |||
773 | 169 | def test_pending_listing_contains_form_to_accept_all_nodegroups(self): | ||
774 | 170 | self.client_log_in(as_admin=True) | ||
775 | 171 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
776 | 172 | response = self.client.get(reverse('cluster-list-pending')) | ||
777 | 173 | doc = fromstring(response.content) | ||
778 | 174 | forms = doc.cssselect('form#accept_all_pending_nodegroups') | ||
779 | 175 | self.assertEqual(1, len(forms)) | ||
780 | 176 | |||
781 | 177 | def test_pending_listing_contains_form_to_reject_all_nodegroups(self): | ||
782 | 178 | self.client_log_in(as_admin=True) | ||
783 | 179 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
784 | 180 | response = self.client.get(reverse('cluster-list-pending')) | ||
785 | 181 | doc = fromstring(response.content) | ||
786 | 182 | forms = doc.cssselect('form#reject_all_pending_nodegroups') | ||
787 | 183 | self.assertEqual(1, len(forms)) | ||
788 | 184 | |||
789 | 185 | def test_pending_listing_accepts_all_pending_nodegroups_POST(self): | ||
790 | 186 | self.client_log_in(as_admin=True) | ||
791 | 187 | nodegroups = { | ||
792 | 188 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
793 | 189 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
794 | 190 | } | ||
795 | 191 | response = self.client.post( | ||
796 | 192 | reverse('cluster-list-pending'), {'mass_accept_submit': 1}) | ||
797 | 193 | self.assertEqual(httplib.FOUND, response.status_code) | ||
798 | 194 | self.assertEqual( | ||
799 | 195 | [reload_object(nodegroup).status for nodegroup in nodegroups], | ||
800 | 196 | [NODEGROUP_STATUS.ACCEPTED] * 2) | ||
801 | 197 | |||
802 | 198 | def test_pending_listing_rejects_all_pending_nodegroups_POST(self): | ||
803 | 199 | self.client_log_in(as_admin=True) | ||
804 | 200 | nodegroups = { | ||
805 | 201 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
806 | 202 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
807 | 203 | } | ||
808 | 204 | response = self.client.post( | ||
809 | 205 | reverse('cluster-list-pending'), {'mass_reject_submit': 1}) | ||
810 | 206 | self.assertEqual(httplib.FOUND, response.status_code) | ||
811 | 207 | self.assertEqual( | ||
812 | 208 | [reload_object(nodegroup).status for nodegroup in nodegroups], | ||
813 | 209 | [NODEGROUP_STATUS.REJECTED] * 2) | ||
814 | 210 | |||
815 | 211 | |||
816 | 212 | class ClusterAcceptedListingTest(MAASServerTestCase): | ||
817 | 213 | |||
818 | 214 | def test_accepted_listing_import_boot_images_calls_tasks(self): | ||
819 | 215 | self.client_log_in(as_admin=True) | ||
820 | 216 | recorder = self.patch(nodegroup_module, 'import_boot_images') | ||
821 | 217 | accepted_nodegroups = [ | ||
822 | 218 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED), | ||
823 | 219 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED), | ||
824 | 220 | ] | ||
825 | 221 | response = self.client.post( | ||
826 | 222 | reverse('cluster-list'), {'import_all_boot_images': 1}) | ||
827 | 223 | self.assertEqual(httplib.FOUND, response.status_code) | ||
828 | 224 | calls = [ | ||
829 | 225 | call(queue=nodegroup.work_queue, kwargs=ANY) | ||
830 | 226 | for nodegroup in accepted_nodegroups | ||
831 | 227 | ] | ||
832 | 228 | self.assertItemsEqual(calls, recorder.apply_async.call_args_list) | ||
833 | 229 | |||
834 | 230 | def test_a_warning_is_displayed_if_the_cluster_has_no_boot_images(self): | ||
835 | 231 | self.client_log_in(as_admin=True) | ||
836 | 232 | nodegroup = factory.make_node_group( | ||
837 | 233 | status=NODEGROUP_STATUS.ACCEPTED) | ||
838 | 234 | response = self.client.get(reverse('cluster-list')) | ||
839 | 235 | document = fromstring(response.content) | ||
840 | 236 | nodegroup_row = document.xpath("//tr[@id='%s']" % nodegroup.uuid)[0] | ||
841 | 237 | self.assertIn('warning', nodegroup_row.get('class')) | ||
842 | 238 | warning_elems = ( | ||
843 | 239 | nodegroup_row.xpath( | ||
844 | 240 | "//img[@title='Warning: this cluster has no boot images.']")) | ||
845 | 241 | self.assertEqual( | ||
846 | 242 | 1, len(warning_elems), "No warning about missing boot images.") | ||
847 | 243 | |||
848 | 65 | 244 | ||
849 | 66 | class ClusterDeleteTest(MAASServerTestCase): | 245 | class ClusterDeleteTest(MAASServerTestCase): |
850 | 67 | 246 | ||
851 | @@ -71,7 +250,7 @@ | |||
852 | 71 | delete_link = reverse('cluster-delete', args=[nodegroup.uuid]) | 250 | delete_link = reverse('cluster-delete', args=[nodegroup.uuid]) |
853 | 72 | response = self.client.post(delete_link, {'post': 'yes'}) | 251 | response = self.client.post(delete_link, {'post': 'yes'}) |
854 | 73 | self.assertEqual( | 252 | self.assertEqual( |
856 | 74 | (httplib.FOUND, reverse('settings')), | 253 | (httplib.FOUND, reverse('cluster-list')), |
857 | 75 | (response.status_code, extract_redirect(response))) | 254 | (response.status_code, extract_redirect(response))) |
858 | 76 | self.assertFalse( | 255 | self.assertFalse( |
859 | 77 | NodeGroup.objects.filter(uuid=nodegroup.uuid).exists()) | 256 | NodeGroup.objects.filter(uuid=nodegroup.uuid).exists()) |
860 | 78 | 257 | ||
861 | === modified file 'src/maasserver/views/tests/test_settings.py' | |||
862 | --- src/maasserver/views/tests/test_settings.py 2014-04-01 06:50:01 +0000 | |||
863 | +++ src/maasserver/views/tests/test_settings.py 2014-04-03 11:20:25 +0000 | |||
864 | @@ -23,11 +23,9 @@ | |||
865 | 23 | from maasserver.enum import ( | 23 | from maasserver.enum import ( |
866 | 24 | COMMISSIONING_DISTRO_SERIES_CHOICES, | 24 | COMMISSIONING_DISTRO_SERIES_CHOICES, |
867 | 25 | DISTRO_SERIES, | 25 | DISTRO_SERIES, |
868 | 26 | NODEGROUP_STATUS, | ||
869 | 27 | ) | 26 | ) |
870 | 28 | from maasserver.models import ( | 27 | from maasserver.models import ( |
871 | 29 | Config, | 28 | Config, |
872 | 30 | nodegroup as nodegroup_module, | ||
873 | 31 | UserProfile, | 29 | UserProfile, |
874 | 32 | ) | 30 | ) |
875 | 33 | from maasserver.testing import ( | 31 | from maasserver.testing import ( |
876 | @@ -37,10 +35,6 @@ | |||
877 | 37 | ) | 35 | ) |
878 | 38 | from maasserver.testing.factory import factory | 36 | from maasserver.testing.factory import factory |
879 | 39 | from maasserver.testing.testcase import MAASServerTestCase | 37 | from maasserver.testing.testcase import MAASServerTestCase |
880 | 40 | from mock import ( | ||
881 | 41 | ANY, | ||
882 | 42 | call, | ||
883 | 43 | ) | ||
884 | 44 | 38 | ||
885 | 45 | 39 | ||
886 | 46 | class SettingsTest(MAASServerTestCase): | 40 | class SettingsTest(MAASServerTestCase): |
887 | @@ -185,73 +179,6 @@ | |||
888 | 185 | new_kernel_opts, | 179 | new_kernel_opts, |
889 | 186 | Config.objects.get_config('kernel_opts')) | 180 | Config.objects.get_config('kernel_opts')) |
890 | 187 | 181 | ||
891 | 188 | def test_settings_contains_form_to_accept_all_nodegroups(self): | ||
892 | 189 | self.client_log_in(as_admin=True) | ||
893 | 190 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
894 | 191 | response = self.client.get(reverse('settings')) | ||
895 | 192 | doc = fromstring(response.content) | ||
896 | 193 | forms = doc.cssselect('form#accept_all_pending_nodegroups') | ||
897 | 194 | self.assertEqual(1, len(forms)) | ||
898 | 195 | |||
899 | 196 | def test_settings_contains_form_to_reject_all_nodegroups(self): | ||
900 | 197 | self.client_log_in(as_admin=True) | ||
901 | 198 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
902 | 199 | response = self.client.get(reverse('settings')) | ||
903 | 200 | doc = fromstring(response.content) | ||
904 | 201 | forms = doc.cssselect('form#reject_all_pending_nodegroups') | ||
905 | 202 | self.assertEqual(1, len(forms)) | ||
906 | 203 | |||
907 | 204 | def test_settings_accepts_all_pending_nodegroups_POST(self): | ||
908 | 205 | self.client_log_in(as_admin=True) | ||
909 | 206 | nodegroups = { | ||
910 | 207 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
911 | 208 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
912 | 209 | } | ||
913 | 210 | response = self.client.post( | ||
914 | 211 | reverse('settings'), {'mass_accept_submit': 1}) | ||
915 | 212 | self.assertEqual(httplib.FOUND, response.status_code) | ||
916 | 213 | self.assertEqual( | ||
917 | 214 | [reload_object(nodegroup).status for nodegroup in nodegroups], | ||
918 | 215 | [NODEGROUP_STATUS.ACCEPTED] * 2) | ||
919 | 216 | |||
920 | 217 | def test_settings_rejects_all_pending_nodegroups_POST(self): | ||
921 | 218 | self.client_log_in(as_admin=True) | ||
922 | 219 | nodegroups = { | ||
923 | 220 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
924 | 221 | factory.make_node_group(status=NODEGROUP_STATUS.PENDING), | ||
925 | 222 | } | ||
926 | 223 | response = self.client.post( | ||
927 | 224 | reverse('settings'), {'mass_reject_submit': 1}) | ||
928 | 225 | self.assertEqual(httplib.FOUND, response.status_code) | ||
929 | 226 | self.assertEqual( | ||
930 | 227 | [reload_object(nodegroup).status for nodegroup in nodegroups], | ||
931 | 228 | [NODEGROUP_STATUS.REJECTED] * 2) | ||
932 | 229 | |||
933 | 230 | def test_settings_import_boot_images_calls_tasks(self): | ||
934 | 231 | self.client_log_in(as_admin=True) | ||
935 | 232 | recorder = self.patch(nodegroup_module, 'import_boot_images') | ||
936 | 233 | accepted_nodegroups = [ | ||
937 | 234 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED), | ||
938 | 235 | factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED), | ||
939 | 236 | ] | ||
940 | 237 | response = self.client.post( | ||
941 | 238 | reverse('settings'), {'import_all_boot_images': 1}) | ||
942 | 239 | self.assertEqual(httplib.FOUND, response.status_code) | ||
943 | 240 | calls = [ | ||
944 | 241 | call(queue=nodegroup.work_queue, kwargs=ANY) | ||
945 | 242 | for nodegroup in accepted_nodegroups | ||
946 | 243 | ] | ||
947 | 244 | self.assertItemsEqual(calls, recorder.apply_async.call_args_list) | ||
948 | 245 | |||
949 | 246 | def test_cluster_no_boot_images_message_displayed_if_no_boot_images(self): | ||
950 | 247 | self.client_log_in(as_admin=True) | ||
951 | 248 | nodegroup = factory.make_node_group( | ||
952 | 249 | status=NODEGROUP_STATUS.ACCEPTED) | ||
953 | 250 | response = self.client.get(reverse('settings')) | ||
954 | 251 | document = fromstring(response.content) | ||
955 | 252 | nodegroup_row = document.xpath("//tr[@id='%s']" % nodegroup.uuid)[0] | ||
956 | 253 | self.assertIn('no boot images', nodegroup_row.text_content()) | ||
957 | 254 | |||
958 | 255 | 182 | ||
959 | 256 | class NonAdminSettingsTest(MAASServerTestCase): | 183 | class NonAdminSettingsTest(MAASServerTestCase): |
960 | 257 | 184 |
Wonderful! And of course we've gone over the design in depth already.
Unfortunately I don't have time for a full review right now; I'll just do what I can and hit the button when I run out of time.
.
A nitpick: why not make status_link an OrderedDict?
.
Might it help clarity to say in get_view_title's docstring that the title basically implements tabs?
.
Also in get_view_title, line 401 of the diff:
Shouldn't the value for the href attribute be quoted? This may be a consequence of the method being slightly too big for comfortable detailed testing.
.
It may be helpful, therefore, to extract get_view_title's loop body: “make me a title entry for this category of cluster.” Then get_view_title would just
return mark_safe(
self. make_title_ entry(status, link_name))
' / '.join(
for status, link_name in self.status_links)
(Of course if status_links were an OrderedDict, then the new function could just take a status argument and look up the link name for itself.)
.
In get_context_data, lines 410—411 of the diff:
# Do no display a warning for each cluster for which the images
# are missing unless this is the display of the 'accepted' clusters.
There's a typo in the “not,” but more importantly, this could be simpler without the double negation. How about "Display warnings for clusters that have no images, but only for the display of 'accepted' clusters”?
.
In ClusterListView .post, since each stanza is meant to return, I wonder if it might be clearer to structure the ‘if’ blocks as if/elif blocks. The final return could do with a comment.
.
I'm not too comfortable about the bulk accept/reject buttons applying to all pending clusters... somebody could attempt to register a malicious cluster after the admin loaded the page, but before they hit the “accept all” button. Or conversely, though less harmfully I guess, a good cluster could present itself after the admin loaded the page but just before they hit the “reject all” button.
.
The scenarios list for ClusterListingTest is pretty daunting... why not compute the ‘url’ value in a method? The definition in the scenarios list looks completely regular.
.
In test_listing_ is_paginated you use self.assertEqua ls(1, len(...)). I *finally* bothered to check: testtools has a HasLength matcher! Might make for nicer test output.