Merge lp:~canonical-isd-hackers/wordpress-teams-integration/prevent-removing-bound-server-fix into lp:wordpress-teams-integration

[Szilveszter Farkas]

Do not allow deleting servers if one of the selected servers are bound to an existing role.

[Stuart Metcalfe (stuartmetcalfe)]

Everything in rev. 27 looks good so far. We discussed removing the 'delete' checkbox from the servers form for each server already assigned to a team to avoid even giving the option to delete a server which the user shouldn't be able to delete. This looks possible, so will re-review when that's landed.

[Stuart Metcalfe (stuartmetcalfe)]

The usual exception to missing tests, for the record. This code doesn't have any unit test coverage and the app doesn't provide a framework that we're aware of so no unit test coverage is included.

[lp:~canonical-isd-hackers/wordpress-teams-integration/prevent-removing-bound-server-fix]

28. By Szilveszter Farkas on 2010-04-27

Do not display the checkbox if cannot delete.

29. By Szilveszter Farkas on 2010-04-27

Add missing documentation.

30. By Szilveszter Farkas on 2010-04-27

Add docstring for delete_server_from_trusted().

[Stuart Metcalfe (stuartmetcalfe)]

With the doctest improvements discussed in IRC, which landed in rev. 30, this is approved. Good work!

review: Approve

=== modified file 'openid-teams.php'
--- openid-teams.php	2010-03-02 14:45:34 +0000
+++ openid-teams.php	2010-04-27 09:45:38 +0000
@@ -175,20 +175,45 @@
   }
 }
 
-function delete_server_from_trusted($id, $trusted_servers = null) {
-  if (is_null($trusted_servers)) {
-    $trusted_servers = openid_get_server_list();
-  }
-  unset($trusted_servers[$id]);
-  openid_teams_update_trusted_servers($trusted_servers);
+/**
+ * Check if given server is bound to a role
+ *
+ * @param int $id Internal ID of the server to be queried
+ *
+ * @return boolean
+ */
+function is_server_bound($id) {
   $all_trust_maps = openid_teams_get_trust_list();
+  $role_found = false;
   foreach ($all_trust_maps as $map_id => $trust_map) {
     if ($trust_map->server == $id) {
-      $all_trust_maps[$map_id]->server = -1;
+      $role_found = true;
+      break;
     }
   }
-  openid_teams_update_trust_list($all_trust_maps);
-}
+  return $role_found;
+}
+
+/**
+ * Delete given server from the list of trusted servers
+ *
+ * @param int $id
+ * @param array $trusted_servers
+ */
+function delete_server_from_trusted($id, $trusted_servers = null) {
+  if (is_null($trusted_servers)) {
+    $trusted_servers = openid_get_server_list();
+  }
+  if (is_server_bound($id)) {
+    print '<div id="message" class="error"><p>'.__("You cannot remove a server that has roles associated with it.").'</p></div>';
+  } else {
+    $all_trust_maps = openid_teams_get_trust_list();
+    unset($trusted_servers[$id]);
+    openid_teams_update_trusted_servers($trusted_servers);
+    openid_teams_update_trust_list($all_trust_maps);
+  }
+}
+
 /**
  * Get the list of trusted servers
  *
@@ -223,6 +248,8 @@
  * Check if given server is on trusted server list
  *
  * @param string $server
+ *
+ * @return bool
  */
 function in_trusted_servers($server) {
   foreach (openid_get_server_list() as $trusted) {
@@ -330,7 +357,7 @@
     ?>
     <tr>
       <td><?php print htmlentities($server); ?></td>
-      <td><input type="checkbox" name="delete[<?php print $id ?>]" value="1" /></td>
+      <td style="text-align:center;"><?php if (!is_server_bound($id)): ?><input type="checkbox" name="delete[<?php print $id ?>]" value="1" /><?php else: print __("(associated with a role)"); endif; ?></td>
     </tr>
     <?php
     }