Merge lp:~benji/charmworld/bundle-feature-ui into lp:~juju-jitsu/charmworld/trunk
- bundle-feature-ui
- Merge into trunk
Proposed by
Benji York
Status: | Merged |
---|---|
Approved by: | Benji York |
Approved revision: | 379 |
Merged at revision: | 381 |
Proposed branch: | lp:~benji/charmworld/bundle-feature-ui |
Merge into: | lp:~juju-jitsu/charmworld/trunk |
Diff against target: |
682 lines (+361/-183) 9 files modified
charmworld/routes.py (+3/-1) charmworld/search.py (+3/-3) charmworld/views/bundles.py (+4/-22) charmworld/views/charms.py (+1/-54) charmworld/views/featured.py (+81/-0) charmworld/views/helpers.py (+44/-2) charmworld/views/tests/test_auth.py (+2/-2) charmworld/views/tests/test_charms.py (+0/-99) charmworld/views/tests/test_featured.py (+223/-0) |
To merge this branch: | bzr merge lp:~benji/charmworld/bundle-feature-ui |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+183709@code.launchpad.net |
Commit message
Add a user interface for setting the featured flag for bundles.
Description of the change
Add a user interface for setting the featured flag for bundles.
To post a comment you must log in.
- 377. By Benji York
-
merge from trunk, fixing conflicts
- 378. By Benji York
-
fix failing test
- 379. By Benji York
-
make non-promulgated charms and bundles generate a 404 when the feature edit form is visited
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'charmworld/routes.py' |
2 | --- charmworld/routes.py 2013-09-03 19:57:54 +0000 |
3 | +++ charmworld/routes.py 2013-09-04 16:51:29 +0000 |
4 | @@ -83,9 +83,11 @@ |
5 | # Quality Assessment |
6 | config.add_route('charm-qa-edit', 'charms/{charm}/{series}/qa/edit') |
7 | |
8 | - # Featured charms |
9 | + # Featured charms and bundles. |
10 | config.add_route('charm-featured-edit', |
11 | 'charms/{series}/{charm}/featured/edit') |
12 | + config.add_route('bundle-featured-edit', |
13 | + 'bundles/{basket}/{bundle}/featured/edit') |
14 | |
15 | # Search |
16 | config.add_route("search", "/search") |
17 | |
18 | === modified file 'charmworld/search.py' |
19 | --- charmworld/search.py 2013-09-04 13:38:14 +0000 |
20 | +++ charmworld/search.py 2013-09-04 16:51:29 +0000 |
21 | @@ -135,9 +135,9 @@ |
22 | settings = get_ini() |
23 | self._client.create_index( |
24 | self.index_name, settings={ |
25 | - "index" : { |
26 | - "number_of_shards" : settings['es_shards'], |
27 | - "number_of_replicas" : settings['es_replicas'], |
28 | + "index": { |
29 | + "number_of_shards": settings['es_shards'], |
30 | + "number_of_replicas": settings['es_replicas'], |
31 | } |
32 | }) |
33 | # The ES server may need some time to actually create the index |
34 | |
35 | === modified file 'charmworld/views/bundles.py' |
36 | --- charmworld/views/bundles.py 2013-08-29 13:55:25 +0000 |
37 | +++ charmworld/views/bundles.py 2013-09-04 16:51:29 +0000 |
38 | @@ -4,7 +4,10 @@ |
39 | from charmworld import cached_view_config |
40 | from charmworld.models import Bundle |
41 | from charmworld.views.api import json_response |
42 | -from charmworld.views.helpers import found |
43 | +from charmworld.views.helpers import ( |
44 | + find_bundle, |
45 | + found, |
46 | +) |
47 | |
48 | |
49 | class BundleDetail(Bundle): |
50 | @@ -63,27 +66,6 @@ |
51 | raise |
52 | |
53 | |
54 | -def find_bundle(request, promulgated=False): |
55 | - """Find the bundle. |
56 | - |
57 | - May return None, which must be handled by the caller. |
58 | - """ |
59 | - spec = dict( |
60 | - name=request.matchdict['bundle'], |
61 | - ) |
62 | - basket = request.matchdict.get('basket') |
63 | - rev = request.matchdict.get('rev') |
64 | - if basket is not None: |
65 | - spec['basket_name'] = basket |
66 | - if rev is not None: |
67 | - spec['basket_revision'] = int(rev) |
68 | - if promulgated: |
69 | - spec["promulgated"] = True |
70 | - else: |
71 | - spec["owner"] = request.matchdict['owner'] |
72 | - return Bundle.from_query(spec, request.db) |
73 | - |
74 | - |
75 | def _bundle_view(request, bundle): |
76 | bundle = BundleDetail(dict(bundle)) |
77 | try: |
78 | |
79 | === modified file 'charmworld/views/charms.py' |
80 | --- charmworld/views/charms.py 2013-08-30 21:03:41 +0000 |
81 | +++ charmworld/views/charms.py 2013-09-04 16:51:29 +0000 |
82 | @@ -8,13 +8,11 @@ |
83 | import json |
84 | import os |
85 | import pymongo |
86 | -from pyramid.httpexceptions import HTTPFound |
87 | from pyramid.view import view_config |
88 | from webob import Response |
89 | |
90 | from charmworld import cached_view_config |
91 | from charmworld.forms.qa_assessment import get_qa_form_schema |
92 | -from charmworld.forms.featured import CharmFeaturedness |
93 | from charmworld.models import ( |
94 | Charm, |
95 | CharmFileSet, |
96 | @@ -30,6 +28,7 @@ |
97 | ) |
98 | from charmworld.views.api import API3 |
99 | from charmworld.views.helpers import ( |
100 | + find_charm, |
101 | format_change, |
102 | format_proof, |
103 | found, |
104 | @@ -278,23 +277,6 @@ |
105 | return found_charm_collection(request.db, query, sub_only) |
106 | |
107 | |
108 | -def find_charm(request, promulgated=False): |
109 | - spec = { |
110 | - "name": request.matchdict['charm'], |
111 | - "series": request.matchdict['series'], |
112 | - } |
113 | - if promulgated: |
114 | - spec["promulgated"] = True |
115 | - else: |
116 | - spec["owner"] = request.matchdict['owner'] |
117 | - charm_data = found(request.db.charms.find_one( |
118 | - spec, sort=[('store_data.revision', pymongo.DESCENDING)])) |
119 | - # We are phasing out is_featured from the charm data, so remove it if this |
120 | - # is an old charm record that still has it. |
121 | - charm_data.pop('is_featured', None) |
122 | - return charm_data |
123 | - |
124 | - |
125 | @cached_view_config( |
126 | route_name="personal-charm", |
127 | renderer="charmworld:templates/charm.pt") |
128 | @@ -437,38 +419,3 @@ |
129 | 'form': form, |
130 | "values": valid_data, |
131 | } |
132 | - |
133 | - |
134 | -@view_config( |
135 | - route_name="charm-featured-edit", |
136 | - renderer='charmworld:templates/qa_edit.pt', |
137 | - permission='edit', |
138 | -) |
139 | -def featured_edit(request): |
140 | - charm = find_charm(request, promulgated=True) |
141 | - form = Form(CharmFeaturedness(), buttons=('submit',)) |
142 | - form.css_class = 'form-horizontal well' |
143 | - featured_source = FeaturedSource.from_db(request.db) |
144 | - |
145 | - if 'submit' in request.POST: |
146 | - try: |
147 | - data = request.POST.items() |
148 | - valid_data = form.validate(data) |
149 | - if valid_data['is_featured']: |
150 | - featured_source.set_featured(charm, 'charm') |
151 | - else: |
152 | - featured_source.clear_featured(charm, 'charm') |
153 | - series = request.matchdict['series'] |
154 | - name = request.matchdict['charm'] |
155 | - url = request.route_url('charm', series=series, charm=name) |
156 | - return HTTPFound(url) |
157 | - except ValidationFailure as form: |
158 | - valid_data = {} |
159 | - else: |
160 | - valid_data = { |
161 | - 'is_featured': featured_source.is_featured(charm, 'charm')} |
162 | - |
163 | - return { |
164 | - 'form': form.render(valid_data), |
165 | - 'values': valid_data, |
166 | - } |
167 | |
168 | === added file 'charmworld/views/featured.py' |
169 | --- charmworld/views/featured.py 1970-01-01 00:00:00 +0000 |
170 | +++ charmworld/views/featured.py 2013-09-04 16:51:29 +0000 |
171 | @@ -0,0 +1,81 @@ |
172 | +# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
173 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
174 | + |
175 | +from deform import ( |
176 | + Form, |
177 | + ValidationFailure, |
178 | +) |
179 | +from pyramid.httpexceptions import HTTPFound |
180 | +from pyramid.view import view_config |
181 | + |
182 | +from charmworld.forms.featured import CharmFeaturedness |
183 | +from charmworld.models import FeaturedSource |
184 | +from charmworld.views.helpers import ( |
185 | + find_charm, |
186 | + find_bundle, |
187 | + found, |
188 | +) |
189 | + |
190 | + |
191 | +def featured_edit(request, item, kind, url): |
192 | + """Back end for featuring an "item" which can be a charm or bundle.""" |
193 | + form = Form(CharmFeaturedness(), buttons=('submit',)) |
194 | + form.css_class = 'form-horizontal well' |
195 | + featured_source = FeaturedSource.from_db(request.db) |
196 | + |
197 | + if 'submit' in request.POST: |
198 | + try: |
199 | + data = request.POST.items() |
200 | + valid_data = form.validate(data) |
201 | + if valid_data['is_featured']: |
202 | + featured_source.set_featured(item, kind) |
203 | + else: |
204 | + featured_source.clear_featured(item, kind) |
205 | + # Send the user back to the item's main page after saving. |
206 | + return HTTPFound(url) |
207 | + except ValidationFailure as form: |
208 | + valid_data = {} |
209 | + else: |
210 | + valid_data = { |
211 | + 'is_featured': featured_source.is_featured(item, kind)} |
212 | + |
213 | + return { |
214 | + 'form': form.render(valid_data), |
215 | + 'values': valid_data, |
216 | + } |
217 | + |
218 | + |
219 | +@view_config( |
220 | + route_name="charm-featured-edit", |
221 | + renderer='charmworld:templates/qa_edit.pt', |
222 | + permission='edit', |
223 | +) |
224 | +def charm_featured_edit(request): |
225 | + """Back end for featuring a charm.""" |
226 | + charm = find_charm(request, promulgated=True) |
227 | + found(charm) |
228 | + series = request.matchdict['series'] |
229 | + charm_name = request.matchdict['charm'] |
230 | + url = request.route_url('charm', series=series, charm=charm_name) |
231 | + return featured_edit(request, charm, 'charm', url) |
232 | + |
233 | + |
234 | +@view_config( |
235 | + route_name="bundle-featured-edit", |
236 | + renderer='charmworld:templates/qa_edit.pt', |
237 | + permission='edit', |
238 | +) |
239 | +def bundle_featured_edit(request): |
240 | + """Back end for featuring a bundle.""" |
241 | + bundle = find_bundle(request, promulgated=True) |
242 | + # Assert that the bundle was found, generating a 404 if not. |
243 | + found(bundle) |
244 | + basket = request.matchdict.get('basket') |
245 | + basket_name = request.matchdict.get('bundle') |
246 | + # Since there is not yet a web page for promulgated bundles, we redirect |
247 | + # to the owner's page for the bundle. |
248 | + # TODO Change this to the promulgated bundle page when they exist. |
249 | + url = request.route_url( |
250 | + 'personal-bundle', owner=bundle.owner, basket=basket, |
251 | + bundle=basket_name) |
252 | + return featured_edit(request, dict(bundle), 'bundle', url) |
253 | |
254 | === modified file 'charmworld/views/helpers.py' |
255 | --- charmworld/views/helpers.py 2013-07-31 14:45:14 +0000 |
256 | +++ charmworld/views/helpers.py 2013-09-04 16:51:29 +0000 |
257 | @@ -3,13 +3,17 @@ |
258 | |
259 | from datetime import datetime |
260 | from pyramid.exceptions import NotFound |
261 | +import pymongo |
262 | |
263 | -from charmworld.models import Charm |
264 | +from charmworld.models import ( |
265 | + Bundle, |
266 | + Charm, |
267 | +) |
268 | from charmworld.models import find_charms |
269 | |
270 | - |
271 | # View helpers used throughout various views |
272 | |
273 | + |
274 | def format_change(change, charm=None): |
275 | """Format a bzr commit.""" |
276 | parts = [] |
277 | @@ -131,3 +135,41 @@ |
278 | names.append(c.name) |
279 | results.sort(lambda x, y: cmp(x.name, y.name)) |
280 | return results |
281 | + |
282 | + |
283 | +def find_charm(request, promulgated=False): |
284 | + spec = { |
285 | + "name": request.matchdict['charm'], |
286 | + "series": request.matchdict['series'], |
287 | + } |
288 | + if promulgated: |
289 | + spec["promulgated"] = True |
290 | + else: |
291 | + spec["owner"] = request.matchdict['owner'] |
292 | + charm_data = found(request.db.charms.find_one( |
293 | + spec, sort=[('store_data.revision', pymongo.DESCENDING)])) |
294 | + # We are phasing out is_featured from the charm data, so remove it if this |
295 | + # is an old charm record that still has it. |
296 | + charm_data.pop('is_featured', None) |
297 | + return charm_data |
298 | + |
299 | + |
300 | +def find_bundle(request, promulgated=False): |
301 | + """Find the bundle. |
302 | + |
303 | + May return None, which must be handled by the caller. |
304 | + """ |
305 | + spec = dict( |
306 | + name=request.matchdict['bundle'], |
307 | + ) |
308 | + basket = request.matchdict.get('basket') |
309 | + rev = request.matchdict.get('rev') |
310 | + if basket is not None: |
311 | + spec['basket_name'] = basket |
312 | + if rev is not None: |
313 | + spec['basket_revision'] = int(rev) |
314 | + if promulgated: |
315 | + spec["promulgated"] = True |
316 | + else: |
317 | + spec["owner"] = request.matchdict['owner'] |
318 | + return Bundle.from_query(spec, request.db) |
319 | |
320 | === modified file 'charmworld/views/tests/test_auth.py' |
321 | --- charmworld/views/tests/test_auth.py 2013-09-04 12:40:00 +0000 |
322 | +++ charmworld/views/tests/test_auth.py 2013-09-04 16:51:29 +0000 |
323 | @@ -56,8 +56,8 @@ |
324 | |
325 | def test_login_route(self): |
326 | path = '/login/openid' |
327 | - response = self.app.get(path, status=200) |
328 | + self.app.get(path, status=200) |
329 | |
330 | def test_logout_route(self): |
331 | path = '/logout' |
332 | - response = self.app.get(path, status=302) |
333 | + self.app.get(path, status=302) |
334 | |
335 | === modified file 'charmworld/views/tests/test_charms.py' |
336 | --- charmworld/views/tests/test_charms.py 2013-08-30 21:03:41 +0000 |
337 | +++ charmworld/views/tests/test_charms.py 2013-09-04 16:51:29 +0000 |
338 | @@ -10,7 +10,6 @@ |
339 | from charmworld.models import ( |
340 | Charm, |
341 | CharmSource, |
342 | - FeaturedSource, |
343 | QADataSource, |
344 | getfs, |
345 | ) |
346 | @@ -37,7 +36,6 @@ |
347 | found_charm_collection, |
348 | hook, |
349 | interface, |
350 | - featured_edit, |
351 | personal_collection, |
352 | personal_config, |
353 | personal_hook, |
354 | @@ -746,100 +744,3 @@ |
355 | charm['series']), |
356 | status=200) |
357 | self.assertEqual(resp.status_code, 200) |
358 | - |
359 | - |
360 | -class TestFeaturedForm(ViewTestBase): |
361 | - |
362 | - def test_form_renders(self): |
363 | - """Sanity check that the form renders without errors.""" |
364 | - cid, charm = factory.makeCharm(self.db, promulgated=True, owner='foo') |
365 | - request = self.getRequest() |
366 | - request.matchdict = { |
367 | - 'charm': charm['name'], |
368 | - 'series': charm['series'], |
369 | - } |
370 | - response = featured_edit(request) |
371 | - |
372 | - self.assertIn('form', response) |
373 | - self.assertIn('<label', response['form']) |
374 | - |
375 | - def test_checking_checkbox_sets_featured_status(self): |
376 | - """Verify checking the checkbox causes the charm to be featured.""" |
377 | - charm = factory.get_charm_json(name='name1', promulgated=True) |
378 | - self.use_index_client() |
379 | - self.enable_routes() |
380 | - charm_source = CharmSource.from_request(self) |
381 | - charm_source.save(charm) |
382 | - # Add the submit button to trigger the right path through the view. |
383 | - post_data = { |
384 | - '_charset_': 'UTF-8', |
385 | - '__formid__': 'deform', |
386 | - 'submit': 'submit', |
387 | - 'is_featured': 'true', |
388 | - } |
389 | - request = self.getRequest(post=post_data) |
390 | - request.matchdict = { |
391 | - 'charm': charm['name'], |
392 | - 'series': charm['series'], |
393 | - } |
394 | - # Before processing the request, the charm is not featured. |
395 | - featured_source = FeaturedSource.from_db(self.db) |
396 | - self.assertFalse(featured_source.is_featured(charm, 'charm')) |
397 | - response = featured_edit(request) |
398 | - self.assertEqual(302, response.status_code) |
399 | - self.assertRegexpMatches(response.headers['Location'], |
400 | - 'http://[^/]*/charms/precise/name1') |
401 | - for found in charm_source._get_all(charm['_id']): |
402 | - self.assertTrue(featured_source.is_featured(found, 'charm')) |
403 | - |
404 | - def test_unchecking_checkbox_clears_featured_status(self): |
405 | - """Verify unchecking the checkbox causes the charm to be unfeatured.""" |
406 | - charm = factory.get_charm_json(name='name1', promulgated=True) |
407 | - self.use_index_client() |
408 | - self.enable_routes() |
409 | - charm_source = CharmSource.from_request(self) |
410 | - charm_source.save(charm) |
411 | - # Add the submit button to trigger the right path through the view. |
412 | - post_data = { |
413 | - '_charset_': 'UTF-8', |
414 | - '__formid__': 'deform', |
415 | - 'submit': 'submit', |
416 | - } |
417 | - request = self.getRequest(post=post_data) |
418 | - request.matchdict = { |
419 | - 'charm': charm['name'], |
420 | - 'series': charm['series'], |
421 | - } |
422 | - # Before processing the request, the charm is featured. |
423 | - featured_source = FeaturedSource.from_db(self.db) |
424 | - featured_source.set_featured(charm, 'charm') |
425 | - self.assertTrue(featured_source.is_featured(charm, 'charm')) |
426 | - response = featured_edit(request) |
427 | - self.assertEqual(302, response.status_code) |
428 | - self.assertRegexpMatches(response.headers['Location'], |
429 | - 'http://[^/]*/charms/precise/name1') |
430 | - for found in charm_source._get_all(charm['_id']): |
431 | - self.assertFalse(featured_source.is_featured(found, 'charm')) |
432 | - |
433 | - |
434 | -class TestFeaturedFormFunc(WebTestBase): |
435 | - |
436 | - def test_form_403s_for_non_charmers(self): |
437 | - """The form is not permitted if you're not a charmer.""" |
438 | - with login_request(self.app, groups=[]) as resp: |
439 | - cid, charm = factory.makeCharm(self.db) |
440 | - # The status force the check against the status code. |
441 | - resp = resp.goto( |
442 | - 'charms/%s/%s/featured/edit' % |
443 | - (charm['series'], charm['name']), status=403) |
444 | - self.assertEqual(403, resp.status_code) |
445 | - |
446 | - def test_form_secured_from_users(self): |
447 | - """The form is only allowed to be edited/submitted by charmers.""" |
448 | - with login_request(self.app, groups=['charmers']) as resp: |
449 | - cid, charm = factory.makeCharm(self.db, promulgated=True) |
450 | - resp = resp.goto( |
451 | - 'charms/%s/%s/featured/edit' % |
452 | - (charm['series'], charm['name']), |
453 | - status=200) |
454 | - self.assertEqual(resp.status_code, 200) |
455 | |
456 | === added file 'charmworld/views/tests/test_featured.py' |
457 | --- charmworld/views/tests/test_featured.py 1970-01-01 00:00:00 +0000 |
458 | +++ charmworld/views/tests/test_featured.py 2013-09-04 16:51:29 +0000 |
459 | @@ -0,0 +1,223 @@ |
460 | +# Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
461 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
462 | + |
463 | +from charmworld.models import ( |
464 | + CharmSource, |
465 | + FeaturedSource, |
466 | +) |
467 | +from charmworld.testing import ( |
468 | + factory, |
469 | + login_request, |
470 | + ViewTestBase, |
471 | + WebTestBase, |
472 | +) |
473 | +from charmworld.views.featured import ( |
474 | + bundle_featured_edit, |
475 | + charm_featured_edit, |
476 | + featured_edit, |
477 | +) |
478 | + |
479 | + |
480 | +class TestFeaturedForm(ViewTestBase): |
481 | + |
482 | + def test_form_renders(self): |
483 | + """Sanity check that the form renders without errors.""" |
484 | + cid, charm = factory.makeCharm(self.db, promulgated=True, owner='foo') |
485 | + request = self.getRequest() |
486 | + request.matchdict = { |
487 | + 'charm': charm['name'], |
488 | + 'series': charm['series'], |
489 | + } |
490 | + response = featured_edit(request, charm, 'charm', '') |
491 | + |
492 | + self.assertIn('form', response) |
493 | + self.assertIn('<label', response['form']) |
494 | + |
495 | + def test_form_includes_current_featured_value(self): |
496 | + """GET requests result in the current featured value being returned.""" |
497 | + cid, charm = factory.makeCharm(self.db, promulgated=True, owner='foo') |
498 | + request = self.getRequest() |
499 | + request.matchdict = { |
500 | + 'charm': charm['name'], |
501 | + 'series': charm['series'], |
502 | + } |
503 | + response = featured_edit(request, charm, 'charm', '') |
504 | + self.assertFalse(response['values']['is_featured']) |
505 | + |
506 | + def test_checking_checkbox_sets_featured_status(self): |
507 | + """Verify checking the checkbox causes the charm to be featured.""" |
508 | + charm = factory.get_charm_json(name='name1', promulgated=True) |
509 | + self.use_index_client() |
510 | + self.enable_routes() |
511 | + charm_source = CharmSource.from_request(self) |
512 | + charm_source.save(charm) |
513 | + # Add the submit button to trigger the right path through the view. |
514 | + post_data = { |
515 | + '_charset_': 'UTF-8', |
516 | + '__formid__': 'deform', |
517 | + 'submit': 'submit', |
518 | + 'is_featured': 'true', |
519 | + } |
520 | + request = self.getRequest(post=post_data) |
521 | + request.matchdict = { |
522 | + 'charm': charm['name'], |
523 | + 'series': charm['series'], |
524 | + } |
525 | + # Before processing the request, the charm is not featured. |
526 | + featured_source = FeaturedSource.from_db(self.db) |
527 | + self.assertFalse(featured_source.is_featured(charm, 'charm')) |
528 | + featured_edit(request, charm, 'charm', '') |
529 | + for found in charm_source._get_all(charm['_id']): |
530 | + self.assertTrue(featured_source.is_featured(found, 'charm')) |
531 | + |
532 | + def test_unchecking_checkbox_clears_featured_status(self): |
533 | + """Verify unchecking the checkbox causes the charm to be unfeatured.""" |
534 | + charm = factory.get_charm_json(name='name1', promulgated=True) |
535 | + self.use_index_client() |
536 | + self.enable_routes() |
537 | + charm_source = CharmSource.from_request(self) |
538 | + charm_source.save(charm) |
539 | + # Add the submit button to trigger the right path through the view. |
540 | + post_data = { |
541 | + '_charset_': 'UTF-8', |
542 | + '__formid__': 'deform', |
543 | + 'submit': 'submit', |
544 | + } |
545 | + request = self.getRequest(post=post_data) |
546 | + request.matchdict = { |
547 | + 'charm': charm['name'], |
548 | + 'series': charm['series'], |
549 | + } |
550 | + # Before processing the request, the charm is featured. |
551 | + featured_source = FeaturedSource.from_db(self.db) |
552 | + featured_source.set_featured(charm, 'charm') |
553 | + self.assertTrue(featured_source.is_featured(charm, 'charm')) |
554 | + featured_edit(request, charm, 'charm', '') |
555 | + for found in charm_source._get_all(charm['_id']): |
556 | + self.assertFalse(featured_source.is_featured(found, 'charm')) |
557 | + |
558 | + def test_redirect_on_submit(self): |
559 | + """After a submit, an HTTP redirect is raised with the given URL.""" |
560 | + charm = factory.get_charm_json(name='name1', promulgated=True) |
561 | + self.use_index_client() |
562 | + self.enable_routes() |
563 | + charm_source = CharmSource.from_request(self) |
564 | + charm_source.save(charm) |
565 | + # Add the submit button to trigger the right path through the view. |
566 | + post_data = { |
567 | + '_charset_': 'UTF-8', |
568 | + '__formid__': 'deform', |
569 | + 'submit': 'submit', |
570 | + 'is_featured': 'true', |
571 | + } |
572 | + request = self.getRequest(post=post_data) |
573 | + request.matchdict = { |
574 | + 'charm': charm['name'], |
575 | + 'series': charm['series'], |
576 | + } |
577 | + # Before processing the request, the charm is not featured. |
578 | + featured_source = FeaturedSource.from_db(self.db) |
579 | + self.assertFalse(featured_source.is_featured(charm, 'charm')) |
580 | + response = featured_edit(request, charm, 'charm', 'URL') |
581 | + self.assertEqual(302, response.status_code) |
582 | + self.assertEqual(response.headers['Location'], 'URL') |
583 | + |
584 | + |
585 | +class TestFeaturedCharmForm(ViewTestBase, WebTestBase): |
586 | + |
587 | + def setUp(self): |
588 | + super(TestFeaturedCharmForm, self).setUp() |
589 | + # Register the routes so URL construction will work in the tests. |
590 | + self.enable_routes() |
591 | + |
592 | + def test_form_renders(self): |
593 | + """Sanity check that the form renders without errors.""" |
594 | + cid, charm = factory.makeCharm(self.db, promulgated=True, owner='foo') |
595 | + request = self.getRequest() |
596 | + request.matchdict = { |
597 | + 'charm': charm['name'], |
598 | + 'series': charm['series'], |
599 | + } |
600 | + response = charm_featured_edit(request) |
601 | + |
602 | + self.assertIn('form', response) |
603 | + self.assertIn('<label', response['form']) |
604 | + |
605 | + def test_charm_feature_form_route(self): |
606 | + """The charm feature form is wired up to a route.""" |
607 | + charm = factory.makeCharm(self.db, promulgated=True, owner='foo')[1] |
608 | + url = '/charms/{series}/{name}/featured/edit'.format(**charm) |
609 | + with login_request(self.app, groups=['charmers']): |
610 | + # Get the URL and assert the given status code. |
611 | + self.app.get(url, status=200) |
612 | + |
613 | + def test_404_if_charm_is_not_promulgated(self): |
614 | + # If the charm indicated by the URL does not exist, a 404 is generated. |
615 | + charm = factory.makeCharm(self.db, promulgated=False, owner='foo')[1] |
616 | + url = '/charms/{series}/{name}/featured/edit'.format(**charm) |
617 | + with login_request(self.app, groups=['charmers']): |
618 | + # Get the URL and assert the given status code. |
619 | + self.app.get(url, status=404) |
620 | + |
621 | + |
622 | +class TestFeaturedBundleForm(ViewTestBase, WebTestBase): |
623 | + |
624 | + def setUp(self): |
625 | + super(TestFeaturedBundleForm, self).setUp() |
626 | + # Register the routes so URL construction will work in the tests. |
627 | + self.enable_routes() |
628 | + |
629 | + def test_form_renders(self): |
630 | + """Sanity check that the form renders without errors.""" |
631 | + bundle, bundle_data = factory.makeBundle(self.db, promulgated=True) |
632 | + request = self.getRequest() |
633 | + request.matchdict = { |
634 | + 'bundle': bundle_data['name'], |
635 | + 'basket': bundle_data['basket_name'], |
636 | + } |
637 | + response = bundle_featured_edit(request) |
638 | + |
639 | + self.assertIn('form', response) |
640 | + self.assertIn('<label', response['form']) |
641 | + |
642 | + def test_bundle_feature_form_route(self): |
643 | + """The bundle feature form is wired up to a route.""" |
644 | + bundle, bundle_data = factory.makeBundle(self.db, promulgated=True) |
645 | + url = '/bundles/{basket_name}/{name}/featured/edit'.format( |
646 | + **bundle_data) |
647 | + with login_request(self.app, groups=['charmers']): |
648 | + # Get the URL and assert the given status code. |
649 | + self.app.get(url, status=200) |
650 | + |
651 | + def test_404_generated_when_bundle_is_not_promulgated(self): |
652 | + # If the bundle indicated by the URL does not exist, a 404 is |
653 | + # generated. |
654 | + bundle, bundle_data = factory.makeBundle(self.db, promulgated=False) |
655 | + url = '/bundles/{basket_name}/{name}/featured/edit'.format( |
656 | + **bundle_data) |
657 | + with login_request(self.app, groups=['charmers']): |
658 | + # Get the URL and assert the given status code. |
659 | + self.app.get(url, status=404) |
660 | + |
661 | + |
662 | +class TestFeaturedFormAuthorization(WebTestBase): |
663 | + |
664 | + def test_form_403s_for_non_charmers(self): |
665 | + """The form is not permitted if you're not a charmer.""" |
666 | + with login_request(self.app, groups=[]) as resp: |
667 | + cid, charm = factory.makeCharm(self.db) |
668 | + # The status force the check against the status code. |
669 | + resp = resp.goto( |
670 | + 'charms/%s/%s/featured/edit' % |
671 | + (charm['series'], charm['name']), status=403) |
672 | + self.assertEqual(403, resp.status_code) |
673 | + |
674 | + def test_form_secured_from_users(self): |
675 | + """The form is only allowed to be edited/submitted by charmers.""" |
676 | + with login_request(self.app, groups=['charmers']) as resp: |
677 | + cid, charm = factory.makeCharm(self.db, promulgated=True) |
678 | + resp = resp.goto( |
679 | + 'charms/%s/%s/featured/edit' % |
680 | + (charm['series'], charm['name']), |
681 | + status=200) |
682 | + self.assertEqual(resp.status_code, 200) |
Thanks for cleaning up my lint in those tests.
At 206 except ValidationFailure as form:
This use is too subtle, I think, if the intent is to re-assign the form variable for the remainder of the method. Perhaps catching it as a temporary and reassigning the real 'form' in the handler would be more explicit.
The rest looks good. The tests are nice. Thanks for including the routing test, we need more of them.
QA:
http:// 127.0.0. 1:2464/ ~bac/bundles/ wiki/wiki/ featured/ edit
This URL should not be supported and gives a 404
http:// 127.0.0. 1:2464/ bundles/ wiki/wiki/ featured/ edit
This one, maybe should be supported, but it is not promulgated. It cause an exception to be raised in bundle_ featured_ edit because find_bundle returns None.
I think you should call found(bundle) to force a 404 in this case.