From 1c0c2352fb2a9730f6582cbd48dcca4d823492d5 Mon Sep 17 00:00:00 2001 From: mike Date: Sat, 15 Aug 2020 23:46:13 -0700 Subject: [PATCH 01/10] Add check to resource_modification for orphaned langs/cats --- app/api/routes/resource_modification.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/api/routes/resource_modification.py b/app/api/routes/resource_modification.py index 5c359ffe..2d753a2d 100644 --- a/app/api/routes/resource_modification.py +++ b/app/api/routes/resource_modification.py @@ -10,7 +10,7 @@ from app.api.routes.helpers import ( failures_counter, get_attributes, latency_summary, logger, ensure_bool) from app.api.validations import requires_body, validate_resource, wrong_type -from app.models import Resource, VoteInformation, Key +from app.models import Resource, VoteInformation, Key, Language, Category import json as json_module @@ -46,11 +46,19 @@ def update_resource(id, json, db): logger.info( f"Updating resource. Old data: {json_module.dumps(resource.serialize)}") if json.get('languages'): + old_languages = resource.languages[:] resource.languages = langs index_object['languages'] = resource.serialize['languages'] + for language in old_languages: + if not Resource.query.filter_by(languages=language).all(): + Language.query.filter_by(name=language).delete() if json.get('category'): + old_categories = resource.category[:] resource.category = categ index_object['category'] = categ.name + for category in old_categories: + if not Resource.query.filter_by(categories=category).all(): + Category.query.filter_by(name=category).delete() if json.get('name'): resource.name = json.get('name') index_object['name'] = json.get('name') From bd6fb5b7ffb6e2b1e15c33a647e4123912943ef4 Mon Sep 17 00:00:00 2001 From: mike Date: Sat, 15 Aug 2020 23:46:13 -0700 Subject: [PATCH 02/10] Add check to resource_modification for orphaned langs/cats --- app/api/routes/resource_modification.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/api/routes/resource_modification.py b/app/api/routes/resource_modification.py index 5c359ffe..2d753a2d 100644 --- a/app/api/routes/resource_modification.py +++ b/app/api/routes/resource_modification.py @@ -10,7 +10,7 @@ from app.api.routes.helpers import ( failures_counter, get_attributes, latency_summary, logger, ensure_bool) from app.api.validations import requires_body, validate_resource, wrong_type -from app.models import Resource, VoteInformation, Key +from app.models import Resource, VoteInformation, Key, Language, Category import json as json_module @@ -46,11 +46,19 @@ def update_resource(id, json, db): logger.info( f"Updating resource. Old data: {json_module.dumps(resource.serialize)}") if json.get('languages'): + old_languages = resource.languages[:] resource.languages = langs index_object['languages'] = resource.serialize['languages'] + for language in old_languages: + if not Resource.query.filter_by(languages=language).all(): + Language.query.filter_by(name=language).delete() if json.get('category'): + old_categories = resource.category[:] resource.category = categ index_object['category'] = categ.name + for category in old_categories: + if not Resource.query.filter_by(categories=category).all(): + Category.query.filter_by(name=category).delete() if json.get('name'): resource.name = json.get('name') index_object['name'] = json.get('name') From 1fcc26e244c0408005e7b3daaa50710dc243b2b6 Mon Sep 17 00:00:00 2001 From: mike Date: Sun, 27 Sep 2020 21:49:13 -0700 Subject: [PATCH 03/10] Add check for orphaned categories. This is just a test of the logic --- app/api/routes/resource_modification.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/app/api/routes/resource_modification.py b/app/api/routes/resource_modification.py index 2d753a2d..801a4e92 100644 --- a/app/api/routes/resource_modification.py +++ b/app/api/routes/resource_modification.py @@ -41,24 +41,31 @@ def update_resource(id, json, db): langs, categ = get_attributes(json) index_object = {'objectID': id} + def get_all_resource_categories_as_strings(): + resources = Resource.query.all() + resource_categories = {resource.category.name for resource in resources} + return resource_categories try: logger.info( f"Updating resource. Old data: {json_module.dumps(resource.serialize)}") if json.get('languages'): - old_languages = resource.languages[:] resource.languages = langs index_object['languages'] = resource.serialize['languages'] - for language in old_languages: - if not Resource.query.filter_by(languages=language).all(): - Language.query.filter_by(name=language).delete() if json.get('category'): - old_categories = resource.category[:] + old_category = resource.category #I'm assuming this is the category name as a string + resource_categories = get_all_resource_categories_as_strings() resource.category = categ index_object['category'] = categ.name - for category in old_categories: - if not Resource.query.filter_by(categories=category).all(): - Category.query.filter_by(name=category).delete() + # Check if the old category is being used anywhere else + # in the Resources table + orphaned_category = Category.query \ + .filter(Category.name.notin_(resource_categories)) \ + .filter(Category.name == old_category) \ + .first() + # If not, delete it from the Category table + if orphaned_category: + orphaned_category.delete() if json.get('name'): resource.name = json.get('name') index_object['name'] = json.get('name') From c5ae5062c51680c376368f8dd2a8f465c0ccc47e Mon Sep 17 00:00:00 2001 From: mike Date: Mon, 28 Sep 2020 22:24:52 -0700 Subject: [PATCH 04/10] Simplify logic and implement orphaned language check --- app/api/routes/resource_modification.py | 32 ++++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/app/api/routes/resource_modification.py b/app/api/routes/resource_modification.py index 801a4e92..efe1177c 100644 --- a/app/api/routes/resource_modification.py +++ b/app/api/routes/resource_modification.py @@ -41,31 +41,35 @@ def update_resource(id, json, db): langs, categ = get_attributes(json) index_object = {'objectID': id} - def get_all_resource_categories_as_strings(): + + def get_unique_resource_categories_as_strings(): + resources = Resource.query.all() + return {resource.category.name for resource in resources} + + def get_unique_resource_languages_as_strings(): resources = Resource.query.all() - resource_categories = {resource.category.name for resource in resources} - return resource_categories + return {language.name + for resource in resources + for language in resource.languages} try: logger.info( f"Updating resource. Old data: {json_module.dumps(resource.serialize)}") if json.get('languages'): + old_languages = resource.languages[:] resource.languages = langs index_object['languages'] = resource.serialize['languages'] + resource_languages = get_unique_resource_languages_as_strings() + for language in old_languages: + if language.name not in resource_languages: + db.session.delete(language) if json.get('category'): - old_category = resource.category #I'm assuming this is the category name as a string - resource_categories = get_all_resource_categories_as_strings() + old_category = resource.category resource.category = categ index_object['category'] = categ.name - # Check if the old category is being used anywhere else - # in the Resources table - orphaned_category = Category.query \ - .filter(Category.name.notin_(resource_categories)) \ - .filter(Category.name == old_category) \ - .first() - # If not, delete it from the Category table - if orphaned_category: - orphaned_category.delete() + resource_categories = get_unique_resource_categories_as_strings() + if old_category.name not in resource_categories: + db.session.delete(old_category) if json.get('name'): resource.name = json.get('name') index_object['name'] = json.get('name') From 01080a417833325bb0160953c1bcb536a30a63fe Mon Sep 17 00:00:00 2001 From: mike Date: Mon, 28 Sep 2020 22:34:02 -0700 Subject: [PATCH 05/10] Remove unused imports for Category and Language classes --- app/api/routes/resource_modification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/api/routes/resource_modification.py b/app/api/routes/resource_modification.py index efe1177c..0305fe3c 100644 --- a/app/api/routes/resource_modification.py +++ b/app/api/routes/resource_modification.py @@ -10,7 +10,7 @@ from app.api.routes.helpers import ( failures_counter, get_attributes, latency_summary, logger, ensure_bool) from app.api.validations import requires_body, validate_resource, wrong_type -from app.models import Resource, VoteInformation, Key, Language, Category +from app.models import Resource, VoteInformation, Key import json as json_module From 4b45e80d3e37ca4b07c83e3a8c0062e244c2aca2 Mon Sep 17 00:00:00 2001 From: Lakshya Singh Date: Tue, 20 Oct 2020 10:50:57 +0530 Subject: [PATCH 06/10] test for checking deletion of orphaned langs random name for language use to prevent collision --- .../unit/test_routes/test_resource_update.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/unit/test_routes/test_resource_update.py b/tests/unit/test_routes/test_resource_update.py index 50fddc98..8939a620 100644 --- a/tests/unit/test_routes/test_resource_update.py +++ b/tests/unit/test_routes/test_resource_update.py @@ -247,3 +247,57 @@ def test_update_resource( follow_redirects=True ) assert_correct_response(response, 404) + + +def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fake_algolia_save): + client = module_client + apikey = get_api_key(client) + + # Happy Path + response = update_resource(client, apikey) + assert (response.status_code == 200) + assert (response.json['resource'].get('name') == "New name") + + #Initial Data + name = "Language Test" + url = None + category = None + #Random Language DS/AI + languages = ["Python", "DS/AI"] + paid = None + notes = None + + #Update response + response = update_resource(client, + apikey, + name, + url, + category, + languages, + paid, + notes) + #Check update + assert (response.status_code == 200) + assert(response.json['resource'].get('name') == "Language Test") + languages_response = response.json['resource'].get('languages') + assert("Python" in languages_response) + assert("DS/AI" in languages_response) + + #Update Languages split remove DS/AI and add JavaScriptmake + languages.append("JavaScript") + languages.append("HTML") + languages.remove("DS/AI") + response = update_resource(client, apikey, name, url, + category, languages, paid, notes) + #Check Update of Languages + assert(response.status_code == 200) + languages_response = response.json['resource'].get('languages') + assert("Python" in languages_response) + assert("JavaScript" in languages_response) + assert("HTML" in languages_response) + assert("DS/AI" not in languages_response) + + db_languages = client.get('api/v1/languages') + db_languages = [language.get('name') for language in db_languages.json['languages']] + + assert("DS/AI" not in db_languages) From 8aee27339e5e58860eb82b45a4261983c9496ecf Mon Sep 17 00:00:00 2001 From: mike Date: Tue, 20 Oct 2020 21:50:05 -0700 Subject: [PATCH 07/10] Cleaning out the lint trap --- tests/unit/test_routes/test_resource_update.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_routes/test_resource_update.py b/tests/unit/test_routes/test_resource_update.py index cc83f69d..78198644 100644 --- a/tests/unit/test_routes/test_resource_update.py +++ b/tests/unit/test_routes/test_resource_update.py @@ -3,8 +3,6 @@ assert_correct_response ) -from ..test_auth_jwt import GOOD_AUTH - def test_update_votes(module_client, module_db, fake_auth_from_oc, fake_algolia_save): client = module_client @@ -255,7 +253,9 @@ def test_update_resource( ) assert_correct_response(response, 404) -def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fake_algolia_save): + +def test_delete_unused_languages(module_client, module_db, + fake_auth_from_oc, fake_algolia_save): client = module_client apikey = get_api_key(client) @@ -264,16 +264,16 @@ def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fa assert (response.status_code == 200) assert (response.json['resource'].get('name') == "New name") - #Initial Data + # Initial Data name = "Language Test" url = None category = None - #Random Language DS/AI + # Random Language DS/AI languages = ["Python", "DS/AI"] paid = None notes = None - #Update response + # Update response response = update_resource(client, apikey, name, @@ -282,20 +282,20 @@ def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fa languages, paid, notes) - #Check update + # Check update assert (response.status_code == 200) assert(response.json['resource'].get('name') == "Language Test") languages_response = response.json['resource'].get('languages') assert("Python" in languages_response) assert("DS/AI" in languages_response) - #Update Languages split remove DS/AI and add JavaScriptmake + # Update Languages split remove DS/AI and add JavaScriptmake languages.append("JavaScript") languages.append("HTML") languages.remove("DS/AI") response = update_resource(client, apikey, name, url, category, languages, paid, notes) - #Check Update of Languages + # Check Update of Languages assert(response.status_code == 200) languages_response = response.json['resource'].get('languages') assert("Python" in languages_response) From 754a8e6c76296f1c6911dece919a7810b256d3ac Mon Sep 17 00:00:00 2001 From: Lakshya Singh Date: Tue, 20 Oct 2020 10:50:57 +0530 Subject: [PATCH 08/10] Fix merge errors on previous commit test for checking deletion of orphaned langs random name for language use to prevent collision --- .../unit/test_routes/test_resource_update.py | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_routes/test_resource_update.py b/tests/unit/test_routes/test_resource_update.py index 78198644..6516f5b0 100644 --- a/tests/unit/test_routes/test_resource_update.py +++ b/tests/unit/test_routes/test_resource_update.py @@ -254,8 +254,35 @@ def test_update_resource( assert_correct_response(response, 404) -def test_delete_unused_languages(module_client, module_db, - fake_auth_from_oc, fake_algolia_save): +def test_update_votes_authorization_header( + module_client, module_db, fake_auth_from_oc, fake_algolia_save): + client = module_client + id = 1 + UPVOTE = 'upvote' + DOWNVOTE = 'downvote' + + data = client.get(f"api/v1/resources/{id}").json['resource'] + response = client.put( + f"/api/v1/resources/{id}/upvote", + follow_redirects=True, + headers={'authorization': GOOD_AUTH}) + initial_upvotes = data.get(f"{UPVOTE}s") + initial_downvotes = data.get(f"{DOWNVOTE}s") + + assert (response.status_code == 200) + assert (response.json['resource'].get(f"{UPVOTE}s") == initial_upvotes + 1) + + response = client.put( + f"/api/v1/resources/{id}/downvote", + follow_redirects=True, + headers={'authorization': GOOD_AUTH}) + + assert (response.status_code == 200) + assert (response.json['resource'].get(f"{UPVOTE}s") == initial_upvotes) + assert (response.json['resource'].get(f"{DOWNVOTE}s") == initial_downvotes + 1) + + +def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fake_algolia_save): client = module_client apikey = get_api_key(client) @@ -264,16 +291,16 @@ def test_delete_unused_languages(module_client, module_db, assert (response.status_code == 200) assert (response.json['resource'].get('name') == "New name") - # Initial Data + #Initial Data name = "Language Test" url = None category = None - # Random Language DS/AI + #Random Language DS/AI languages = ["Python", "DS/AI"] paid = None notes = None - # Update response + #Update response response = update_resource(client, apikey, name, @@ -282,20 +309,20 @@ def test_delete_unused_languages(module_client, module_db, languages, paid, notes) - # Check update + #Check update assert (response.status_code == 200) assert(response.json['resource'].get('name') == "Language Test") languages_response = response.json['resource'].get('languages') assert("Python" in languages_response) assert("DS/AI" in languages_response) - # Update Languages split remove DS/AI and add JavaScriptmake + #Update Languages split remove DS/AI and add JavaScriptmake languages.append("JavaScript") languages.append("HTML") languages.remove("DS/AI") response = update_resource(client, apikey, name, url, category, languages, paid, notes) - # Check Update of Languages + #Check Update of Languages assert(response.status_code == 200) languages_response = response.json['resource'].get('languages') assert("Python" in languages_response) From b9a38a49c6ffc42bab0246d74e8f38eb561328e1 Mon Sep 17 00:00:00 2001 From: mike Date: Wed, 21 Oct 2020 19:07:41 -0700 Subject: [PATCH 09/10] Fix lint errors and some other stuff --- tests/unit/test_routes/test_resource_update.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_routes/test_resource_update.py b/tests/unit/test_routes/test_resource_update.py index 6516f5b0..8f50325c 100644 --- a/tests/unit/test_routes/test_resource_update.py +++ b/tests/unit/test_routes/test_resource_update.py @@ -3,6 +3,8 @@ assert_correct_response ) +from ..test_auth_jwt import GOOD_AUTH + def test_update_votes(module_client, module_db, fake_auth_from_oc, fake_algolia_save): client = module_client @@ -282,7 +284,8 @@ def test_update_votes_authorization_header( assert (response.json['resource'].get(f"{DOWNVOTE}s") == initial_downvotes + 1) -def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fake_algolia_save): +def test_delete_unused_languages(module_client, module_db, + fake_auth_from_oc, fake_algolia_save): client = module_client apikey = get_api_key(client) @@ -291,16 +294,16 @@ def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fa assert (response.status_code == 200) assert (response.json['resource'].get('name') == "New name") - #Initial Data + # Initial Data name = "Language Test" url = None category = None - #Random Language DS/AI + # Random Language DS/AI languages = ["Python", "DS/AI"] paid = None notes = None - #Update response + # Update response response = update_resource(client, apikey, name, @@ -309,20 +312,20 @@ def test_delete_unused_languages(module_client, module_db, fake_auth_from_oc, fa languages, paid, notes) - #Check update + # Check update assert (response.status_code == 200) assert(response.json['resource'].get('name') == "Language Test") languages_response = response.json['resource'].get('languages') assert("Python" in languages_response) assert("DS/AI" in languages_response) - #Update Languages split remove DS/AI and add JavaScriptmake + # Update Languages split remove DS/AI and add JavaScriptmake languages.append("JavaScript") languages.append("HTML") languages.remove("DS/AI") response = update_resource(client, apikey, name, url, category, languages, paid, notes) - #Check Update of Languages + # Check Update of Languages assert(response.status_code == 200) languages_response = response.json['resource'].get('languages') assert("Python" in languages_response) From 14638f7d34491a225c6a3d8f19453ea5bc45ea23 Mon Sep 17 00:00:00 2001 From: mike Date: Wed, 21 Oct 2020 21:38:49 -0700 Subject: [PATCH 10/10] Add test for unused categories --- .../unit/test_routes/test_resource_update.py | 75 ++++++++++++++++--- 1 file changed, 65 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_routes/test_resource_update.py b/tests/unit/test_routes/test_resource_update.py index 8f50325c..ef980b06 100644 --- a/tests/unit/test_routes/test_resource_update.py +++ b/tests/unit/test_routes/test_resource_update.py @@ -314,26 +314,81 @@ def test_delete_unused_languages(module_client, module_db, notes) # Check update assert (response.status_code == 200) - assert(response.json['resource'].get('name') == "Language Test") + assert (response.json['resource'].get('name') == "Language Test") languages_response = response.json['resource'].get('languages') - assert("Python" in languages_response) - assert("DS/AI" in languages_response) + assert ("Python" in languages_response) + assert ("DS/AI" in languages_response) - # Update Languages split remove DS/AI and add JavaScriptmake + # Update Languages remove DS/AI and add JavaScript and HTML languages.append("JavaScript") languages.append("HTML") languages.remove("DS/AI") response = update_resource(client, apikey, name, url, category, languages, paid, notes) + # Check Update of Languages - assert(response.status_code == 200) + assert (response.status_code == 200) languages_response = response.json['resource'].get('languages') - assert("Python" in languages_response) - assert("JavaScript" in languages_response) - assert("HTML" in languages_response) - assert("DS/AI" not in languages_response) + assert ("Python" in languages_response) + assert ("JavaScript" in languages_response) + assert ("HTML" in languages_response) + assert ("DS/AI" not in languages_response) db_languages = client.get('api/v1/languages') db_languages = [language.get('name') for language in db_languages.json['languages']] - assert("DS/AI" not in db_languages) + assert ("DS/AI" not in db_languages) + + +def test_delete_unused_categories(module_client, module_db, + fake_auth_from_oc, fake_algolia_save): + client = module_client + apikey = get_api_key(client) + + # Happy Path + response = update_resource(client, apikey) + assert (response.status_code == 200) + assert (response.json['resource'].get('name') == "New name") + + # Test Categories + test_cat_1 = 'Holy hand grenades' + test_cat_2 = 'News' + + # Initial Data + name = "Category Test" + url = None + category = test_cat_1 + languages = None + paid = None + notes = None + + # Update response + response = update_resource(client, + apikey, + name, + url, + category, + languages, + paid, + notes) + # Check update + assert (response.status_code == 200) + assert (response.json['resource'].get('name') == "Category Test") + category_response = response.json['resource'].get('category') + assert (category_response == test_cat_1) + + # Update category to something else + category = test_cat_2 + response = update_resource(client, apikey, name, url, + category, languages, paid, notes) + + # Check Update of category + assert (response.status_code == 200) + category_response = response.json['resource'].get('category') + assert (category_response == test_cat_2) + + db_categories = client.get('api/v1/categories') + db_categories = [category.get('name') + for category in db_categories.json['categories']] + + assert (test_cat_1 not in db_categories)