From ee913032c30f45876c490c8fdaca769b09dc62be Mon Sep 17 00:00:00 2001 From: Stefanos Date: Fri, 28 Aug 2020 16:21:06 +0300 Subject: [PATCH 01/22] feat: parse null multivalue as empty string --- OIPA/solr/activity/indexing.py | 386 ++++++++++++++++++++++----------- 1 file changed, 261 insertions(+), 125 deletions(-) diff --git a/OIPA/solr/activity/indexing.py b/OIPA/solr/activity/indexing.py index 1f0c09c0b..21674a6c1 100644 --- a/OIPA/solr/activity/indexing.py +++ b/OIPA/solr/activity/indexing.py @@ -38,7 +38,6 @@ get_narrative_lang_list, value_string ) - class ActivityIndexing(BaseIndexing): def dataset(self): @@ -139,7 +138,13 @@ def description(self): 'description_xml', DescriptionReference(description=description).to_string() ) - self.add_value_list('description_type', description.type_id) + if description.type_id: + self.add_value_list( + 'description_type', + description.type_id + ) + else: + self.indexing['description_type'].append(' ') self.related_narrative( description, @@ -177,14 +182,20 @@ def participating_org(self): ).to_string() ) - self.add_value_list( - 'participating_org_ref', - participating_organisation.ref - ) - self.add_value_list( - 'participating_org_type', - participating_organisation.type_id - ) + if participating_organisation.ref: + self.add_value_list( + 'participating_org_ref', + participating_organisation.ref + ) + else: + self.indexing['participating_org_ref'].append(' ') + if participating_organisation.type_id: + self.add_value_list( + 'participating_org_type', + participating_organisation.type_id + ) + else: + self.indexing['participating_org_type'].append(' ') self.add_value_list( 'participating_org_role', participating_organisation.role_id @@ -369,7 +380,13 @@ def contact_info(self): ).to_string() ) - self.add_value_list('contact_info_type', contact_info.type_id) + if contact_info.type_id: + self.add_value_list( + 'contact_info_type', + contact_info.type_id + ) + else: + self.indexing['contact_info_type'].append(' ') self.add_value_list( 'contact_info_telephone', contact_info.telephone @@ -499,14 +516,20 @@ def recipient_region(self): 'recipient_region_name', recipient_region.region.name ) - self.add_value_list( - 'recipient_region_vocabulary', - recipient_region.vocabulary.code - ) - self.add_value_list( - 'recipient_region_vocabulary_uri', - recipient_region.vocabulary_uri - ) + if recipient_region.vocabulary.code: + self.add_value_list( + 'recipient_region_vocabulary', + recipient_region.vocabulary.code + ) + else: + self.indexing['recipient_region_vocabulary'].append(' ') + if recipient_region.vocabulary_uri: + self.add_value_list( + 'recipient_region_vocabulary_uri', + recipient_region.vocabulary_uri + ) + else: + self.indexing['recipient_region_vocabulary_uri'].append(' ') self.add_value_list( 'recipient_region_percentage', decimal_string(recipient_region.percentage) @@ -564,7 +587,13 @@ def location(self): ).to_string() ) - self.add_value_list('location_ref', location.ref) + if location.ref: + self.add_value_list( + 'location_ref', + location.ref + ) + else: + self.indexing['location_ref'].append(' ') self.add_value_list( 'location_reach_code', location.location_reach_id @@ -618,10 +647,13 @@ def location(self): 'location_administrative_vocabulary', location_administrative.vocabulary.code ) - self.add_value_list( - 'location_administrative_level', - location_administrative.level - ) + if location_administrative.level: + self.add_value_list( + 'location_administrative_level', + location_administrative.level + ) + else: + self.indexing['location_administrative_level'].append(' ') self.add_value_list( 'location_administrative_code', location_administrative.code @@ -656,14 +688,20 @@ def sector(self): ).to_string() ) - self.add_value_list( - 'sector_vocabulary', - activity_sector.vocabulary_id - ) - self.add_value_list( - 'sector_vocabulary_uri', - activity_sector.vocabulary_uri - ) + if activity_sector.vocabulary_id: + self.add_value_list( + 'sector_vocabulary', + activity_sector.vocabulary_id + ) + else: + self.indexing['sector_vocabulary'].append(' ') + if activity_sector.vocabulary_uri: + self.add_value_list( + 'sector_vocabulary_uri', + activity_sector.vocabulary_uri + ) + else: + self.indexing['sector_vocabulary_uri'].append(' ') self.add_value_list( 'sector_code', activity_sector.sector.code @@ -711,10 +749,13 @@ def tag(self): 'tag_vocabulary', tag.vocabulary_id ) - self.add_value_list( - 'tag_vocabulary_uri', - tag.vocabulary_uri - ) + if tag.vocabulary_uri: + self.add_value_list( + 'tag_vocabulary_uri', + tag.vocabulary_uri + ) + else: + self.indexing['tag_vocabulary_uri'].append(' ') self.add_value_list( 'tag_code', tag.code @@ -821,10 +862,13 @@ def humanitarian_scope(self): 'humanitarian_scope_vocabulary', humanitarian_scope.vocabulary_id ) - self.add_value_list( - 'humanitarian_scope_vocabulary_uri', - humanitarian_scope.vocabulary_uri - ) + if humanitarian_scope.vocabulary_uri: + self.add_value_list( + 'humanitarian_scope_vocabulary_uri', + humanitarian_scope.vocabulary_uri + ) + else: + self.indexing['humanitarian_scope_vocabulary_uri'].append(' ') self.add_value_list( 'humanitarian_scope_code', humanitarian_scope.code @@ -865,15 +909,21 @@ def policy_marker(self): policy_marker=policy_marker ).to_string() ) - - self.add_value_list( - 'policy_marker_vocabulary', - policy_marker.vocabulary_id - ) - self.add_value_list( - 'policy_marker_vocabulary_uri', - policy_marker.vocabulary_uri - ) + + if policy_marker.vocabulary_id: + self.add_value_list( + 'policy_marker_vocabulary', + policy_marker.vocabulary_id + ) + else: + self.indexing['policy_marker_vocabulary'].append(' ') + if policy_marker.vocabulary_uri: + self.add_value_list( + 'policy_marker_vocabulary_uri', + policy_marker.vocabulary_uri + ) + else: + self.indexing['policy_marker_vocabulary_uri'].append(' ') self.add_value_list( 'policy_marker_code', policy_marker.code_id @@ -954,7 +1004,13 @@ def budget(self): BudgetReference(budget=budget).to_string() ) - self.add_value_list('budget_type', budget.type_id) + if budget.type_id: + self.add_value_list( + 'budget_type', + budget.type_id + ) + else: + self.indexing['budget_type'].append(' ') self.add_value_list('budget_status', budget.status_id) self.add_value_list( 'budget_period_start_iso_date', @@ -1035,10 +1091,13 @@ def planned_disbursement(self): ).to_string() ) - self.add_value_list( - 'planned_disbursement_type', - planned_disbursement.type_id - ) + if planned_disbursement.type_id: + self.add_value_list( + 'planned_disbursement_type', + planned_disbursement.type_id + ) + else: + self.indexing['planned_disbursement_type'].append(' ') self.add_value_list( 'planned_disbursement_period_start_iso_date', date_string(planned_disbursement.period_start) @@ -1552,10 +1611,13 @@ def legacy_data(self): 'legacy_data_value', legacy_data.value ) - self.add_value_list( - 'legacy_data_iati_equivalent', - legacy_data.iati_equivalent - ) + if legacy_data.iati_equivalent: + self.add_value_list( + 'legacy_data_iati_equivalent', + legacy_data.iati_equivalent + ) + else: + self.indexing['legacy_data_iati_equivalent'].append(' ') def conditions(self): activity_condition = get_child_attr(self.record, 'conditions') @@ -1829,10 +1891,13 @@ def result(self): ) self.add_value_list('result_type', result.type_id) - self.add_value_list( - 'result_aggregation_status', - bool_string(result.aggregation_status) - ) + if result.aggregation_status: + self.add_value_list( + 'result_aggregation_status', + bool_string(result.aggregation_status) + ) + else: + self.indexing['result_aggregation_status'].append(' ') self.related_narrative( get_child_attr(result, 'resulttitle'), @@ -1906,20 +1971,33 @@ def result(self): 'result_reference_vocabulary_uri', result_reference.vocabulary_uri ) + if result_reference.vocabulary_uri: + self.add_value_list( + 'result_reference_vocabulary_uri', + result_reference.vocabulary_uri + ) + else: + self.indexing['result_reference_vocabulary_uri'].append(' ') for result_indicator in result.resultindicator_set.all(): self.add_value_list( 'result_indicator_measure', result_indicator.measure_id ) - self.add_value_list( - 'result_indicator_ascending', - bool_string(result_indicator.ascending) - ) - self.add_value_list( - 'result_indicator_aggregation_status', - bool_string(result_indicator.aggregation_status) - ) + if result_indicator.ascending: + self.add_value_list( + 'result_indicator_ascending', + bool_string(result_indicator.ascending) + ) + else: + self.indexing['result_indicator_ascending'].append(' ') + if result_indicator.aggregation_status: + self.add_value_list( + 'result_indicator_aggregation_status', + bool_string(result_indicator.aggregation_status) + ) + else: + self.indexing['result_indicator_aggregation_status'].append(' ') self.related_narrative( get_child_attr( @@ -1980,41 +2058,60 @@ def result(self): 'result_indicator_reference_vocabulary', result_indicator_reference.vocabulary_id ) - self.add_value_list( - 'result_indicator_reference_vocabulary_uri', - result_indicator_reference.indicator_uri - ) + if result_indicator_reference.indicator_uri: + self.add_value_list( + 'result_indicator_reference_vocabulary_uri', + result_indicator_reference.indicator_uri + ) + else: + self.indexing['result_indicator_reference_vocabulary_uri'].append(' ') for result_indicator_baseline in result_indicator.resultindicatorbaseline_set.all(): # NOQA: E501 self.add_value_list( 'result_indicator_baseline_year', result_indicator_baseline.year ) - self.add_value_list( - 'result_indicator_baseline_iso_date', - date_string(result_indicator_baseline.iso_date) - ) - self.add_value_list( - 'result_indicator_baseline_value', - result_indicator_baseline.value - ) + if result_indicator_baseline.iso_date: + self.add_value_list( + 'result_indicator_baseline_iso_date', + date_string(result_indicator_baseline.iso_date) + ) + else: + self.indexing['result_indicator_baseline_iso_date'].append(' ') + if result_indicator_baseline.value: + self.add_value_list( + 'result_indicator_baseline_value', + result_indicator_baseline.value + ) + else: + self.indexing['result_indicator_baseline_value'].append(' ') for result_indicator_baseline_location in \ result_indicator_baseline.location_set.all(): - self.add_value_list( - 'result_indicator_baseline_location_ref', - result_indicator_baseline_location.ref - ) + if result_indicator_baseline_location.ref: + self.add_value_list( + 'result_indicator_baseline_location_ref', + result_indicator_baseline_location.ref + ) + else: + self.indexing['result_indicator_baseline_location_ref'].append(' ') + for result_indicator_baseline_dimension in result_indicator_baseline.resultindicatorbaselinedimension_set.all(): # NOQA: E501 - self.add_value_list( - 'result_indicator_baseline_dimension_name', - result_indicator_baseline_dimension.name - ) - self.add_value_list( - 'result_indicator_baseline_dimension_value', - result_indicator_baseline_dimension.value - ) + if result_indicator_baseline_dimension.name: + self.add_value_list( + 'result_indicator_baseline_dimension_name', + result_indicator_baseline_dimension.name + ) + else: + self.indexing['result_indicator_baseline_dimension_value'].append(' ') + if result_indicator_baseline_dimension.value: + self.add_value_list( + 'result_indicator_baseline_dimension_value', + result_indicator_baseline_dimension.value + ) + else: + self.indexing['result_indicator_baseline_dimension_value'].append(' ') self.related_narrative(get_child_attr(result_indicator_baseline, 'resultindicatorbaselinecomment'), 'result_indicator_baseline_comment_narrative', 'result_indicator_baseline_comment_narrative_text', 'result_indicator_baseline_comment_narrative_lang') # NOQA: E501 @@ -2048,17 +2145,38 @@ def result(self): for result_period_target in \ result_period.targets.all(): - self.add_value_list( - 'result_indicator_period_target_value', - result_period_target.value - ) + if result_period_target.value: + self.add_value_list( + 'result_indicator_period_target_value', + result_period_target.value + ) + else: + self.indexing['result_indicator_period_target_value'].append(' ') for result_period_target_location in result_period_target.resultindicatorperiodtargetlocation_set.all(): # NOQA: E501 - self.add_value_list('result_indicator_period_target_location_ref', result_period_target_location.ref) # NOQA: E501 + if result_period_target_location.ref: + self.add_value_list( + 'result_indicator_period_target_location_ref', + result_period_target_location.ref + ) + else: + self.indexing['result_indicator_period_target_location_ref'].append(' ') for result_period_target_dimension in result_period_target.resultindicatorperiodtargetdimension_set.all(): # NOQA: E501 - self.add_value_list('result_indicator_period_target_dimension_name', result_period_target_dimension.name) # NOQA: E501 - self.add_value_list('result_indicator_period_target_dimension_value', result_period_target_dimension.value) # NOQA: E501 + if result_period_target.name: + self.add_value_list( + 'result_indicator_period_target_dimension_name', + result_period_target_dimension.name + ) + else: + self.indexing['result_indicator_period_target_dimension_name'].append(' ') + if result_period_target.value: + self.add_value_list( + 'result_indicator_period_target_dimension_value', + result_period_target_dimension.value + ) + else: + self.indexing['result_indicator_period_target_dimension_value'].append(' ') for result_period_target_comment in result_period_target.resultindicatorperiodtargetcomment_set.all(): # NOQA: E501 self.related_narrative(result_period_target_comment, 'result_indicator_period_target_comment_narrative', 'result_indicator_period_target_comment_narrative_text', 'result_indicator_period_target_comment_narrative_lang') # NOQA: E501 @@ -2111,26 +2229,38 @@ def result(self): for result_period_actual in \ result_period.actuals.all(): - self.add_value_list( - 'result_indicator_period_actual_value', - result_period_actual.value - ) - - for result_period_actual_location in result_period_actual.resultindicatorperiodactuallocation_set.all(): # NOQA: E501 + if result_period_actual.value: self.add_value_list( - 'result_indicator_period_actual_location_ref', # NOQA: E501 - result_period_actual_location.ref + 'result_indicator_period_actual_value', + result_period_actual.value ) + else: + self.indexing['result_indicator_period_actual_value'].append(' ') + + for result_period_actual_location in result_period_actual.resultindicatorperiodactuallocation_set.all(): # NOQA: E501 + if result_period_actual_location.ref: + self.add_value_list( + 'result_indicator_period_actual_location_ref', + result_period_actual_location.ref + ) + else: + self.indexing['result_indicator_period_actual_location_ref'].append(' ') for result_period_actual_dimension in result_period_actual.resultindicatorperiodactualdimension_set.all(): # NOQA: E501 - self.add_value_list( - 'result_indicator_period_actual_dimension_name', # NOQA: E501 - result_period_actual_dimension.name - ) - self.add_value_list( - 'result_indicator_period_actual_dimension_value', # NOQA: E501 - result_period_actual_dimension.value - ) + if result_period_actual_dimension.name: + self.add_value_list( + 'result_indicator_period_actual_dimension_name', + result_period_actual_dimension.name + ) + else: + self.indexing['result_indicator_period_actual_dimension_name'].append(' ') + if result_period_actual_dimension.value: + self.add_value_list( + 'result_indicator_period_actual_dimension_value', + result_period_actual_dimension.value + ) + else: + self.indexing['result_indicator_period_actual_dimension_value'].append(' ') for result_period_actual_comment in result_period_actual.resultindicatorperiodactualcomment_set.all(): # NOQA: E501 self.related_narrative( @@ -2366,14 +2496,20 @@ def fss(self): 'fss_forecast_year', value_string(forecast.year) ) - self.add_value_list( - 'fss_forecast_value_date', - value_string(forecast.value_date) - ) - self.add_value_list( - 'fss_forecast_currency', - forecast.currency_id - ) + if forecast.value_date: + self.add_value_list( + 'fss_forecast_value_date', + value_string(forecast.value_date) + ) + else: + self.indexing['fss_forecast_value_date'].append(' ') + if forecast.currency_id: + self.add_value_list( + 'fss_forecast_currency', + forecast.currency_id + ) + else: + self.indexing['fss_forecast_currency'].append(' ') self.add_value_list( 'fss_forecast_value', value_string(forecast.value) From e790e30dfca07c4edbc48f8f6dc96b22b27e1665 Mon Sep 17 00:00:00 2001 From: Stefanos Date: Mon, 31 Aug 2020 11:51:45 +0300 Subject: [PATCH 02/22] chore: pep8 fixes --- OIPA/solr/activity/indexing.py | 61 ++++++++++++++++++++++------------ OIPA/solr/activity/tasks.py | 2 +- OIPA/solr/transaction/tasks.py | 2 +- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/OIPA/solr/activity/indexing.py b/OIPA/solr/activity/indexing.py index 21674a6c1..7dc295a25 100644 --- a/OIPA/solr/activity/indexing.py +++ b/OIPA/solr/activity/indexing.py @@ -38,6 +38,7 @@ get_narrative_lang_list, value_string ) + class ActivityIndexing(BaseIndexing): def dataset(self): @@ -529,7 +530,8 @@ def recipient_region(self): recipient_region.vocabulary_uri ) else: - self.indexing['recipient_region_vocabulary_uri'].append(' ') + self.indexing['recipient_region_vocabulary_uri'].append( + ' ') self.add_value_list( 'recipient_region_percentage', decimal_string(recipient_region.percentage) @@ -653,7 +655,8 @@ def location(self): location_administrative.level ) else: - self.indexing['location_administrative_level'].append(' ') + self.indexing['location_administrative_level'].append( + ' ') self.add_value_list( 'location_administrative_code', location_administrative.code @@ -868,7 +871,8 @@ def humanitarian_scope(self): humanitarian_scope.vocabulary_uri ) else: - self.indexing['humanitarian_scope_vocabulary_uri'].append(' ') + self.indexing['humanitarian_scope_vocabulary_uri'].append( + ' ') self.add_value_list( 'humanitarian_scope_code', humanitarian_scope.code @@ -909,7 +913,7 @@ def policy_marker(self): policy_marker=policy_marker ).to_string() ) - + if policy_marker.vocabulary_id: self.add_value_list( 'policy_marker_vocabulary', @@ -1977,7 +1981,8 @@ def result(self): result_reference.vocabulary_uri ) else: - self.indexing['result_reference_vocabulary_uri'].append(' ') + self.indexing['result_reference_vocabulary_uri'].append( + ' ') for result_indicator in result.resultindicator_set.all(): self.add_value_list( @@ -1997,7 +2002,8 @@ def result(self): bool_string(result_indicator.aggregation_status) ) else: - self.indexing['result_indicator_aggregation_status'].append(' ') + self.indexing['result_indicator_aggregation_status'].append( + ' ') self.related_narrative( get_child_attr( @@ -2064,7 +2070,8 @@ def result(self): result_indicator_reference.indicator_uri ) else: - self.indexing['result_indicator_reference_vocabulary_uri'].append(' ') + self.indexing['result_indicator_reference_vocabulary_uri'].append( + ' ') for result_indicator_baseline in result_indicator.resultindicatorbaseline_set.all(): # NOQA: E501 self.add_value_list( @@ -2077,14 +2084,16 @@ def result(self): date_string(result_indicator_baseline.iso_date) ) else: - self.indexing['result_indicator_baseline_iso_date'].append(' ') + self.indexing['result_indicator_baseline_iso_date'].append( + ' ') if result_indicator_baseline.value: self.add_value_list( 'result_indicator_baseline_value', result_indicator_baseline.value ) else: - self.indexing['result_indicator_baseline_value'].append(' ') + self.indexing['result_indicator_baseline_value'].append( + ' ') for result_indicator_baseline_location in \ result_indicator_baseline.location_set.all(): @@ -2094,8 +2103,8 @@ def result(self): result_indicator_baseline_location.ref ) else: - self.indexing['result_indicator_baseline_location_ref'].append(' ') - + self.indexing['result_indicator_baseline_location_ref'].append( + ' ') for result_indicator_baseline_dimension in result_indicator_baseline.resultindicatorbaselinedimension_set.all(): # NOQA: E501 if result_indicator_baseline_dimension.name: @@ -2104,14 +2113,16 @@ def result(self): result_indicator_baseline_dimension.name ) else: - self.indexing['result_indicator_baseline_dimension_value'].append(' ') + self.indexing['result_indicator_baseline_dimension_value'].append( + ' ') if result_indicator_baseline_dimension.value: self.add_value_list( 'result_indicator_baseline_dimension_value', result_indicator_baseline_dimension.value ) else: - self.indexing['result_indicator_baseline_dimension_value'].append(' ') + self.indexing['result_indicator_baseline_dimension_value'].append( + ' ') self.related_narrative(get_child_attr(result_indicator_baseline, 'resultindicatorbaselinecomment'), 'result_indicator_baseline_comment_narrative', 'result_indicator_baseline_comment_narrative_text', 'result_indicator_baseline_comment_narrative_lang') # NOQA: E501 @@ -2151,7 +2162,8 @@ def result(self): result_period_target.value ) else: - self.indexing['result_indicator_period_target_value'].append(' ') + self.indexing['result_indicator_period_target_value'].append( + ' ') for result_period_target_location in result_period_target.resultindicatorperiodtargetlocation_set.all(): # NOQA: E501 if result_period_target_location.ref: @@ -2160,7 +2172,8 @@ def result(self): result_period_target_location.ref ) else: - self.indexing['result_indicator_period_target_location_ref'].append(' ') + self.indexing['result_indicator_period_target_location_ref'].append( + ' ') for result_period_target_dimension in result_period_target.resultindicatorperiodtargetdimension_set.all(): # NOQA: E501 if result_period_target.name: @@ -2169,14 +2182,16 @@ def result(self): result_period_target_dimension.name ) else: - self.indexing['result_indicator_period_target_dimension_name'].append(' ') + self.indexing['result_indicator_period_target_dimension_name'].append( + ' ') if result_period_target.value: self.add_value_list( 'result_indicator_period_target_dimension_value', result_period_target_dimension.value ) else: - self.indexing['result_indicator_period_target_dimension_value'].append(' ') + self.indexing['result_indicator_period_target_dimension_value'].append( + ' ') for result_period_target_comment in result_period_target.resultindicatorperiodtargetcomment_set.all(): # NOQA: E501 self.related_narrative(result_period_target_comment, 'result_indicator_period_target_comment_narrative', 'result_indicator_period_target_comment_narrative_text', 'result_indicator_period_target_comment_narrative_lang') # NOQA: E501 @@ -2235,7 +2250,8 @@ def result(self): result_period_actual.value ) else: - self.indexing['result_indicator_period_actual_value'].append(' ') + self.indexing['result_indicator_period_actual_value'].append( + ' ') for result_period_actual_location in result_period_actual.resultindicatorperiodactuallocation_set.all(): # NOQA: E501 if result_period_actual_location.ref: @@ -2244,7 +2260,8 @@ def result(self): result_period_actual_location.ref ) else: - self.indexing['result_indicator_period_actual_location_ref'].append(' ') + self.indexing['result_indicator_period_actual_location_ref'].append( + ' ') for result_period_actual_dimension in result_period_actual.resultindicatorperiodactualdimension_set.all(): # NOQA: E501 if result_period_actual_dimension.name: @@ -2253,14 +2270,16 @@ def result(self): result_period_actual_dimension.name ) else: - self.indexing['result_indicator_period_actual_dimension_name'].append(' ') + self.indexing['result_indicator_period_actual_dimension_name'].append( + ' ') if result_period_actual_dimension.value: self.add_value_list( 'result_indicator_period_actual_dimension_value', result_period_actual_dimension.value ) else: - self.indexing['result_indicator_period_actual_dimension_value'].append(' ') + self.indexing['result_indicator_period_actual_dimension_value'].append( + ' ') for result_period_actual_comment in result_period_actual.resultindicatorperiodactualcomment_set.all(): # NOQA: E501 self.related_narrative( diff --git a/OIPA/solr/activity/tasks.py b/OIPA/solr/activity/tasks.py index 1fc526792..b8f75c64f 100644 --- a/OIPA/solr/activity/tasks.py +++ b/OIPA/solr/activity/tasks.py @@ -6,7 +6,7 @@ from iati.models import Activity from solr.activity.indexing import ActivityIndexing -from solr.activity_sector.tasks import ActivitySectorTaskIndexing +# from solr.activity_sector.tasks import ActivitySectorTaskIndexing from solr.budget.tasks import BudgetTaskIndexing # from solr.result.tasks import ResultTaskIndexing from solr.tasks import BaseTaskIndexing diff --git a/OIPA/solr/transaction/tasks.py b/OIPA/solr/transaction/tasks.py index b7281673b..a54823474 100644 --- a/OIPA/solr/transaction/tasks.py +++ b/OIPA/solr/transaction/tasks.py @@ -7,7 +7,7 @@ from iati.transaction.models import Transaction from solr.tasks import BaseTaskIndexing from solr.transaction.indexing import TransactionIndexing -from solr.transaction_sector.tasks import TransactionSectorTaskIndexing +# from solr.transaction_sector.tasks import TransactionSectorTaskIndexing solr = pysolr.Solr( '{url}/{core}'.format( From ae44dec47b103b661884cbb71c6a33ec648cb354 Mon Sep 17 00:00:00 2001 From: Stefanos Date: Mon, 31 Aug 2020 12:08:34 +0300 Subject: [PATCH 03/22] chore: pep8 fixes --- OIPA/solr/activity/indexing.py | 75 +++++++++++++++------------------- OIPA/solr/transaction/tasks.py | 1 + 2 files changed, 35 insertions(+), 41 deletions(-) diff --git a/OIPA/solr/activity/indexing.py b/OIPA/solr/activity/indexing.py index 7dc295a25..4967e1915 100644 --- a/OIPA/solr/activity/indexing.py +++ b/OIPA/solr/activity/indexing.py @@ -1981,7 +1981,7 @@ def result(self): result_reference.vocabulary_uri ) else: - self.indexing['result_reference_vocabulary_uri'].append( + self.indexing['result_reference_vocabulary_uri'].append( # NOQA: E501 ' ') for result_indicator in result.resultindicator_set.all(): @@ -2002,7 +2002,7 @@ def result(self): bool_string(result_indicator.aggregation_status) ) else: - self.indexing['result_indicator_aggregation_status'].append( + self.indexing['result_indicator_aggregation_status'].append( # NOQA: E501 ' ') self.related_narrative( @@ -2036,7 +2036,7 @@ def result(self): ) self.related_narrative(get_child_attr(result_indicator_document_link, 'documentlinktitle'), 'result_indicator_document_link_title_narrative', 'result_indicator_document_link_title_narrative_text', 'result_indicator_document_link_title_narrative_lang') # NOQA: E501 - self.related_narrative(get_child_attr(result_indicator_document_link, 'documentlinkdescription'), 'result_indicator_document_link_description_narrative', 'result_indicator_document_link_description_narrative_text', 'result_indicator_document_link_description_narrative_lang') # NOQA: E501 + self.related_narrative(get_child_attr(result_indicator_document_link, 'documentlinkdescription'), 'result_indicator_document_link_description_narrative', 'result_indicator_document_link_description_narrative_text', 'result_indicator_document_link_description_narrative_lang') # NOQA: E501 for document_link_category in result_indicator_document_link.documentlinkcategory_set.all(): # NOQA: E501 self.add_value_list( @@ -2070,7 +2070,7 @@ def result(self): result_indicator_reference.indicator_uri ) else: - self.indexing['result_indicator_reference_vocabulary_uri'].append( + self.indexing['result_indicator_reference_vocabulary_uri'].append( # NOQA: E501 ' ') for result_indicator_baseline in result_indicator.resultindicatorbaseline_set.all(): # NOQA: E501 @@ -2084,7 +2084,7 @@ def result(self): date_string(result_indicator_baseline.iso_date) ) else: - self.indexing['result_indicator_baseline_iso_date'].append( + self.indexing['result_indicator_baseline_iso_date'].append( # NOQA: E501 ' ') if result_indicator_baseline.value: self.add_value_list( @@ -2092,7 +2092,7 @@ def result(self): result_indicator_baseline.value ) else: - self.indexing['result_indicator_baseline_value'].append( + self.indexing['result_indicator_baseline_value'].append( # NOQA: E501 ' ') for result_indicator_baseline_location in \ @@ -2103,7 +2103,7 @@ def result(self): result_indicator_baseline_location.ref ) else: - self.indexing['result_indicator_baseline_location_ref'].append( + self.indexing['result_indicator_baseline_location_ref'].append( # NOQA: E501 ' ') for result_indicator_baseline_dimension in result_indicator_baseline.resultindicatorbaselinedimension_set.all(): # NOQA: E501 @@ -2113,15 +2113,14 @@ def result(self): result_indicator_baseline_dimension.name ) else: - self.indexing['result_indicator_baseline_dimension_value'].append( + self.indexing['result_indicator_baseline_dimension_value'].append( # NOQA: E501 ' ') if result_indicator_baseline_dimension.value: self.add_value_list( - 'result_indicator_baseline_dimension_value', - result_indicator_baseline_dimension.value - ) + 'result_indicator_baseline_dimension_value', # NOQA: E501 + result_indicator_baseline_dimension.value) else: - self.indexing['result_indicator_baseline_dimension_value'].append( + self.indexing['result_indicator_baseline_dimension_value'].append( # NOQA: E501 ' ') self.related_narrative(get_child_attr(result_indicator_baseline, 'resultindicatorbaselinecomment'), 'result_indicator_baseline_comment_narrative', 'result_indicator_baseline_comment_narrative_text', 'result_indicator_baseline_comment_narrative_lang') # NOQA: E501 @@ -2131,9 +2130,9 @@ def result(self): 'result_indicator_baseline_document_link_url', result_indicator_baseline_document_link.url ) - self.add_value_list('result_indicator_baseline_document_link_format', result_indicator_baseline_document_link.file_format_id) # NOQA: E501 - self.related_narrative(get_child_attr(result_indicator_baseline_document_link, 'documentlinktitle'), 'result_indicator_baseline_document_link_title', 'result_indicator_baseline_document_link_title_narrative_text', 'result_indicator_baseline_document_link_title_narrative_lang') # NOQA: E501 - self.related_narrative(get_child_attr(result_indicator_baseline_document_link, 'documentlinkdescription'), 'result_indicator_baseline_document_link_description', 'result_indicator_baseline_document_link_description_text', 'result_indicator_baseline_document_link_description_lang') # NOQA: E501 + self.add_value_list('result_indicator_baseline_document_link_format', result_indicator_baseline_document_link.file_format_id) # NOQA: E501 + self.related_narrative(get_child_attr(result_indicator_baseline_document_link, 'documentlinktitle'), 'result_indicator_baseline_document_link_title', 'result_indicator_baseline_document_link_title_narrative_text', 'result_indicator_baseline_document_link_title_narrative_lang') # NOQA: E501 + self.related_narrative(get_child_attr(result_indicator_baseline_document_link, 'documentlinkdescription'), 'result_indicator_baseline_document_link_description', 'result_indicator_baseline_document_link_description_text', 'result_indicator_baseline_document_link_description_lang') # NOQA: E501 for document_link_category in result_indicator_baseline_document_link.documentlinkcategory_set.all(): # NOQA: E501 self.add_value_list('result_indicator_baseline_document_link_category_code', document_link_category.category_id) # NOQA: E501 @@ -2162,35 +2161,32 @@ def result(self): result_period_target.value ) else: - self.indexing['result_indicator_period_target_value'].append( + self.indexing['result_indicator_period_target_value'].append( # NOQA: E501 ' ') for result_period_target_location in result_period_target.resultindicatorperiodtargetlocation_set.all(): # NOQA: E501 if result_period_target_location.ref: self.add_value_list( - 'result_indicator_period_target_location_ref', - result_period_target_location.ref - ) + 'result_indicator_period_target_location_ref', # NOQA: E501 + result_period_target_location.ref) else: - self.indexing['result_indicator_period_target_location_ref'].append( + self.indexing['result_indicator_period_target_location_ref'].append( # NOQA: E501 ' ') for result_period_target_dimension in result_period_target.resultindicatorperiodtargetdimension_set.all(): # NOQA: E501 if result_period_target.name: self.add_value_list( - 'result_indicator_period_target_dimension_name', - result_period_target_dimension.name - ) + 'result_indicator_period_target_dimension_name', # NOQA: E501 + result_period_target_dimension.name) else: - self.indexing['result_indicator_period_target_dimension_name'].append( + self.indexing['result_indicator_period_target_dimension_name'].append( # NOQA: E501 ' ') if result_period_target.value: self.add_value_list( - 'result_indicator_period_target_dimension_value', - result_period_target_dimension.value - ) + 'result_indicator_period_target_dimension_value', # NOQA: E501 + result_period_target_dimension.value) else: - self.indexing['result_indicator_period_target_dimension_value'].append( + self.indexing['result_indicator_period_target_dimension_value'].append( # NOQA: E501 ' ') for result_period_target_comment in result_period_target.resultindicatorperiodtargetcomment_set.all(): # NOQA: E501 @@ -2250,35 +2246,32 @@ def result(self): result_period_actual.value ) else: - self.indexing['result_indicator_period_actual_value'].append( + self.indexing['result_indicator_period_actual_value'].append( # NOQA: E501 ' ') for result_period_actual_location in result_period_actual.resultindicatorperiodactuallocation_set.all(): # NOQA: E501 if result_period_actual_location.ref: self.add_value_list( - 'result_indicator_period_actual_location_ref', - result_period_actual_location.ref - ) + 'result_indicator_period_actual_location_ref', # NOQA: E501 + result_period_actual_location.ref) else: - self.indexing['result_indicator_period_actual_location_ref'].append( + self.indexing['result_indicator_period_actual_location_ref'].append( # NOQA: E501 ' ') for result_period_actual_dimension in result_period_actual.resultindicatorperiodactualdimension_set.all(): # NOQA: E501 if result_period_actual_dimension.name: self.add_value_list( - 'result_indicator_period_actual_dimension_name', - result_period_actual_dimension.name - ) + 'result_indicator_period_actual_dimension_name', # NOQA: E501 + result_period_actual_dimension.name) else: - self.indexing['result_indicator_period_actual_dimension_name'].append( + self.indexing['result_indicator_period_actual_dimension_name'].append( # NOQA: E501 ' ') if result_period_actual_dimension.value: self.add_value_list( - 'result_indicator_period_actual_dimension_value', - result_period_actual_dimension.value - ) + 'result_indicator_period_actual_dimension_value', # NOQA: E501 + result_period_actual_dimension.value) else: - self.indexing['result_indicator_period_actual_dimension_value'].append( + self.indexing['result_indicator_period_actual_dimension_value'].append( # NOQA: E501 ' ') for result_period_actual_comment in result_period_actual.resultindicatorperiodactualcomment_set.all(): # NOQA: E501 diff --git a/OIPA/solr/transaction/tasks.py b/OIPA/solr/transaction/tasks.py index a54823474..0bff7e332 100644 --- a/OIPA/solr/transaction/tasks.py +++ b/OIPA/solr/transaction/tasks.py @@ -7,6 +7,7 @@ from iati.transaction.models import Transaction from solr.tasks import BaseTaskIndexing from solr.transaction.indexing import TransactionIndexing + # from solr.transaction_sector.tasks import TransactionSectorTaskIndexing solr = pysolr.Solr( From 674531240eeb395132c371bd2086612b9f9a9b09 Mon Sep 17 00:00:00 2001 From: helene Date: Fri, 4 Sep 2020 11:54:33 +0200 Subject: [PATCH 04/22] feat : transformed percentage field from numeric to varchar to fix rounding issue --- OIPA/api/activity/serializers.py | 18 ++----- .../migrations/0071_auto_20200904_0706.py | 53 +++++++++++++++++++ OIPA/iati/models.py | 32 ++--------- OIPA/iati/parser/post_save_validators.py | 32 ++++++----- OIPA/iati/transaction/models.py | 12 ++--- 5 files changed, 83 insertions(+), 64 deletions(-) create mode 100644 OIPA/iati/migrations/0071_auto_20200904_0706.py diff --git a/OIPA/api/activity/serializers.py b/OIPA/api/activity/serializers.py index 9fda4074b..f7b5fd88a 100644 --- a/OIPA/api/activity/serializers.py +++ b/OIPA/api/activity/serializers.py @@ -1160,11 +1160,7 @@ def update(self, instance, validated_data): class ActivitySectorSerializer(ModelSerializerNoValidation): id = serializers.HiddenField(default=None) sector = SectorSerializer(fields=('url', 'code', 'name')) - percentage = serializers.DecimalField( - max_digits=5, - decimal_places=2, - coerce_to_string=False - ) + percentage = serializers.CharField() vocabulary = VocabularySerializer() vocabulary_uri = serializers.URLField() @@ -1475,11 +1471,7 @@ class ActivityRecipientRegionSerializer(DynamicFieldsModelSerializer): region = BasicRegionSerializer( fields=('url', 'code', 'name'), ) - percentage = serializers.DecimalField( - max_digits=5, - decimal_places=2, - coerce_to_string=False - ) + percentage = serializers.CharField() vocabulary = VocabularySerializer() vocabulary_uri = serializers.URLField(required=False) @@ -1601,11 +1593,7 @@ def update(self, instance, validated_data): class RecipientCountrySerializer(DynamicFieldsModelSerializer): id = serializers.HiddenField(default=None) country = CountrySerializer(fields=('url', 'code', 'name')) - percentage = serializers.DecimalField( - max_digits=5, - decimal_places=2, - coerce_to_string=False - ) + percentage = serializers.CharField() activity = serializers.CharField(write_only=True) narrative = NarrativeSerializer(many=True, required=True, source='narratives') diff --git a/OIPA/iati/migrations/0071_auto_20200904_0706.py b/OIPA/iati/migrations/0071_auto_20200904_0706.py new file mode 100644 index 000000000..9f9918c45 --- /dev/null +++ b/OIPA/iati/migrations/0071_auto_20200904_0706.py @@ -0,0 +1,53 @@ +# Generated by Django 2.0.13 on 2020-09-04 07:06 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('iati', '0070_auto_20200703_1100'), + ] + + operations = [ + migrations.AlterField( + model_name='activityrecipientcountry', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='activityrecipientregion', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='activitysector', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='budgetitem', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='budgetsector', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='transactionrecipientcountry', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='transactionrecipientregion', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + migrations.AlterField( + model_name='transactionsector', + name='percentage', + field=models.CharField(blank=True, max_length=100, null=True), + ), + ] diff --git a/OIPA/iati/models.py b/OIPA/iati/models.py index 2a2eb39bf..323e4f6e6 100644 --- a/OIPA/iati/models.py +++ b/OIPA/iati/models.py @@ -650,12 +650,7 @@ class ActivitySector(models.Model): null=True, blank=True, default=None, on_delete=models.CASCADE) vocabulary_uri = models.URLField(null=True, blank=True) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2, - null=True, - blank=True, - default=None) + percentage = models.CharField(max_length=100, null=True, blank=True) narratives = GenericRelation( Narrative, content_type_field='related_content_type', @@ -675,12 +670,7 @@ def get_activity(self): class ActivityRecipientCountry(models.Model): activity = models.ForeignKey(Activity, on_delete=models.CASCADE) country = models.ForeignKey(Country, on_delete=models.CASCADE) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2, - null=True, - blank=True, - default=None) + percentage = models.CharField(max_length=100, null=True, blank=True) narratives = GenericRelation( Narrative, content_type_field='related_content_type', @@ -730,12 +720,7 @@ class BudgetItem(models.Model): country_budget_item = models.ForeignKey(CountryBudgetItem, on_delete=models.CASCADE) code = models.ForeignKey(BudgetIdentifier, on_delete=models.CASCADE) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2, - null=True, - blank=True, - default=None) + percentage = models.CharField(max_length=100, null=True, blank=True) def get_activity(self): return self.country_budget_item.activity @@ -763,12 +748,7 @@ class ActivityRecipientRegion(models.Model): Narrative, content_type_field='related_content_type', object_id_field='related_object_id') - percentage = models.DecimalField( - max_digits=5, - decimal_places=2, - null=True, - blank=True, - default=None) + percentage = models.CharField(max_length=100, null=True, blank=True) def __unicode__(self,): return "name: %s" % self.region @@ -1389,9 +1369,7 @@ class BudgetSector(models.Model): Sector, on_delete=models.CASCADE) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2) + percentage = models.CharField(max_length=100, null=True, blank=True) def __unicode__(self, ): return "%s - %s" % (self.budget.id, self.sector.code) diff --git a/OIPA/iati/parser/post_save_validators.py b/OIPA/iati/parser/post_save_validators.py index 7c225f495..726556e3a 100644 --- a/OIPA/iati/parser/post_save_validators.py +++ b/OIPA/iati/parser/post_save_validators.py @@ -38,17 +38,20 @@ def geo_percentages_add_up(self, a): Rule: Percentages for all reported countries and regions must add up to 100% """ - recipient_country_sum = a.activityrecipientcountry_set.all().aggregate( - Sum('percentage') - ).get('percentage__sum') - recipient_region_sum = a.activityrecipientregion_set.all( - ).aggregate(Sum('percentage')).get('percentage__sum') - - total_sum = 0 - if recipient_country_sum: - total_sum += recipient_country_sum - if recipient_region_sum: - total_sum += recipient_region_sum + + recipient_country_percentages = a.activityrecipientcountry_set.values_list('percentage') + recipient_country_sum = 0 + for i in recipient_country_percentages: + for j in i: + recipient_country_sum += float(j) + + recipient_region_percentages = a.activityrecipientregion_set.values_list('percentage') + recipient_region_sum = 0 + for i in recipient_region_percentages: + for j in i: + recipient_region_sum += float(j) + + total_sum = recipient_country_sum + recipient_region_sum if not (total_sum == 0 or total_sum == 100): self.append_error( @@ -66,8 +69,11 @@ def sector_percentages_add_up(self, a): """ Rule: Percentages for all reported sectors must add up to 100% """ - sector_sum = a.activitysector_set.all().aggregate( - Sum('percentage')).get('percentage__sum') + sector_percentage = a.activitysector_set.values_list('percentage') + sector_sum = 0 + for i in sector_percentage: + for j in i: + sector_sum += float(j) if not (sector_sum is None or sector_sum == 0 or sector_sum == 100): self.append_error( diff --git a/OIPA/iati/transaction/models.py b/OIPA/iati/transaction/models.py index 6eb8bf7be..5313b9b73 100644 --- a/OIPA/iati/transaction/models.py +++ b/OIPA/iati/transaction/models.py @@ -243,9 +243,7 @@ class TransactionSector(models.Model): reported_on_transaction = models.BooleanField(default=True) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2) + percentage = models.CharField(max_length=100, null=True, blank=True) narratives = GenericRelation( Narrative, @@ -278,9 +276,7 @@ class TransactionRecipientCountry(models.Model): reported_on_transaction = models.BooleanField(default=True) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2) + percentage = models.CharField(max_length=100, null=True, blank=True) narratives = GenericRelation( Narrative, @@ -322,9 +318,7 @@ class TransactionRecipientRegion(models.Model): reported_on_transaction = models.BooleanField(default=True) - percentage = models.DecimalField( - max_digits=5, - decimal_places=2) + percentage = models.CharField(max_length=100, null=True, blank=True) narratives = GenericRelation( Narrative, From b68bdabba2c2493cee68f2f73110d27a67879330 Mon Sep 17 00:00:00 2001 From: helene Date: Tue, 8 Sep 2020 15:21:43 +0200 Subject: [PATCH 05/22] fix : fixed rounded percentage for indexing --- OIPA/solr/activity/indexing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OIPA/solr/activity/indexing.py b/OIPA/solr/activity/indexing.py index 1f0c09c0b..b95c5d96a 100644 --- a/OIPA/solr/activity/indexing.py +++ b/OIPA/solr/activity/indexing.py @@ -449,7 +449,7 @@ def recipient_country(self): ) self.add_value_list( 'recipient_country_percentage', - decimal_string(recipient_country.percentage) + recipient_country.percentage ) self.related_narrative( @@ -509,7 +509,7 @@ def recipient_region(self): ) self.add_value_list( 'recipient_region_percentage', - decimal_string(recipient_region.percentage) + recipient_region.percentage ) self.related_narrative( @@ -670,7 +670,7 @@ def sector(self): ) self.add_value_list( 'sector_percentage', - decimal_string(activity_sector.percentage) + activity_sector.percentage ) self.related_narrative( @@ -774,7 +774,7 @@ def country_budget_items(self): ) self.add_value_list( 'country_budget_items_budget_item_percentage', - decimal_string(budget_item.percentage) + budget_item.percentage ) self.related_narrative( From 1c97cb9c07676f11096f573a97daff9751a03d7b Mon Sep 17 00:00:00 2001 From: helene Date: Wed, 9 Sep 2020 15:47:47 +0200 Subject: [PATCH 06/22] fix : logic of code for total percentage sum --- OIPA/iati/parser/post_save_validators.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/OIPA/iati/parser/post_save_validators.py b/OIPA/iati/parser/post_save_validators.py index 726556e3a..7fbc0b5d9 100644 --- a/OIPA/iati/parser/post_save_validators.py +++ b/OIPA/iati/parser/post_save_validators.py @@ -51,7 +51,11 @@ def geo_percentages_add_up(self, a): for j in i: recipient_region_sum += float(j) - total_sum = recipient_country_sum + recipient_region_sum + total_sum = 0 + if recipient_country_sum: + total_sum += recipient_country_sum + if recipient_region_sum: + total_sum += recipient_region_sum if not (total_sum == 0 or total_sum == 100): self.append_error( From bf0da135cc3edd45510494edb9ccf33298d73274 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Thu, 10 Sep 2020 18:17:07 +0200 Subject: [PATCH 07/22] -test fixed --- .../tests/test_activities_csv_endpoints.py | 2 +- OIPA/api/activity/tests/test_serializers.py | 9 ++++++--- OIPA/iati/tests/test_parser_post_models.py | 20 +++++++++---------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/OIPA/api/activity/tests/test_activities_csv_endpoints.py b/OIPA/api/activity/tests/test_activities_csv_endpoints.py index ed08967bd..e6601688f 100644 --- a/OIPA/api/activity/tests/test_activities_csv_endpoints.py +++ b/OIPA/api/activity/tests/test_activities_csv_endpoints.py @@ -108,7 +108,7 @@ def test_activities_csv_endpoint_data(self): self.assertEqual(row[default_csv_headers[1]], sector.sector.code + ';') self.assertEqual(row[default_csv_headers[2]], - str(format(sector.percentage, '.2f')) + str(sector.percentage) + ';') self.assertEqual(row[default_csv_headers[4]], country.country.code + ';') diff --git a/OIPA/api/activity/tests/test_serializers.py b/OIPA/api/activity/tests/test_serializers.py index 9f5bd58dd..7c76613f0 100644 --- a/OIPA/api/activity/tests/test_serializers.py +++ b/OIPA/api/activity/tests/test_serializers.py @@ -331,7 +331,8 @@ def test_ActivitySectorSerializer(self): activity_sector, context={'request': self.request_dummy}, ) - assert serializer.data['percentage'] == activity_sector.percentage,\ + assert serializer.data['percentage'] == str( + activity_sector.percentage),\ """ 'activity_sector.percentage' should be serialized to a field called percentage @@ -360,7 +361,8 @@ def test_ActivityRecipientRegionSerializer(self): serializer = serializers.ActivityRecipientRegionSerializer( recipient_region, context={'request': self.request_dummy}) - assert serializer.data['percentage'] == recipient_region.percentage,\ + assert serializer.data['percentage'] == \ + str(recipient_region.percentage),\ """ 'recipient_region.percentage' should be serialized to a field called 'percentage' @@ -405,7 +407,8 @@ def test_RecipientCountrySerializer(self): recipient_country, context={'request': self.request_dummy} ) - assert serializer.data['percentage'] == recipient_country.percentage,\ + assert serializer.data['percentage'] == \ + str(recipient_country.percentage),\ """ 'recipient_country.percentage' should be serialized to a field called 'percentage' diff --git a/OIPA/iati/tests/test_parser_post_models.py b/OIPA/iati/tests/test_parser_post_models.py index 4c31bebca..85f92978d 100644 --- a/OIPA/iati/tests/test_parser_post_models.py +++ b/OIPA/iati/tests/test_parser_post_models.py @@ -232,12 +232,12 @@ def test_set_country_region_transaction_without_percentages(self): trc1 = TransactionRecipientCountry.objects.filter( country=self.c1, transaction=self.t1)[0] # 10000 / 3 - self.assertEqual(round(trc1.percentage), 33) + self.assertEqual(round(Decimal(trc1.percentage)), 33) trr1 = TransactionRecipientRegion.objects.filter( region=self.r1, transaction=self.t2)[0] # 20000 / 3 - self.assertEqual(round(trr1.percentage), 33) + self.assertEqual(round(Decimal(trr1.percentage)), 33) def test_set_sector_transaction_with_percentages(self): """ @@ -293,22 +293,22 @@ def test_set_sector_transaction_without_percentages(self): ts1 = TransactionSector.objects.filter( sector=self.s1, transaction=self.t1)[0] # 10000 / 2 - self.assertEqual(ts1.percentage, 50) + self.assertEqual(Decimal(ts1.percentage), 50) ts2 = TransactionSector.objects.filter( sector=self.s2, transaction=self.t1)[0] # 20000 / 3 - self.assertEqual(round(ts2.percentage), 50) + self.assertEqual(round(Decimal(ts2.percentage)), 50) TransactionSector.objects.filter( sector=self.s3, transaction=self.t1)[0] # 10000 / 2 - self.assertEqual(ts1.percentage, 50) + self.assertEqual(Decimal(ts1.percentage), 50) TransactionSector.objects.filter( sector=self.s4, transaction=self.t1)[0] # 20000 / 3 - self.assertEqual(round(ts2.percentage), 50) + self.assertEqual(round(Decimal(ts2.percentage)), 50) def test_set_sector_budget_with_percentages(self): """ @@ -361,16 +361,16 @@ def test_set_sector_budget_without_percentages(self): ts1 = BudgetSector.objects.filter( sector=self.s1, budget=self.budget1)[0] - self.assertEqual(ts1.percentage, 25) + self.assertEqual(Decimal(ts1.percentage), 25) ts2 = BudgetSector.objects.filter( sector=self.s2, budget=self.budget2)[0] - self.assertEqual(round(ts2.percentage), 25) + self.assertEqual(round(Decimal(ts2.percentage)), 25) BudgetSector.objects.filter( sector=self.s3, budget=self.budget1)[0] - self.assertEqual(ts1.percentage, 25) + self.assertEqual(Decimal(ts1.percentage), 25) BudgetSector.objects.filter( sector=self.s4, budget=self.budget2)[0] - self.assertEqual(round(ts2.percentage), 25) + self.assertEqual(round(Decimal(ts2.percentage)), 25) From 45ea6f7b543d02322c20f3d8420fbb6cc5e8772c Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Thu, 10 Sep 2020 20:14:51 +0200 Subject: [PATCH 08/22] fix: currency annotation update for percentage field. Because we change percentage field from decimal to string --- OIPA/api/budget/views.py | 9 ++++++--- OIPA/api/transaction/views.py | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/OIPA/api/budget/views.py b/OIPA/api/budget/views.py index 3ec5cada3..d57a0b388 100644 --- a/OIPA/api/budget/views.py +++ b/OIPA/api/budget/views.py @@ -1,5 +1,6 @@ from django.conf import settings -from django.db.models import Count, F, Sum +from django.db.models import Count, ExpressionWrapper, F, FloatField, Sum +from django.db.models.functions import Cast from django.utils.decorators import method_decorator from django.views.decorators.cache import cache_page from django_filters.rest_framework import DjangoFilterBackend @@ -66,10 +67,12 @@ def annotate_currency(query_params, groupings): additions = list(set(param_additions).union(grouping_additions)) for percentage_field in additions: - percentage_expression = F(percentage_field) / 100.0 + percentage_expression = Cast(percentage_field, + output_field=FloatField()) / 100.0 annotation_components = annotation_components * percentage_expression - return Sum(annotation_components) + return ExpressionWrapper(Sum(annotation_components), + output_field=FloatField()) class BudgetAggregations(AggregationView): diff --git a/OIPA/api/transaction/views.py b/OIPA/api/transaction/views.py index 36c366679..c2dafba4a 100644 --- a/OIPA/api/transaction/views.py +++ b/OIPA/api/transaction/views.py @@ -1,5 +1,6 @@ from django.conf import settings -from django.db.models import Count, F, Q, Sum +from django.db.models import Count, ExpressionWrapper, F, FloatField, Q, Sum +from django.db.models.functions import Cast from django.utils.decorators import method_decorator from django.views.decorators.cache import cache_page from django_filters.rest_framework import DjangoFilterBackend @@ -276,10 +277,12 @@ def annotate_currency(query_params, groupings): additions = list(set(param_additions).union(grouping_additions)) for percentage_field in additions: - percentage_expression = F(percentage_field) / 100.0 + percentage_expression = Cast(percentage_field, + output_field=FloatField()) / 100.0 annotation_components = annotation_components * percentage_expression - return Sum(annotation_components) + return ExpressionWrapper(Sum(annotation_components), + output_field=FloatField()) class TransactionAggregation(AggregationView): From 64dbc1f6e61a00554a41b214ad4f9fb6c28baff0 Mon Sep 17 00:00:00 2001 From: helene Date: Fri, 11 Sep 2020 11:50:44 +0200 Subject: [PATCH 09/22] fix : flake8 errors --- OIPA/api/activity/tests/test_serializers.py | 8 ++++---- OIPA/iati/parser/post_save_validators.py | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/OIPA/api/activity/tests/test_serializers.py b/OIPA/api/activity/tests/test_serializers.py index 7c76613f0..f5578722f 100644 --- a/OIPA/api/activity/tests/test_serializers.py +++ b/OIPA/api/activity/tests/test_serializers.py @@ -361,8 +361,8 @@ def test_ActivityRecipientRegionSerializer(self): serializer = serializers.ActivityRecipientRegionSerializer( recipient_region, context={'request': self.request_dummy}) - assert serializer.data['percentage'] == \ - str(recipient_region.percentage),\ + assert serializer.data['percentage'] == str( + recipient_region.percentage),\ """ 'recipient_region.percentage' should be serialized to a field called 'percentage' @@ -407,8 +407,8 @@ def test_RecipientCountrySerializer(self): recipient_country, context={'request': self.request_dummy} ) - assert serializer.data['percentage'] == \ - str(recipient_country.percentage),\ + assert serializer.data['percentage'] == str( + recipient_country.percentage),\ """ 'recipient_country.percentage' should be serialized to a field called 'percentage' diff --git a/OIPA/iati/parser/post_save_validators.py b/OIPA/iati/parser/post_save_validators.py index 7fbc0b5d9..2059c6db3 100644 --- a/OIPA/iati/parser/post_save_validators.py +++ b/OIPA/iati/parser/post_save_validators.py @@ -1,4 +1,4 @@ -from django.db.models import Max, Sum +from django.db.models import Max from iati.models import ( Activity, ActivityParticipatingOrganisation, RelatedActivity, @@ -39,13 +39,15 @@ def geo_percentages_add_up(self, a): 100% """ - recipient_country_percentages = a.activityrecipientcountry_set.values_list('percentage') + recipient_country_percentages = \ + a.activityrecipientcountry_set.values_list('percentage') recipient_country_sum = 0 for i in recipient_country_percentages: for j in i: recipient_country_sum += float(j) - recipient_region_percentages = a.activityrecipientregion_set.values_list('percentage') + recipient_region_percentages = \ + a.activityrecipientregion_set.values_list('percentage') recipient_region_sum = 0 for i in recipient_region_percentages: for j in i: From 5dfa35585a2ae50b2a30bcf57e59e33d7e28f7cd Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Thu, 17 Sep 2020 16:45:22 +0200 Subject: [PATCH 10/22] fix: add try/except block for float() in geo_percentages_add_up --- OIPA/iati/parser/post_save_validators.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/OIPA/iati/parser/post_save_validators.py b/OIPA/iati/parser/post_save_validators.py index 2059c6db3..d72c94093 100644 --- a/OIPA/iati/parser/post_save_validators.py +++ b/OIPA/iati/parser/post_save_validators.py @@ -44,14 +44,20 @@ def geo_percentages_add_up(self, a): recipient_country_sum = 0 for i in recipient_country_percentages: for j in i: - recipient_country_sum += float(j) + try: + recipient_country_sum += float(j) + except TypeError: + pass recipient_region_percentages = \ a.activityrecipientregion_set.values_list('percentage') recipient_region_sum = 0 for i in recipient_region_percentages: for j in i: - recipient_region_sum += float(j) + try: + recipient_region_sum += float(j) + except TypeError: + pass total_sum = 0 if recipient_country_sum: @@ -79,7 +85,10 @@ def sector_percentages_add_up(self, a): sector_sum = 0 for i in sector_percentage: for j in i: - sector_sum += float(j) + try: + sector_sum += float(j) + except TypeError: + pass if not (sector_sum is None or sector_sum == 0 or sector_sum == 100): self.append_error( From 349203bf2ccd233adcff4f8ac9be47164d9be62a Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Fri, 18 Sep 2020 12:55:15 +0200 Subject: [PATCH 11/22] fix: bug fix in indexing.py and sector bug fix in parsers. --- OIPA/iati/parser/IATI_2_02.py | 6 ++++-- OIPA/iati/parser/IATI_2_03.py | 6 ++++-- OIPA/solr/activity/indexing.py | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/OIPA/iati/parser/IATI_2_02.py b/OIPA/iati/parser/IATI_2_02.py index 0eb144450..0b5217cc6 100644 --- a/OIPA/iati/parser/IATI_2_02.py +++ b/OIPA/iati/parser/IATI_2_02.py @@ -1411,7 +1411,7 @@ def iati_activities__iati_activity__sector(self, element): None, vocabulary.code) - sector = models.Sector.objects.filter(code=code, + sector = models.Sector.objects.filter(code=slugify(code), vocabulary=vocabulary).first() if not sector and vocabulary.code == '1': @@ -1433,6 +1433,7 @@ def iati_activities__iati_activity__sector(self, element): if not sector: sector = models.Sector() sector.code = code + sector.vocabulary = vocabulary sector.name = 'Vocabulary 99 or 98' sector.description = 'The sector reported corresponds to a sector vocabulary maintained by the reporting organisation for this activity' # NOQA: E501 sector.save() @@ -2573,7 +2574,7 @@ def iati_activities__iati_activity__transaction__sector(self, element): None, element.attrib.get('vocabulary')) - sector = models.Sector.objects.filter(code=code, + sector = models.Sector.objects.filter(code=slugify(code), vocabulary=vocabulary).first() if not sector and vocabulary.code == '1': @@ -2595,6 +2596,7 @@ def iati_activities__iati_activity__transaction__sector(self, element): if not sector: sector = models.Sector() sector.code = code + sector.vocabulary = vocabulary sector.name = 'Vocabulary 99 or 98' sector.description = 'The sector reported corresponds to a sector vocabulary maintained by the reporting organisation for this activity' # NOQA: E501 sector.save() diff --git a/OIPA/iati/parser/IATI_2_03.py b/OIPA/iati/parser/IATI_2_03.py index 830f5e6c2..24e55f300 100644 --- a/OIPA/iati/parser/IATI_2_03.py +++ b/OIPA/iati/parser/IATI_2_03.py @@ -1460,7 +1460,7 @@ def iati_activities__iati_activity__sector(self, element): None, None) - sector = models.Sector.objects.filter(code=code, + sector = models.Sector.objects.filter(code=slugify(code), vocabulary=vocabulary).first() if not sector and vocabulary.code == '1': @@ -1482,6 +1482,7 @@ def iati_activities__iati_activity__sector(self, element): if not sector: sector = models.Sector() sector.code = code + sector.vocabulary = vocabulary sector.name = 'Vocabulary 99 or 98' sector.description = 'The sector reported corresponds to a sector vocabulary maintained by the reporting organisation for this activity' # NOQA: E501 sector.save() @@ -2673,7 +2674,7 @@ def iati_activities__iati_activity__transaction__sector(self, element): None, element.attrib.get('vocabulary')) - sector = models.Sector.objects.filter(code=code, + sector = models.Sector.objects.filter(code=slugify(code), vocabulary=vocabulary).first() if not sector and vocabulary.code == '1': @@ -2694,6 +2695,7 @@ def iati_activities__iati_activity__transaction__sector(self, element): if not sector: sector = models.Sector() sector.code = code + sector.vocabulary = vocabulary sector.name = 'Vocabulary 99 or 98' sector.description = 'The sector reported corresponds to a sector vocabulary maintained by the reporting organisation for this activity' # NOQA: E501 sector.save() diff --git a/OIPA/solr/activity/indexing.py b/OIPA/solr/activity/indexing.py index 18dffb9a6..1352ca24b 100644 --- a/OIPA/solr/activity/indexing.py +++ b/OIPA/solr/activity/indexing.py @@ -2174,14 +2174,14 @@ def result(self): ' ') for result_period_target_dimension in result_period_target.resultindicatorperiodtargetdimension_set.all(): # NOQA: E501 - if result_period_target.name: + if result_period_target_dimension.name: self.add_value_list( 'result_indicator_period_target_dimension_name', # NOQA: E501 result_period_target_dimension.name) else: self.indexing['result_indicator_period_target_dimension_name'].append( # NOQA: E501 ' ') - if result_period_target.value: + if result_period_target_dimension.value: self.add_value_list( 'result_indicator_period_target_dimension_value', # NOQA: E501 result_period_target_dimension.value) From 8eac09fcd1eba0495c2ba75f02f4f68740372c24 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Fri, 18 Sep 2020 16:52:25 +0200 Subject: [PATCH 12/22] fix: add timeout parameter in requests. --- OIPA/task_queue/validation.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/OIPA/task_queue/validation.py b/OIPA/task_queue/validation.py index 993fe665e..91e685052 100644 --- a/OIPA/task_queue/validation.py +++ b/OIPA/task_queue/validation.py @@ -73,7 +73,7 @@ def run(self, dataset_id=None, *args, **kwargs): def _check(self): try: - get_respons = requests.get(self._dataset.source_url) + get_respons = requests.get(self._dataset.source_url, timeout=30) if get_respons.status_code == 200: md5 = hashlib.md5() md5.update(get_respons.content) @@ -86,7 +86,8 @@ def _check(self): except requests.exceptions.SSLError as e: logger.info('%s (%s)' % (e, type(e)) + self._dataset.source_url) try: - resp = requests.get(self._dataset.source_url, verify=False) + resp = requests.get(self._dataset.source_url, verify=False, + timeout=30) if resp.status_code == 200: md5 = hashlib.md5() md5.update(resp.content) @@ -258,13 +259,13 @@ def _get(self, ad_hoc=False): ).get('get_json_file').format(json_file=json_file) ) # Request to the JSON result to the server - response = requests.get(get_json_file_url) + response = requests.get(get_json_file_url, timeout=30) # If the response if OK then save the result # to the variable json result if response.status_code == 200: self._json_result = response.json() else: - response = requests.get(get_json_file_url, verify=False) + response = requests.get(get_json_file_url, verify=False, timeout=30) if response.status_code == 200: self._json_result = response.json() From af5b470f89fe78e8439b349693bf602a0f3945a1 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Fri, 18 Sep 2020 16:56:44 +0200 Subject: [PATCH 13/22] fix: flake8 error fixed --- OIPA/task_queue/validation.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/OIPA/task_queue/validation.py b/OIPA/task_queue/validation.py index 91e685052..e62c20a8f 100644 --- a/OIPA/task_queue/validation.py +++ b/OIPA/task_queue/validation.py @@ -86,8 +86,7 @@ def _check(self): except requests.exceptions.SSLError as e: logger.info('%s (%s)' % (e, type(e)) + self._dataset.source_url) try: - resp = requests.get(self._dataset.source_url, verify=False, - timeout=30) + resp = requests.get(self._dataset.source_url, verify=False, timeout=30) # NOQA: E501 if resp.status_code == 200: md5 = hashlib.md5() md5.update(resp.content) @@ -265,7 +264,7 @@ def _get(self, ad_hoc=False): if response.status_code == 200: self._json_result = response.json() else: - response = requests.get(get_json_file_url, verify=False, timeout=30) + response = requests.get(get_json_file_url, verify=False, timeout=30) # NOQA: E501 if response.status_code == 200: self._json_result = response.json() From 94866598e36eb08f68b2d753d7c139229e1fb357 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Sat, 19 Sep 2020 11:24:58 +0200 Subject: [PATCH 14/22] fix: add header in request in validation.py --- OIPA/task_queue/validation.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/OIPA/task_queue/validation.py b/OIPA/task_queue/validation.py index e62c20a8f..14c615834 100644 --- a/OIPA/task_queue/validation.py +++ b/OIPA/task_queue/validation.py @@ -72,8 +72,12 @@ def run(self, dataset_id=None, *args, **kwargs): # self._updated() def _check(self): + headers = {'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X ' + '10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36'} # NOQA: E501 + try: - get_respons = requests.get(self._dataset.source_url, timeout=30) + get_respons = requests.get(self._dataset.source_url, + headers=headers, timeout=30) if get_respons.status_code == 200: md5 = hashlib.md5() md5.update(get_respons.content) @@ -86,7 +90,8 @@ def _check(self): except requests.exceptions.SSLError as e: logger.info('%s (%s)' % (e, type(e)) + self._dataset.source_url) try: - resp = requests.get(self._dataset.source_url, verify=False, timeout=30) # NOQA: E501 + resp = requests.get(self._dataset.source_url, headers=headers, + verify=False, timeout=30) # NOQA: E501 if resp.status_code == 200: md5 = hashlib.md5() md5.update(resp.content) From fdfbf52a0036cdd39abf1f0bc3c02569c8744f8f Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Sat, 19 Sep 2020 18:54:32 +0200 Subject: [PATCH 15/22] fix: datefield cannot be empty string in solr so use date_string() in indexing.py --- OIPA/solr/activity/indexing.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/OIPA/solr/activity/indexing.py b/OIPA/solr/activity/indexing.py index 1352ca24b..4e591be8b 100644 --- a/OIPA/solr/activity/indexing.py +++ b/OIPA/solr/activity/indexing.py @@ -2085,7 +2085,7 @@ def result(self): ) else: self.indexing['result_indicator_baseline_iso_date'].append( # NOQA: E501 - ' ') + date_string(' ')) if result_indicator_baseline.value: self.add_value_list( 'result_indicator_baseline_value', @@ -2511,10 +2511,11 @@ def fss(self): if forecast.value_date: self.add_value_list( 'fss_forecast_value_date', - value_string(forecast.value_date) + date_string(forecast.value_date) ) else: - self.indexing['fss_forecast_value_date'].append(' ') + self.indexing['fss_forecast_value_date'].append( + date_string(' ')) if forecast.currency_id: self.add_value_list( 'fss_forecast_currency', From b4cddaa283a87d34119f5ec8f7458b2263f9815a Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Mon, 21 Sep 2020 13:40:29 +0200 Subject: [PATCH 16/22] fix: fixed broken budget/transaction aggregation group_by sector and recipient_region (because we change the db table for those twos) --- OIPA/api/budget/views.py | 8 ++++++-- OIPA/api/transaction/views.py | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/OIPA/api/budget/views.py b/OIPA/api/budget/views.py index d57a0b388..bc8b3310e 100644 --- a/OIPA/api/budget/views.py +++ b/OIPA/api/budget/views.py @@ -158,20 +158,24 @@ class BudgetAggregations(AggregationView): ), GroupBy( query_param="recipient_region", - fields="activity__recipient_region", + fields="activity__recipient_region__code", renamed_fields="recipient_region", queryset=Region.objects.all(), serializer=RegionSerializer, + serializer_fk='code', serializer_fields=('url', 'code', 'name',), name_search_field="activity__recipient_region__name", renamed_name_search_field="recipient_region_name", ), GroupBy( query_param="sector", - fields="budgetsector__sector", + fields="budgetsector__sector__code", renamed_fields="sector", queryset=Sector.objects.all(), serializer=SectorSerializer, + # though code is not fk, it is used in searching sector code + # in Sector model. + serializer_fk='code', serializer_fields=('url', 'code', 'name'), name_search_field="budgetsector__sector__name", renamed_name_search_field="sector_name", diff --git a/OIPA/api/transaction/views.py b/OIPA/api/transaction/views.py index c2dafba4a..3623e8000 100644 --- a/OIPA/api/transaction/views.py +++ b/OIPA/api/transaction/views.py @@ -455,20 +455,22 @@ class TransactionAggregation(AggregationView): ), GroupBy( query_param="recipient_region", - fields="transactionrecipientregion__region", + fields="transactionrecipientregion__region__code", renamed_fields="recipient_region", queryset=Region.objects.all(), serializer=RegionSerializer, + serializer_fk='code', serializer_fields=('url', 'code', 'name', 'location'), name_search_field="transactionrecipientregion__region__name", renamed_name_search_field="recipient_region_name", ), GroupBy( query_param="sector", - fields="transactionsector__sector", + fields="transactionsector__sector__code", renamed_fields="sector", queryset=Sector.objects.all(), serializer=SectorSerializer, + serializer_fk='code', serializer_fields=('url', 'code', 'name', 'location'), name_search_field="transactionsector__sector__name", renamed_name_search_field="sector_name", From d64cb746024d85306a63fcb1b5c42a1984ccd688 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Thu, 24 Sep 2020 14:25:22 +0200 Subject: [PATCH 17/22] feat: add two factor auth --- OIPA/OIPA/settings.py | 11 +++ OIPA/OIPA/urls.py | 41 ++++++++++ OIPA/templates/two_factor/_base.html | 93 ++++++++++++++++++++++ OIPA/templates/two_factor/_base_focus.html | 20 +++++ 4 files changed, 165 insertions(+) create mode 100644 OIPA/templates/two_factor/_base.html create mode 100644 OIPA/templates/two_factor/_base_focus.html diff --git a/OIPA/OIPA/settings.py b/OIPA/OIPA/settings.py index 552e05c19..2dcc4b619 100644 --- a/OIPA/OIPA/settings.py +++ b/OIPA/OIPA/settings.py @@ -144,11 +144,19 @@ def rel(*x): 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'api.middleware.FileExportMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'django_otp.middleware.OTPMiddleware', ] ROOT_URLCONF = 'OIPA.urls' INSTALLED_APPS = [ + # 'two-factor + 'django_otp', + 'django_otp.plugins.otp_static', + 'django_otp.plugins.otp_totp', + 'two_factor', + # 'django_rq', 'django.contrib.auth', 'django.contrib.contenttypes', @@ -244,6 +252,9 @@ def rel(*x): 'DEFAULT_TIMEOUT': 10800, } } +TWO_FACTOR_FORCE_OTP_ADMIN = True +LOGIN_URL = 'two_factor:login' +LOGIN_REDIRECT_URL = '/admin' # Redirect admin dashboard GRAPPELLI_ADMIN_TITLE = 'OIPA admin' ADMINFILES_UPLOAD_TO = 'csv_files' diff --git a/OIPA/OIPA/urls.py b/OIPA/OIPA/urls.py index 9e61eae31..01e95a234 100644 --- a/OIPA/OIPA/urls.py +++ b/OIPA/OIPA/urls.py @@ -2,13 +2,53 @@ from django.conf.urls import include, url from django.conf.urls.static import static from django.contrib import admin +from django.contrib.auth import REDIRECT_FIELD_NAME from django.views.generic import TemplateView from django.views.generic.base import RedirectView +from two_factor.admin import AdminSiteOTPRequired, AdminSiteOTPRequiredMixin +from two_factor.urls import urlpatterns as tf_urls +from django.urls import reverse +from django.utils.http import is_safe_url +from django.http import HttpResponseRedirect +from django.shortcuts import resolve_url +from django.contrib.auth.views import redirect_to_login + from OIPA import views admin.autodiscover() + +class AdminSiteOTPRequiredMixinRedirSetup(AdminSiteOTPRequired): + def login(self, request, extra_context=None): + redirect_to = request.POST.get( + REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME) + ) + # For users not yet verified the AdminSiteOTPRequired.has_permission + # will fail. So use the standard admin has_permission check: + # (is_active and is_staff) and then check for verification. + # Go to index if they pass, otherwise make them setup OTP device. + if request.method == "GET" and super( + AdminSiteOTPRequiredMixin, self + ).has_permission(request): + # Already logged-in and verified by OTP + if request.user.is_verified(): + # User has permission + index_path = reverse("admin:index", current_app=self.name) + else: + # User has permission but no OTP set: + index_path = reverse("two_factor:setup", current_app=self.name) + return HttpResponseRedirect(index_path) + + if not redirect_to or not is_safe_url( + url=redirect_to, allowed_hosts=[request.get_host()] + ): + redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL) + + return redirect_to_login(redirect_to) + + +admin.site.__class__ = AdminSiteOTPRequiredMixinRedirSetup urlpatterns = [ # url(r'^grappelli/', include('grappelli.urls')), url(r'^admin/queue/', include('django_rq.urls')), @@ -22,6 +62,7 @@ url(r'^about$', TemplateView.as_view(template_name='home/about.html')), url(r'^accounts/profile/', RedirectView.as_view(url='/admin')), url(r'^$', RedirectView.as_view(url='/home', permanent=True)), + url(r'', include(tf_urls, "two_factor")), ] handler404 = views.error404 diff --git a/OIPA/templates/two_factor/_base.html b/OIPA/templates/two_factor/_base.html new file mode 100644 index 000000000..2c99ba2c1 --- /dev/null +++ b/OIPA/templates/two_factor/_base.html @@ -0,0 +1,93 @@ +{% load i18n static %} +{% get_current_language as LANGUAGE_CODE %}{% get_current_language_bidi as LANGUAGE_BIDI %} + + +{% block title %}{% endblock %} + +{% block extrastyle %}{% endblock %} +{% if LANGUAGE_BIDI %}{% endif %} +{% block extrahead %}{% endblock %} +{% block responsive %} + + + {% if LANGUAGE_BIDI %}{% endif %} +{% endblock %} +{% block blockbots %}{% endblock %} + +{% load i18n %} + + + + +
+ + {% if not is_popup %} + + + + {% block breadcrumbs %} +
+ {% trans 'Home' %} + {% if title %} › {{ title }}{% endif %} +
+ {% endblock %} + {% endif %} + + {% block messages %} + {% if messages %} +
    {% for message in messages %} + {{ message|capfirst }} + {% endfor %}
+ {% endif %} + {% endblock messages %} + + +
+ {% block pretitle %}{% endblock %} + {% block content_title %}{% if title %}

{{ title }}

{% endif %}{% endblock %} + {% block content %} + {% block object-tools %}{% endblock %} + {{ content }} + {% endblock %} + {% block sidebar %}{% endblock %} +
+
+ + + {% block footer %}{% endblock %} +
+ + + + \ No newline at end of file diff --git a/OIPA/templates/two_factor/_base_focus.html b/OIPA/templates/two_factor/_base_focus.html new file mode 100644 index 000000000..0b85ec376 --- /dev/null +++ b/OIPA/templates/two_factor/_base_focus.html @@ -0,0 +1,20 @@ +{% extends "admin/base_site.html" %} +{% load i18n static %} + +{% block extrastyle %}{{ block.super }} +{{ form.media }} +{% endblock %} + +{% block bodyclass %}{{ block.super }} login{% endblock %} + +{% block usertools %}{% endblock %} + +{% block nav-global %}{% endblock %} + +{% block content_title %}{% endblock %} + +{% block breadcrumbs %}{% endblock %} + +{% block content %} + +{% endblock %} From 82341a863eb5cf34930418f6d4889683803b7a51 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Thu, 24 Sep 2020 16:08:34 +0200 Subject: [PATCH 18/22] feat: add two factor auth --- OIPA/OIPA/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/OIPA/OIPA/settings.py b/OIPA/OIPA/settings.py index 2dcc4b619..a488908d1 100644 --- a/OIPA/OIPA/settings.py +++ b/OIPA/OIPA/settings.py @@ -15,6 +15,7 @@ LOGIN_REDIRECT_URL = '/admin/' LOGOUT_URL = '/logout' +LOGOUT_REDIRECT_URL = '/admin/logout' DATA_UPLOAD_MAX_NUMBER_FIELDS = 3000 SECRET_KEY = env.get('OIPA_SECRET_KEY', 'PXwlMOpfNJTgIdQeH5zk39jKfUMZPOUK') From 901b3aaa9b4517ab5c481246285084dbef43c960 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Thu, 24 Sep 2020 16:09:34 +0200 Subject: [PATCH 19/22] feat: add two factor auth --- OIPA/OIPA/urls.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/OIPA/OIPA/urls.py b/OIPA/OIPA/urls.py index 01e95a234..a68e46872 100644 --- a/OIPA/OIPA/urls.py +++ b/OIPA/OIPA/urls.py @@ -3,16 +3,15 @@ from django.conf.urls.static import static from django.contrib import admin from django.contrib.auth import REDIRECT_FIELD_NAME +from django.contrib.auth.views import redirect_to_login +from django.http import HttpResponseRedirect +from django.shortcuts import resolve_url +from django.urls import reverse +from django.utils.http import is_safe_url from django.views.generic import TemplateView from django.views.generic.base import RedirectView from two_factor.admin import AdminSiteOTPRequired, AdminSiteOTPRequiredMixin from two_factor.urls import urlpatterns as tf_urls -from django.urls import reverse -from django.utils.http import is_safe_url -from django.http import HttpResponseRedirect -from django.shortcuts import resolve_url -from django.contrib.auth.views import redirect_to_login - from OIPA import views From 812ad7167a7258de210d9e3b20537a0d2f5d9961 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Mon, 28 Sep 2020 11:11:27 +0200 Subject: [PATCH 20/22] chore : update signals.py --- OIPA/solr/signals.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/OIPA/solr/signals.py b/OIPA/solr/signals.py index 75661e989..de281e1a3 100644 --- a/OIPA/solr/signals.py +++ b/OIPA/solr/signals.py @@ -88,11 +88,11 @@ def transaction_pre_delete(sender, instance, **kwargs): TransactionTaskIndexing(instance=instance).delete() -@receiver(signals.pre_delete, sender=ActivitySector) -def activity_sector_pre_delete(sender, instance, **kwargs): - ActivitySectorTaskIndexing(instance=instance).delete() - - -@receiver(signals.pre_delete, sender=TransactionSector) -def transaction_sector_pre_delete(sender, instance, **kwargs): - TransactionSectorTaskIndexing(instance=instance).delete() +# @receiver(signals.pre_delete, sender=ActivitySector) +# def activity_sector_pre_delete(sender, instance, **kwargs): +# ActivitySectorTaskIndexing(instance=instance).delete() +# +# +# @receiver(signals.pre_delete, sender=TransactionSector) +# def transaction_sector_pre_delete(sender, instance, **kwargs): +# TransactionSectorTaskIndexing(instance=instance).delete() From fb438749eb8c4d0d6cc06d5111ee82fde631c72b Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Mon, 28 Sep 2020 12:45:23 +0200 Subject: [PATCH 21/22] fix: flake8 error fixed --- OIPA/solr/signals.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/OIPA/solr/signals.py b/OIPA/solr/signals.py index de281e1a3..16bc5618d 100644 --- a/OIPA/solr/signals.py +++ b/OIPA/solr/signals.py @@ -2,12 +2,11 @@ from django.dispatch import receiver from geodata.models import Country, Region -from iati.models import Activity, ActivitySector, Budget -from iati.transaction.models import Transaction, TransactionSector +from iati.models import Activity, Budget +from iati.transaction.models import Transaction from iati_organisation.models import Organisation from iati_synchroniser.models import Dataset, Publisher from solr.activity.tasks import ActivityTaskIndexing -from solr.activity_sector.tasks import ActivitySectorTaskIndexing from solr.budget.tasks import BudgetTaskIndexing from solr.codelists.country.tasks import CodeListCountryTaskIndexing from solr.codelists.region.tasks import CodeListRegionTaskIndexing @@ -15,7 +14,6 @@ from solr.organisation.tasks import OrganisationTaskIndexing from solr.publisher.tasks import PublisherTaskIndexing from solr.transaction.tasks import TransactionTaskIndexing -from solr.transaction_sector.tasks import TransactionSectorTaskIndexing @receiver(signals.post_save, sender=Dataset) @@ -86,13 +84,3 @@ def budget_pre_delete(sender, instance, **kwargs): @receiver(signals.pre_delete, sender=Transaction) def transaction_pre_delete(sender, instance, **kwargs): TransactionTaskIndexing(instance=instance).delete() - - -# @receiver(signals.pre_delete, sender=ActivitySector) -# def activity_sector_pre_delete(sender, instance, **kwargs): -# ActivitySectorTaskIndexing(instance=instance).delete() -# -# -# @receiver(signals.pre_delete, sender=TransactionSector) -# def transaction_sector_pre_delete(sender, instance, **kwargs): -# TransactionSectorTaskIndexing(instance=instance).delete() From 3eda4526c657581b976d9c965a36f6e079cde648 Mon Sep 17 00:00:00 2001 From: Lu Min Han Date: Mon, 28 Sep 2020 13:24:13 +0200 Subject: [PATCH 22/22] Revert "Two Factor Authenticatioon" --- OIPA/OIPA/settings.py | 12 --- OIPA/OIPA/urls.py | 40 ---------- OIPA/templates/two_factor/_base.html | 93 ---------------------- OIPA/templates/two_factor/_base_focus.html | 20 ----- 4 files changed, 165 deletions(-) delete mode 100644 OIPA/templates/two_factor/_base.html delete mode 100644 OIPA/templates/two_factor/_base_focus.html diff --git a/OIPA/OIPA/settings.py b/OIPA/OIPA/settings.py index a488908d1..552e05c19 100644 --- a/OIPA/OIPA/settings.py +++ b/OIPA/OIPA/settings.py @@ -15,7 +15,6 @@ LOGIN_REDIRECT_URL = '/admin/' LOGOUT_URL = '/logout' -LOGOUT_REDIRECT_URL = '/admin/logout' DATA_UPLOAD_MAX_NUMBER_FIELDS = 3000 SECRET_KEY = env.get('OIPA_SECRET_KEY', 'PXwlMOpfNJTgIdQeH5zk39jKfUMZPOUK') @@ -145,19 +144,11 @@ def rel(*x): 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'api.middleware.FileExportMiddleware', - 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django_otp.middleware.OTPMiddleware', ] ROOT_URLCONF = 'OIPA.urls' INSTALLED_APPS = [ - # 'two-factor - 'django_otp', - 'django_otp.plugins.otp_static', - 'django_otp.plugins.otp_totp', - 'two_factor', - # 'django_rq', 'django.contrib.auth', 'django.contrib.contenttypes', @@ -253,9 +244,6 @@ def rel(*x): 'DEFAULT_TIMEOUT': 10800, } } -TWO_FACTOR_FORCE_OTP_ADMIN = True -LOGIN_URL = 'two_factor:login' -LOGIN_REDIRECT_URL = '/admin' # Redirect admin dashboard GRAPPELLI_ADMIN_TITLE = 'OIPA admin' ADMINFILES_UPLOAD_TO = 'csv_files' diff --git a/OIPA/OIPA/urls.py b/OIPA/OIPA/urls.py index a68e46872..9e61eae31 100644 --- a/OIPA/OIPA/urls.py +++ b/OIPA/OIPA/urls.py @@ -2,52 +2,13 @@ from django.conf.urls import include, url from django.conf.urls.static import static from django.contrib import admin -from django.contrib.auth import REDIRECT_FIELD_NAME -from django.contrib.auth.views import redirect_to_login -from django.http import HttpResponseRedirect -from django.shortcuts import resolve_url -from django.urls import reverse -from django.utils.http import is_safe_url from django.views.generic import TemplateView from django.views.generic.base import RedirectView -from two_factor.admin import AdminSiteOTPRequired, AdminSiteOTPRequiredMixin -from two_factor.urls import urlpatterns as tf_urls from OIPA import views admin.autodiscover() - -class AdminSiteOTPRequiredMixinRedirSetup(AdminSiteOTPRequired): - def login(self, request, extra_context=None): - redirect_to = request.POST.get( - REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME) - ) - # For users not yet verified the AdminSiteOTPRequired.has_permission - # will fail. So use the standard admin has_permission check: - # (is_active and is_staff) and then check for verification. - # Go to index if they pass, otherwise make them setup OTP device. - if request.method == "GET" and super( - AdminSiteOTPRequiredMixin, self - ).has_permission(request): - # Already logged-in and verified by OTP - if request.user.is_verified(): - # User has permission - index_path = reverse("admin:index", current_app=self.name) - else: - # User has permission but no OTP set: - index_path = reverse("two_factor:setup", current_app=self.name) - return HttpResponseRedirect(index_path) - - if not redirect_to or not is_safe_url( - url=redirect_to, allowed_hosts=[request.get_host()] - ): - redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL) - - return redirect_to_login(redirect_to) - - -admin.site.__class__ = AdminSiteOTPRequiredMixinRedirSetup urlpatterns = [ # url(r'^grappelli/', include('grappelli.urls')), url(r'^admin/queue/', include('django_rq.urls')), @@ -61,7 +22,6 @@ def login(self, request, extra_context=None): url(r'^about$', TemplateView.as_view(template_name='home/about.html')), url(r'^accounts/profile/', RedirectView.as_view(url='/admin')), url(r'^$', RedirectView.as_view(url='/home', permanent=True)), - url(r'', include(tf_urls, "two_factor")), ] handler404 = views.error404 diff --git a/OIPA/templates/two_factor/_base.html b/OIPA/templates/two_factor/_base.html deleted file mode 100644 index 2c99ba2c1..000000000 --- a/OIPA/templates/two_factor/_base.html +++ /dev/null @@ -1,93 +0,0 @@ -{% load i18n static %} -{% get_current_language as LANGUAGE_CODE %}{% get_current_language_bidi as LANGUAGE_BIDI %} - - -{% block title %}{% endblock %} - -{% block extrastyle %}{% endblock %} -{% if LANGUAGE_BIDI %}{% endif %} -{% block extrahead %}{% endblock %} -{% block responsive %} - - - {% if LANGUAGE_BIDI %}{% endif %} -{% endblock %} -{% block blockbots %}{% endblock %} - -{% load i18n %} - - - - -
- - {% if not is_popup %} - - - - {% block breadcrumbs %} -
- {% trans 'Home' %} - {% if title %} › {{ title }}{% endif %} -
- {% endblock %} - {% endif %} - - {% block messages %} - {% if messages %} -
    {% for message in messages %} - {{ message|capfirst }} - {% endfor %}
- {% endif %} - {% endblock messages %} - - -
- {% block pretitle %}{% endblock %} - {% block content_title %}{% if title %}

{{ title }}

{% endif %}{% endblock %} - {% block content %} - {% block object-tools %}{% endblock %} - {{ content }} - {% endblock %} - {% block sidebar %}{% endblock %} -
-
- - - {% block footer %}{% endblock %} -
- - - - \ No newline at end of file diff --git a/OIPA/templates/two_factor/_base_focus.html b/OIPA/templates/two_factor/_base_focus.html deleted file mode 100644 index 0b85ec376..000000000 --- a/OIPA/templates/two_factor/_base_focus.html +++ /dev/null @@ -1,20 +0,0 @@ -{% extends "admin/base_site.html" %} -{% load i18n static %} - -{% block extrastyle %}{{ block.super }} -{{ form.media }} -{% endblock %} - -{% block bodyclass %}{{ block.super }} login{% endblock %} - -{% block usertools %}{% endblock %} - -{% block nav-global %}{% endblock %} - -{% block content_title %}{% endblock %} - -{% block breadcrumbs %}{% endblock %} - -{% block content %} - -{% endblock %}