Merge lp:~allenap/maas/add-remove-tag-using-post--bug-1611711 into lp:~maas-committers/maas/trunk

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 5236
Proposed branch: lp:~allenap/maas/add-remove-tag-using-post--bug-1611711
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 111 lines (+12/-12)
2 files modified
src/maasserver/api/blockdevices.py (+4/-4)
src/maasserver/api/tests/test_blockdevice.py (+8/-8)
To merge this branch: bzr merge lp:~allenap/maas/add-remove-tag-using-post--bug-1611711
Reviewer Review Type Date Requested Status
Blake Rouse (community) Approve
Review via email: mp+302886@code.launchpad.net

Commit message

Switch BlockDevice.add_tag and remove_tag to be POST operations rather than GET.

To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) wrote :

Well that was easy. Might be a good time in 2.1 to rename that variable.

review: Approve
Revision history for this message
Gavin Panella (allenap) wrote :

> Well that was easy. Might be a good time in 2.1 to rename that variable.

Yeah!

Thanks for the review.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/blockdevices.py'
2--- src/maasserver/api/blockdevices.py 2016-03-28 13:54:47 +0000
3+++ src/maasserver/api/blockdevices.py 2016-08-14 17:48:29 +0000
4@@ -267,7 +267,7 @@
5 else:
6 raise MAASAPIValidationError(form.errors)
7
8- @operation(idempotent=True)
9+ @operation(idempotent=False)
10 def add_tag(self, request, system_id, device_id):
11 """Add a tag to block device on a machine.
12
13@@ -283,11 +283,11 @@
14 if node.status != NODE_STATUS.READY:
15 raise NodeStateViolation(
16 "Cannot update block device because the machine is not Ready.")
17- device.add_tag(get_mandatory_param(request.GET, 'tag'))
18+ device.add_tag(get_mandatory_param(request.POST, 'tag'))
19 device.save()
20 return device
21
22- @operation(idempotent=True)
23+ @operation(idempotent=False)
24 def remove_tag(self, request, system_id, device_id):
25 """Remove a tag from block device on a machine.
26
27@@ -303,7 +303,7 @@
28 if node.status != NODE_STATUS.READY:
29 raise NodeStateViolation(
30 "Cannot update block device because the machine is not Ready.")
31- device.remove_tag(get_mandatory_param(request.GET, 'tag'))
32+ device.remove_tag(get_mandatory_param(request.POST, 'tag'))
33 device.save()
34 return device
35
36
37=== modified file 'src/maasserver/api/tests/test_blockdevice.py'
38--- src/maasserver/api/tests/test_blockdevice.py 2016-05-24 22:05:45 +0000
39+++ src/maasserver/api/tests/test_blockdevice.py 2016-08-14 17:48:29 +0000
40@@ -422,7 +422,7 @@
41 def test_add_tag_returns_403_when_not_admin(self):
42 block_device = factory.make_PhysicalBlockDevice()
43 uri = get_blockdevice_uri(block_device)
44- response = self.client.get(
45+ response = self.client.post(
46 uri, {'op': 'add_tag', 'tag': factory.make_name('tag')})
47 self.assertEqual(
48 http.client.FORBIDDEN, response.status_code, response.content)
49@@ -432,7 +432,7 @@
50 block_device = factory.make_PhysicalBlockDevice()
51 other_node = factory.make_Node()
52 uri = get_blockdevice_uri(block_device, node=other_node)
53- response = self.client.get(
54+ response = self.client.post(
55 uri, {'op': 'add_tag', 'tag': factory.make_name('tag')})
56 self.assertEqual(
57 http.client.NOT_FOUND, response.status_code, response.content)
58@@ -443,7 +443,7 @@
59 status=factory.pick_enum(NODE_STATUS, but_not=[NODE_STATUS.READY]))
60 block_device = factory.make_VirtualBlockDevice(node=node)
61 uri = get_blockdevice_uri(block_device)
62- response = self.client.get(
63+ response = self.client.post(
64 uri, {'op': 'add_tag', 'tag': factory.make_name('tag')})
65 self.assertEqual(
66 http.client.CONFLICT, response.status_code, response.content)
67@@ -454,7 +454,7 @@
68 block_device = factory.make_PhysicalBlockDevice(node=node)
69 tag_to_be_added = factory.make_name('tag')
70 uri = get_blockdevice_uri(block_device)
71- response = self.client.get(
72+ response = self.client.post(
73 uri, {'op': 'add_tag', 'tag': tag_to_be_added})
74
75 self.assertEqual(
76@@ -467,7 +467,7 @@
77 def test_remove_tag_returns_403_when_not_admin(self):
78 block_device = factory.make_PhysicalBlockDevice()
79 uri = get_blockdevice_uri(block_device)
80- response = self.client.get(
81+ response = self.client.post(
82 uri, {'op': 'remove_tag', 'tag': factory.make_name('tag')})
83
84 self.assertEqual(
85@@ -478,7 +478,7 @@
86 block_device = factory.make_PhysicalBlockDevice()
87 other_node = factory.make_Node()
88 uri = get_blockdevice_uri(block_device, node=other_node)
89- response = self.client.get(
90+ response = self.client.post(
91 uri, {'op': 'remove_tag', 'tag': factory.make_name('tag')})
92
93 self.assertEqual(
94@@ -490,7 +490,7 @@
95 status=factory.pick_enum(NODE_STATUS, but_not=[NODE_STATUS.READY]))
96 block_device = factory.make_PhysicalBlockDevice(node=node)
97 uri = get_blockdevice_uri(block_device)
98- response = self.client.get(
99+ response = self.client.post(
100 uri, {'op': 'remove_tag', 'tag': factory.make_name('tag')})
101
102 self.assertEqual(
103@@ -502,7 +502,7 @@
104 block_device = factory.make_PhysicalBlockDevice(node=node)
105 tag_to_be_removed = block_device.tags[0]
106 uri = get_blockdevice_uri(block_device)
107- response = self.client.get(
108+ response = self.client.post(
109 uri, {'op': 'remove_tag', 'tag': tag_to_be_removed})
110
111 self.assertEqual(