Merge pull request #3219 from kobotoolbox/3218-implied-add-submission…

…-permission

Assigning "Change submissions" implies "Add submissions"

jsapp/js/actions.es6

@@ -539,7 +539,7 @@ actions.resources.duplicateSubmission.listen((uid, sid, duplicatedSubmission) =>
       actions.resources.loadAsset({id: uid});
     })
     .fail((response) => {
-      alertify.error(t('Failed to duplicate submisson'));
+      alertify.error(t('Failed to duplicate submission'));
       actions.resources.duplicateSubmission.failed(response);
     });
 });

jsapp/js/components/assetrow.es6

@@ -203,7 +203,7 @@ class AssetRow extends React.Component {
             { this.props.asset_type == ASSET_TYPES.survey.id &&
               <bem.AssetRow__cell
                 m={'submission-count'}
-                key={'submisson-count'}
+                key={'submission-count'}
                 className='mdl-cell mdl-cell--1-col mdl-cell--1-col-tablet mdl-cell--1-col-phone'
               >
                 {

jsapp/js/components/modalForms/submission.es6

@@ -457,7 +457,6 @@ class Submission extends React.Component {
               }
 
               {this.userCan('change_submissions', this.props.asset) &&
-                      this.userCan('add_submissions', this.props.asset) &&
                 <a
                   onClick={this.duplicateSubmission.bind(this)}
                   className='kobo-button kobo-button--blue submission-duplicate__button'

jsapp/js/components/permissions/permissionsMocks.es6

@@ -33,7 +33,8 @@ const permissions = {
       'codename': 'change_submissions',
       'implied': [
         '/api/v2/permissions/view_asset/',
-        '/api/v2/permissions/view_submissions/'
+        '/api/v2/permissions/view_submissions/',
+        '/api/v2/permissions/add_submissions/'
       ],
       'contradictory': [
         '/api/v2/permissions/partial_submissions/'

jsapp/js/components/permissions/userAssetPermsEditor.es6

@@ -160,6 +160,12 @@ class UserAssetPermsEditor extends React.Component {
       stateObj.formViewDisabled = true;
     }
 
+    // checking `submissionsEdit` implies having `submissionsAdd` checked
+    if (stateObj.submissionsEdit) {
+      stateObj.submissionsAdd = true;
+      stateObj.submissionsAddDisabled = true;
+    }
+
     // checking these options implies having `submissionsView` checked
     if (
       stateObj.submissionsDelete ||

kpi/models/asset.py

@@ -630,7 +630,10 @@ class Meta:
         PERM_ADD_SUBMISSIONS: (PERM_VIEW_ASSET,),
         PERM_VIEW_SUBMISSIONS: (PERM_VIEW_ASSET,),
         PERM_PARTIAL_SUBMISSIONS: (PERM_VIEW_ASSET,),
-        PERM_CHANGE_SUBMISSIONS: (PERM_VIEW_SUBMISSIONS,),
+        PERM_CHANGE_SUBMISSIONS: (
+            PERM_VIEW_SUBMISSIONS,
+            PERM_ADD_SUBMISSIONS,
+        ),
         PERM_DELETE_SUBMISSIONS: (PERM_VIEW_SUBMISSIONS,),
         PERM_VALIDATE_SUBMISSIONS: (PERM_VIEW_SUBMISSIONS,),
     }

kpi/permissions.py

@@ -262,10 +262,7 @@ class EditSubmissionPermission(SubmissionPermission):
 class DuplicateSubmissionPermission(SubmissionPermission):
     perms_map = {
         'GET': ['%(app_label)s.view_%(model_name)s'],
-        'POST': [
-            '%(app_label)s.add_%(model_name)s',
-            '%(app_label)s.change_%(model_name)s',
-        ],
+        'POST': ['%(app_label)s.change_%(model_name)s'],
     }
 
 

kpi/tests/api/v2/test_api_submissions.py

@@ -533,11 +533,12 @@ def test_duplicate_submission_by_anotheruser_shared_view_only_not_allowed(self):
         response = self.client.post(self.submission_url, {'format': 'json'})
         assert response.status_code == status.HTTP_403_FORBIDDEN
 
-    def test_duplicate_submission_by_anotheruser_shared_change_not_allowed(self):
+    def test_duplicate_submission_by_anotheruser_shared_as_editor_allowed(self):
         self.asset.assign_perm(self.anotheruser, PERM_CHANGE_SUBMISSIONS)
         self._log_in_as_another_user()
         response = self.client.post(self.submission_url, {'format': 'json'})
-        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert response.status_code == status.HTTP_201_CREATED
+        self._check_duplicate(response)
 
     def test_duplicate_submission_by_anotheruser_shared_add_not_allowed(self):
         for perm in [PERM_VIEW_SUBMISSIONS, PERM_ADD_SUBMISSIONS]:
@@ -546,15 +547,6 @@ def test_duplicate_submission_by_anotheruser_shared_add_not_allowed(self):
         response = self.client.post(self.submission_url, {'format': 'json'})
         assert response.status_code == status.HTTP_403_FORBIDDEN
 
-    def test_duplicate_submission_by_anotheruser_shared_add_and_change_allowed(self):
-        # user needs both `add_submissions` and `change_submissions` permissions
-        for perm in [PERM_CHANGE_SUBMISSIONS, PERM_ADD_SUBMISSIONS]:
-            self.asset.assign_perm(self.anotheruser, perm)
-        self._log_in_as_another_user()
-        response = self.client.post(self.submission_url, {'format': 'json'})
-        assert response.status_code == status.HTTP_201_CREATED
-        self._check_duplicate(response)
-
 
 class BulkUpdateSubmissionsApiTests(BaseSubmissionTestCase):
 

kpi/tests/test_permissions.py

@@ -1,4 +1,5 @@
 # coding: utf-8
+import unittest
 from django.contrib.auth.models import User, AnonymousUser
 from django.test import TestCase
 
@@ -256,7 +257,11 @@ def test_implied_asset_grant_permissions(self):
             PERM_CHANGE_ASSET: (PERM_VIEW_ASSET,),
             PERM_ADD_SUBMISSIONS: (PERM_VIEW_ASSET,),
             PERM_VIEW_SUBMISSIONS: (PERM_VIEW_ASSET,),
-            PERM_CHANGE_SUBMISSIONS: (PERM_VIEW_ASSET, PERM_VIEW_SUBMISSIONS),
+            PERM_CHANGE_SUBMISSIONS: (
+                PERM_VIEW_ASSET,
+                PERM_VIEW_SUBMISSIONS,
+                PERM_ADD_SUBMISSIONS,
+            ),
             PERM_VALIDATE_SUBMISSIONS: (PERM_VIEW_ASSET, PERM_VIEW_SUBMISSIONS),
         }
         asset = self.admin_asset
@@ -293,6 +298,7 @@ def test_remove_implied_asset_permissions(self):
             PERM_VIEW_ASSET,
             PERM_VIEW_SUBMISSIONS,
             PERM_CHANGE_SUBMISSIONS,
+            PERM_ADD_SUBMISSIONS,
         ]
         self.assertListEqual(
             sorted(asset.get_perms(grantee)), sorted(expected_perms)
@@ -394,7 +400,7 @@ def test_contradict_implied_asset_deny_permissions(self):
         )
         expected_perms = [
             # codename, deny
-            (PERM_ADD_SUBMISSIONS, True),
+            (PERM_ADD_SUBMISSIONS, False),
             (PERM_CHANGE_ASSET, True),
             (PERM_CHANGE_SUBMISSIONS, False),
             (PERM_DELETE_SUBMISSIONS, True),
@@ -473,6 +479,7 @@ def test_calculated_submission_editor_permissions(self):
         grantee = self.someuser
         asset = self.admin_asset
         submission_editor_permissions = [
+            PERM_ADD_SUBMISSIONS,
             PERM_CHANGE_SUBMISSIONS,
             PERM_VIEW_ASSET,
             PERM_VIEW_SUBMISSIONS,
@@ -715,6 +722,8 @@ def test_remove_partial_submission_permission(self):
         self.assertFalse(grantee.has_perm(PERM_PARTIAL_SUBMISSIONS, asset))
         self.assertTrue(asset.asset_partial_permissions.count() == 0)
 
+    @unittest.skip(reason='Skip until this branch is merged within '
+                          '`3115-allowed-write-actions-with-partial-perm`')
     def test_implied_partial_submission_permission(self):
         """
         This test is present even though we can only restrict users to view