Allow unique campaign names (#3119)

* Allow unique campaign names #2728 * Migration file * Delete 5053c01cb170_.py * improve text and position of campaign creation error message * Hanlde duplicate names for editing existing campaigns * Update campaign error code update * Update down revision * fix bug in Organisation.as_dto() + enable editMode on campaign save failure Co-authored-by: Wille Marcel <wille.yyz@gmail.com>
hotosm · Jun 30, 2020 · f6f98b6 · f6f98b6
1 parent 1ef9aed
commit f6f98b6
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 16 deletions.
diff --git a/backend/api/campaigns/resources.py b/backend/api/campaigns/resources.py
@@ -116,6 +116,8 @@ def patch(self, campaign_id):
                 description: Unauthorized - Invalid credentials
             403:
                 description: Forbidden
+            409:
+                description: Resource duplication
             500:
                 description: Internal Server Error
         """
@@ -141,8 +143,11 @@ def patch(self, campaign_id):
             return {"Success": "Campaign {} updated".format(campaign.id)}, 200
         except NotFound:
             return {"Error": "Campaign not found"}, 404
+        except ValueError:
+            error_msg = "Campaign PATCH - name already exists"
+            return {"Error": error_msg}, 409
         except Exception as e:
-            error_msg = f"Campaign PUT - unhandled error: {str(e)}"
+            error_msg = f"Campaign PATCH - unhandled error: {str(e)}"
             current_app.logger.critical(error_msg)
             return {"Error": error_msg}, 500
 
@@ -280,6 +285,8 @@ def post(self):
                 description: Unauthorized - Invalid credentials
             403:
                 description: Forbidden
+            409:
+                description: Resource duplication
             500:
                 description: Internal Server Error
         """
@@ -303,6 +310,9 @@ def post(self):
         try:
             campaign = CampaignService.create_campaign(campaign_dto)
             return {"campaignId": campaign.id}, 200
+        except ValueError:
+            error_msg = "Campaign POST - name already exists"
+            return {"Error": error_msg}, 409
         except Exception as e:
             error_msg = f"Campaign POST - unhandled error: {str(e)}"
             current_app.logger.critical(error_msg)

diff --git a/backend/models/postgis/campaign.py b/backend/models/postgis/campaign.py
@@ -26,7 +26,7 @@ class Campaign(db.Model):
     __tablename__ = "campaigns"
 
     id = db.Column(db.Integer, primary_key=True)
-    name = db.Column(db.String, nullable=False)
+    name = db.Column(db.String, nullable=False, unique=True)
     logo = db.Column(db.String)
     url = db.Column(db.String)
     description = db.Column(db.String)

diff --git a/backend/models/postgis/organisation.py b/backend/models/postgis/organisation.py
@@ -156,7 +156,7 @@ def get_organisations_managed_by_user(user_id: int):
         )
         return query_results
 
-    def as_dto(self, omit_managers):
+    def as_dto(self, omit_managers=False):
         """ Returns a dto for an organisation """
         organisation_dto = OrganisationDTO()
         organisation_dto.organisation_id = self.id

diff --git a/backend/services/campaign_service.py b/backend/services/campaign_service.py
@@ -1,4 +1,6 @@
 from backend import db
+from flask import current_app
+from sqlalchemy.exc import IntegrityError
 from backend.models.dtos.campaign_dto import (
     CampaignDTO,
     NewCampaignDTO,
@@ -94,12 +96,17 @@ def get_all_campaigns() -> CampaignListDTO:
     @staticmethod
     def create_campaign(campaign_dto: NewCampaignDTO):
         campaign = Campaign.from_dto(campaign_dto)
-        campaign.create()
-        if campaign_dto.organisations:
-            for org_id in campaign_dto.organisations:
-                organisation = OrganisationService.get_organisation_by_id(org_id)
-                campaign.organisation.append(organisation)
-            db.session.commit()
+        try:
+            campaign.create()
+            if campaign_dto.organisations:
+                for org_id in campaign_dto.organisations:
+                    organisation = OrganisationService.get_organisation_by_id(org_id)
+                    campaign.organisation.append(organisation)
+                db.session.commit()
+        except IntegrityError as e:
+            current_app.logger.info("Integrity error: {}".format(e.args[0]))
+            raise ValueError()
+
         return campaign
 
     @staticmethod
@@ -151,5 +158,10 @@ def update_campaign(campaign_dto: CampaignDTO, campaign_id: int):
         campaign = Campaign.query.get(campaign_id)
         if not campaign:
             raise NotFound(f"Campaign id {campaign_id} not found")
-        campaign.update(campaign_dto)
+        try:
+            campaign.update(campaign_dto)
+        except IntegrityError as e:
+            current_app.logger.info("Integrity error: {}".format(e.args[0]))
+            raise ValueError()
+
         return campaign
diff --git a/frontend/src/components/teamsAndOrgs/campaigns.js b/frontend/src/components/teamsAndOrgs/campaigns.js
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import { Link } from '@reach/router';
 import { FormattedMessage } from 'react-intl';
 import { Form, Field } from 'react-final-form';
@@ -68,6 +68,10 @@ export function CampaignInformation(props) {
 export function CampaignForm(props) {
   const [editMode, setEditMode] = useState(false);
 
+  useEffect(() => {
+    if (props.saveError) setEditMode(true);
+  }, [props.saveError]);
+
   return (
     <Form
       onSubmit={(values) => props.updateCampaign(values)}

diff --git a/frontend/src/locales/en.json b/frontend/src/locales/en.json
@@ -895,5 +895,6 @@
   "pages.edit_project.sections.permissions": "Permissions",
   "pages.edit_project.sections.settings": "Settings",
   "pages.edit_project.sections.actions": "Actions",
-  "pages.edit_project.sections.custom_editor": "Custom Editor"
+  "pages.edit_project.sections.custom_editor": "Custom Editor",
+  "pages.create_campaign.duplicate": "A campaign with the same name already exists"
 }
diff --git a/frontend/src/views/campaigns.js b/frontend/src/views/campaigns.js
@@ -18,6 +18,7 @@ import { Projects } from '../components/teamsAndOrgs/projects';
 import { FormSubmitButton, CustomButton } from '../components/button';
 import { DeleteModal } from '../components/deleteModal';
 import { useSetTitleTag } from '../hooks/UseMetaTags';
+import { CloseIcon } from '../components/svgIcons';
 
 export function ListCampaigns() {
   useSetTitleTag('Manage campaigns');
@@ -50,6 +51,7 @@ export function ListCampaigns() {
 export function CreateCampaign() {
   useSetTitleTag('Create new campaign');
   const token = useSelector((state) => state.auth.get('token'));
+  const [error, setError] = useState(null);
   const [newCampaignId, setNewCampaignId] = useState(null);
 
   useEffect(() => {
@@ -59,11 +61,29 @@ export function CreateCampaign() {
   }, [newCampaignId]);
 
   const createCampaign = (payload) => {
-    pushToLocalJSONAPI('campaigns/', JSON.stringify(payload), token, 'POST').then((result) =>
-      setNewCampaignId(result.campaignId),
+    pushToLocalJSONAPI('campaigns/', JSON.stringify(payload), token, 'POST')
+      .then((result) => setNewCampaignId(result.campaignId))
+      .catch((e) => setError(e));
+  };
+
+  const ServerMessage = () => {
+    return (
+      <div className="red ba b--red pa2 br1 dib pa2">
+        <CloseIcon className="h1 w1 v-mid pb1 red mr2" />
+        <FormattedMessage {...messages.duplicateCampaign} />
+      </div>
     );
   };
 
+  const ErrorMessage = ({ error, success }) => {
+    let message = null;
+    if (error !== null) {
+      message = <ServerMessage />;
+    }
+
+    return <div className="db mt3">{message}</div>;
+  };
+
   return (
     <Form
       onSubmit={(values) => createCampaign(values)}
@@ -80,9 +100,9 @@ export function CreateCampaign() {
                     <FormattedMessage {...messages.campaignInfo} />
                   </h3>
                   <CampaignInformation />
+                  <ErrorMessage error={error} />
                 </div>
               </div>
-              <div className="w-40-l w-100 fl pl5-l pl0 "></div>
             </div>
             <div className="fixed left-0 bottom-0 cf bg-white h3 w-100">
               <div className="w-80-ns w-60-m w-50 h-100 fl tr">
@@ -118,9 +138,31 @@ export function EditCampaign(props) {
     `projects/?campaign=${encodeURIComponent(campaign.name)}&omitMapResults=true`,
     campaign.name !== undefined,
   );
+  const [nameError, setNameError] = useState(null);
 
   const updateCampaign = (payload) => {
-    pushToLocalJSONAPI(`campaigns/${props.id}/`, JSON.stringify(payload), token, 'PATCH');
+    pushToLocalJSONAPI(`campaigns/${props.id}/`, JSON.stringify(payload), token, 'PATCH')
+      .then((res) => setNameError(null))
+      .catch((e) => setNameError(e));
+  };
+
+  const ServerMessage = () => {
+    return (
+      <div className="red ba b--red pa2 br1 dib pa2">
+        <CloseIcon className="h1 w1 v-mid pb1 red mr2" />
+        <FormattedMessage {...messages.duplicateCampaign} />
+      </div>
+    );
+  };
+
+  const ErrorMessage = ({ nameError, success }) => {
+    let message = null;
+    console.log(nameError);
+    if (nameError !== null) {
+      message = <ServerMessage />;
+    }
+
+    return <div className="db mt3">{message}</div>;
   };
 
   return (
@@ -137,8 +179,11 @@ export function EditCampaign(props) {
           campaign={{ name: campaign.name }}
           updateCampaign={updateCampaign}
           disabledForm={error || loading}
+          saveError={nameError}
         />
+        <ErrorMessage nameError={nameError} />
       </div>
+
       <div className="w-60-l w-100 mt4 pl5-l pl0 fl">
         <Projects
           projects={!projectsLoading && !projectsError && projects}

diff --git a/frontend/src/views/messages.js b/frontend/src/views/messages.js
@@ -569,4 +569,8 @@ export default defineMessages({
     id: 'pages.edit_project.sections.custom_editor',
     defaultMessage: 'Custom Editor',
   },
+  duplicateCampaign: {
+    id: 'pages.create_campaign.duplicate',
+    defaultMessage: 'A campaign with the same name already exists',
+  },
 });
diff --git a/migrations/versions/5053c01cb170_.py b/migrations/versions/5053c01cb170_.py
@@ -0,0 +1,27 @@
+"""empty message
+
+Revision ID: 5053c01cb170
+Revises: 5952780a577e
+Create Date: 2020-06-09 13:51:18.754882
+"""
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+
+revision = "5053c01cb170"
+down_revision = "5952780a577e"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_unique_constraint("campaigns_name_key", "campaigns", ["name"])
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_constraint("campaigns_name_key", "campaigns", type_="unique")
+    # ### end Alembic commands ###