Merge pull request #2453 from alphagov/model-service-update

Use the model for updating a service and its permissions

app/main/views/service_settings.py

@@ -108,8 +108,7 @@ def service_name_change_confirm(service_id):
 
     if form.validate_on_submit():
         try:
-            service_api_client.update_service(
-                current_service.id,
+            current_service.update(
                 name=session['service_name_change'],
                 email_from=email_safe(session['service_name_change'])
             )
@@ -209,8 +208,7 @@ def submit_request_to_go_live(service_id):
 @login_required
 @user_is_platform_admin
 def service_switch_live(service_id):
-    service_api_client.update_service(
-        current_service.id,
+    current_service.update(
         # TODO This limit should be set depending on the agreement signed by
         # with Notify.
         message_limit=250000 if current_service.trial_mode else 50,
@@ -223,49 +221,15 @@ def service_switch_live(service_id):
 @login_required
 @user_is_platform_admin
 def service_switch_research_mode(service_id):
-    service_api_client.update_service_with_properties(
-        service_id,
-        {"research_mode": not current_service.research_mode}
-    )
+    current_service.toggle_research_mode()
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
-def switch_service_permissions(service_id, permission, sms_sender=None):
-
-    force_service_permission(
-        service_id,
-        permission,
-        on=permission not in current_service.permissions,
-        sms_sender=sms_sender
-    )
-
-
-def force_service_permission(service_id, permission, on=False, sms_sender=None):
-
-    permissions, permission = set(current_service.permissions), {permission}
-
-    update_service_permissions(
-        service_id,
-        permissions | permission if on else permissions - permission,
-        sms_sender=sms_sender
-    )
-
-
-def update_service_permissions(service_id, permissions, sms_sender=None):
-
-    data = {'permissions': list(permissions)}
-
-    if sms_sender:
-        data['sms_sender'] = sms_sender
-
-    service_api_client.update_service_with_properties(service_id, data)
-
-
 @main.route("/services/<service_id>/service-settings/can-send-email")
 @login_required
 @user_is_platform_admin
 def service_switch_can_send_email(service_id):
-    switch_service_permissions(service_id, 'email')
+    current_service.switch_permission('email')
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
@@ -273,7 +237,7 @@ def service_switch_can_send_email(service_id):
 @login_required
 @user_is_platform_admin
 def service_switch_can_send_sms(service_id):
-    switch_service_permissions(service_id, 'sms')
+    current_service.switch_permission('sms')
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
@@ -281,7 +245,7 @@ def service_switch_can_send_sms(service_id):
 @login_required
 @user_is_platform_admin
 def service_switch_email_auth(service_id):
-    switch_service_permissions(service_id, 'email_auth')
+    current_service.switch_permission('email_auth')
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
@@ -289,7 +253,7 @@ def service_switch_email_auth(service_id):
 @login_required
 @user_is_platform_admin
 def service_switch_can_send_precompiled_letter(service_id):
-    switch_service_permissions(service_id, 'precompiled_letter')
+    current_service.switch_permission('precompiled_letter')
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
@@ -302,17 +266,16 @@ def service_switch_can_upload_document(service_id):
     # If turning the permission off, or turning it on and the service already has a contact_link,
     # don't show the form to add the link
     if current_service.has_permission('upload_document') or current_service.contact_link:
-        switch_service_permissions(service_id, 'upload_document')
+        current_service.switch_permission('upload_document')
         return redirect(url_for('.service_settings', service_id=service_id))
 
     if form.validate_on_submit():
         contact_type = form.contact_details_type.data
 
-        service_api_client.update_service(
-            current_service.id,
+        current_service.update(
             contact_link=form.data[contact_type]
         )
-        switch_service_permissions(service_id, 'upload_document')
+        current_service.switch_permission('upload_document')
         return redirect(url_for('.service_settings', service_id=service_id))
 
     return render_template('views/service-settings/contact_link.html', form=form)
@@ -322,7 +285,7 @@ def service_switch_can_upload_document(service_id):
 @login_required
 @user_is_platform_admin
 def service_switch_can_edit_folders(service_id):
-    switch_service_permissions(service_id, 'edit_folders')
+    current_service.switch_permission('edit_folders')
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
@@ -380,8 +343,7 @@ def service_set_contact_link(service_id):
     if form.validate_on_submit():
         contact_type = form.contact_details_type.data
 
-        service_api_client.update_service(
-            current_service.id,
+        current_service.update(
             contact_link=form.data[contact_type]
         )
         return redirect(url_for('.service_settings', service_id=current_service.id))
@@ -495,7 +457,7 @@ def service_set_inbound_number(service_id):
             is_default=True,
             inbound_number_id=form.inbound_number.data
         )
-        switch_service_permissions(current_service.id, 'inbound_sms')
+        current_service.force_permission('inbound_sms', on=True)
         return redirect(url_for('.service_settings', service_id=service_id))
     return render_template(
         'views/service-settings/set-inbound-number.html',
@@ -525,8 +487,7 @@ def service_set_sms_prefix(service_id):
     form.enabled.label.text = 'Start all text messages with ‘{}:’'.format(current_service.name)
 
     if form.validate_on_submit():
-        service_api_client.update_service(
-            current_service.id,
+        current_service.update(
             prefix_sms=(form.enabled.data == 'on')
         )
         return redirect(url_for('.service_settings', service_id=service_id))
@@ -545,8 +506,7 @@ def service_set_international_sms(service_id):
         enabled='on' if current_service.has_permission('international_sms') else 'off'
     )
     if form.validate_on_submit():
-        force_service_permission(
-            service_id,
+        current_service.force_permission(
             'international_sms',
             on=(form.enabled.data == 'on'),
         )
@@ -576,8 +536,7 @@ def service_set_letters(service_id):
         enabled='on' if current_service.has_permission('letter') else 'off'
     )
     if form.validate_on_submit():
-        force_service_permission(
-            service_id,
+        current_service.force_permission(
             'letter',
             on=(form.enabled.data == 'on'),
         )
@@ -597,7 +556,7 @@ def service_set_postage(service_id):
     form = ServicePostageForm(postage=current_service.postage)
 
     if form.validate_on_submit():
-        service_api_client.update_service(service_id, postage=form.postage.data)
+        current_service.update(postage=form.postage.data)
         return redirect(url_for(".service_settings", service_id=service_id))
 
     return render_template('views/service-settings/set-postage.html', form=form)
@@ -762,8 +721,7 @@ def service_set_letter_contact_block(service_id):
 
     form = ServiceLetterContactBlockForm(letter_contact_block=current_service.letter_contact_block)
     if form.validate_on_submit():
-        service_api_client.update_service(
-            current_service.id,
+        current_service.update(
             letter_contact_block=form.letter_contact_block.data.replace('\r', '') or None
         )
         if request.args.get('from_template'):
@@ -788,8 +746,7 @@ def set_organisation_type(service_id):
         free_sms_fragment_limit = current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get(
             form.organisation_type.data)
 
-        service_api_client.update_service(
-            service_id,
+        current_service.update(
             organisation_type=form.organisation_type.data,
         )
         billing_api_client.create_or_update_free_sms_fragment_limit(service_id, free_sms_fragment_limit)
@@ -857,8 +814,7 @@ def service_preview_email_branding(service_id):
 
     if form.validate_on_submit():
         branding_style = None if form.branding_style.data == 'None' else form.branding_style.data
-        service_api_client.update_service(
-            service_id,
+        current_service.update(
             email_branding=branding_style
         )
         return redirect(url_for('.service_settings', service_id=service_id))
@@ -879,8 +835,7 @@ def set_letter_branding(service_id):
     form = LetterBranding(choices=email_branding_client.get_letter_email_branding().items())
 
     if form.validate_on_submit():
-        service_api_client.update_service(
-            service_id,
+        current_service.update(
             dvla_organisation=form.dvla_org_id.data
         )
         return redirect(url_for('.service_settings', service_id=service_id))

app/models/service.py

@@ -47,6 +47,29 @@ class Service():
             return self._dict[attr]
         raise AttributeError('`{}` is not a service attribute'.format(attr))
 
+    def update(self, **kwargs):
+        return service_api_client.update_service(self.id, **kwargs)
+
+    def switch_permission(self, permission):
+        return self.force_permission(
+            permission,
+            on=not self.has_permission(permission),
+        )
+
+    def force_permission(self, permission, on=False):
+
+        permissions, permission = set(self.permissions), {permission}
+
+        return self.update_permissions(
+            permissions | permission if on else permissions - permission,
+        )
+
+    def update_permissions(self, permissions):
+        return self.update(permissions=list(permissions))
+
+    def toggle_research_mode(self):
+        self.update(research_mode=not self.research_mode)
+
     @property
     def trial_mode(self):
         return self._dict['restricted']