address PR comments

Author: schen1870

pybossa/api/project.py

@@ -33,6 +33,7 @@
 from pybossa.util import is_reserved_name, description_from_long_description, validate_ownership_id
 from pybossa.core import auditlog_repo, result_repo, http_signer
 from pybossa.auditlogger import AuditLogger
+from pybossa.messages import DEPRECATED_PRODUCT_SUBPRODUCT_WARNING
 from pybossa.data_access import ensure_user_assignment_to_project, set_default_amp_store
 from sqlalchemy.orm.base import _entity_descriptor
 from pybossa.cache import delete_memoized
@@ -262,10 +263,6 @@ def _generate_product_subproduct_warnings(self, product, subproduct):
         warnings = []
 
         if product and self._is_product_or_subproduct_deprecated(product, subproduct):
-            warnings.append(
-                'Combination of selected Product and Subproduct has been deprecated '
-                'and will be removed in future. Refer to GIGwork documentation for '
-                'taxonomy updates.'
-            )
+            warnings.append(DEPRECATED_PRODUCT_SUBPRODUCT_WARNING)
 
         return warnings

pybossa/messages.py

@@ -34,7 +34,8 @@
 from werkzeug.exceptions import NotFound, BadRequest
 
 __all__ = ['SUCCESS', 'ERROR', 'WARNING', 'FORBIDDEN', 'NOTFOUND', 'BADREQUEST',
-           'INFO', 'FAILED', 'NOTFOUND', 'INTERNALSERVERERROR', 'UNAUTHORIZED', 'LOCKED']
+           'INFO', 'FAILED', 'NOTFOUND', 'INTERNALSERVERERROR', 'UNAUTHORIZED', 'LOCKED',
+           'DEPRECATED_PRODUCT_SUBPRODUCT_WARNING']
 
 SUCCESS = "success"
 
@@ -58,6 +59,10 @@
 
 UNAUTHORIZED = Unauthorized.description
 
+DEPRECATED_PRODUCT_SUBPRODUCT_WARNING = ('This combination of selected Product and Subproduct has been deprecated '
+                                         'and will be removed in future. Refer to GIGwork documentation for '
+                                         'taxonomy updates.')
+
 assert SUCCESS
 assert ERROR
 assert WARNING

pybossa/view/projects.py

@@ -83,6 +83,7 @@
 from pybossa.forms.admin_view_forms import SearchForm
 from pybossa.importers import BulkImportException
 from pybossa.pro_features import ProFeatureHandler
+from pybossa.messages import DEPRECATED_PRODUCT_SUBPRODUCT_WARNING
 
 from pybossa.core import (project_repo, user_repo, task_repo, blog_repo,
                           result_repo, webhook_repo, auditlog_repo,
@@ -114,9 +115,6 @@
 
 cors_headers = ['Content-Type', 'Authorization']
 
-# Warning message for deprecated products/subproducts
-DEPRECATED_PRODUCT_SUBPRODUCT_WARNING = 'Combination of selected Product and Subproduct has been deprecated and will be removed in future. Refer to GIGwork documentation for taxonomy updates.'
-
 blueprint = Blueprint('project', __name__)
 blueprint_projectid = Blueprint('projectid', __name__)
 

test/test_api/test_project_warnings_api.py

@@ -66,15 +66,12 @@
 from test.factories import UserFactory, CategoryFactory, ProjectFactory
 from test.test_api import TestAPI
 from pybossa.core import project_repo
+from pybossa.messages import DEPRECATED_PRODUCT_SUBPRODUCT_WARNING
 
 
 class TestProjectWarningsAPI(TestAPI):
     """Test Project API warnings functionality."""
 
-    DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE = ('Combination of selected Product and Subproduct has been deprecated '
-                'and will be removed in future. Refer to GIGwork documentation for '
-                'taxonomy updates.')
-
     @with_context
     def test_post_valid_product_with_deprecated_subproduct_shows_warning(self):
         """Test POST with valid product but deprecated subproduct shows warning."""
@@ -119,7 +116,7 @@ def test_post_valid_product_with_deprecated_subproduct_shows_warning(self):
             # Verify warning is present when using valid product with deprecated subproduct
             assert 'warnings' in response_data
             assert len(response_data['warnings']) == 1
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE == response_data['warnings'][0]
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING == response_data['warnings'][0]
 
             # Verify project was saved to database
             project = project_repo.get(response_data['id'])
@@ -227,7 +224,7 @@ def test_post_deprecated_product_with_subproduct_shows_warning(self):
             # Verify warning is present when using deprecated product
             assert 'warnings' in response_data
             assert len(response_data['warnings']) == 1
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE == response_data['warnings'][0]
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING == response_data['warnings'][0]
 
             # Verify project was saved to database
             project = project_repo.get(response_data['id'])
@@ -353,7 +350,7 @@ def test_put_valid_product_with_deprecated_subproduct_shows_warning(self):
             # Verify warning is present when using valid product with deprecated subproduct
             assert 'warnings' in response_data
             assert len(response_data['warnings']) == 1
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE == response_data['warnings'][0]
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING == response_data['warnings'][0]
 
             # Verify project was updated in database
             updated_project = project_repo.get(project.id)
@@ -416,7 +413,7 @@ def test_put_deprecated_product_with_subproduct_shows_warning(self):
             # Verify warning is present when using deprecated product
             assert 'warnings' in response_data
             assert len(response_data['warnings']) == 1
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE == response_data['warnings'][0]
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING == response_data['warnings'][0]
 
             # Verify project was updated in database
             updated_project = project_repo.get(project.id)
@@ -496,7 +493,7 @@ def test_project_warnings_not_persisted_after_multiple_operations(self):
 
             # Verify PUT response has different warnings
             assert 'warnings' in put_response
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE in put_response['warnings']
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING in put_response['warnings']
 
             # Check database after PUT
             project = project_repo.get(project_id)
@@ -579,7 +576,7 @@ def test_customize_response_dict_handles_warnings_dynamically(self):
             # Should generate warnings dynamically
             assert 'warnings' in result
             assert len(result['warnings']) == 1
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE == result['warnings'][0]
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING == result['warnings'][0]
 
             # Other fields should remain unchanged
             assert result['id'] == 1
@@ -708,7 +705,7 @@ def test_warnings_survive_api_base_response_processing(self):
             # Verify warnings are properly formatted
             assert isinstance(response_data['warnings'], list)
             assert len(response_data['warnings']) == 1
-            assert self.DEPRECATED_PRODUCT_SUBPRODUCT_WARNING_MESSAGE == response_data['warnings'][0]
+            assert DEPRECATED_PRODUCT_SUBPRODUCT_WARNING == response_data['warnings'][0]
 
             # Verify info field integrity
             assert response_data['info']['product'] == 'Old Product'
@@ -763,7 +760,7 @@ def test_post_invalid_product_raises_bad_request_no_warnings(self):
             url = '/api/project?api_key=%s' % user.api_key
             res = self.app.post(url, data=json.dumps(project_data))
 
-            # Should fail with 400 error due to ValueError during response generation
+            # Should fail with 400 error when product/subproduct value is validated at repository layer
             assert res.status_code == 400, res.data
             response_data = json.loads(res.data)
             assert 'exception_msg' in response_data
@@ -808,7 +805,7 @@ def test_post_invalid_subproduct_raises_bad_request_no_warnings(self):
             url = '/api/project?api_key=%s' % user.api_key
             res = self.app.post(url, data=json.dumps(project_data))
 
-            # Should fail with 400 error due to ValueError during response generation
+            # Should fail with 400 error when product/subproduct value is validated at repository layer
             assert res.status_code == 400, res.data
             response_data = json.loads(res.data)
             assert 'exception_msg' in response_data
@@ -861,11 +858,12 @@ def test_put_invalid_product_raises_bad_request_no_warnings(self):
             url = '/api/project/%s?api_key=%s' % (project.id, user.api_key)
             res = self.app.put(url, data=json.dumps(update_data))
 
-            # Should fail with 400 error due to ValueError during response generation
+            # Should fail with 400 error when product/subproduct value is validated at repository layer
             assert res.status_code == 400, res.data
             response_data = json.loads(res.data)
             assert 'exception_msg' in response_data
             assert 'Invalid product' in response_data['exception_msg']
+            # check that invalid product is caught before warning generation check
             assert 'warnings' not in response_data
 
     @with_context
@@ -913,10 +911,11 @@ def test_put_invalid_subproduct_raises_bad_request_no_warnings(self):
             url = '/api/project/%s?api_key=%s' % (project.id, user.api_key)
             res = self.app.put(url, data=json.dumps(update_data))
 
-            # Should fail with 400 error due to ValueError during response generation
+            # Should fail with 400 error when product/subproduct value is validated at repository layer
             assert res.status_code == 400, res.data
             response_data = json.loads(res.data)
             assert 'exception_msg' in response_data
             assert 'Invalid subproduct' in response_data['exception_msg']
+            # check that invalid subproduct is caught before warning generation check
             assert 'warnings' not in response_data