From 3eac767caef69526f0002a0a1abfe4e643141edc Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 4 Feb 2020 17:00:36 +0000 Subject: [PATCH 1/5] Fix error on opportunity page --- great_international/serializers.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/great_international/serializers.py b/great_international/serializers.py index c4c03505..d2c631fd 100644 --- a/great_international/serializers.py +++ b/great_international/serializers.py @@ -1,3 +1,5 @@ +import random + from directory_constants import cms from rest_framework import serializers from wagtail.images.api import fields as wagtail_fields @@ -1637,16 +1639,24 @@ def get_sub_sectors(self, instance): related_opportunities = serializers.SerializerMethodField() def get_related_opportunities(self, instance): - random_sector = instance.related_sectors.order_by('?').first() + random_sector_id = random.choice( + instance.related_sectors.values_list('related_sector_id', flat=True)) + + if random_sector_id: + + related_opps_ids = CapitalInvestOpportunityPage.objects.filter( + related_sectors__related_sector_id=random_sector_id + ).exclude(id=instance.id).values_list('pk', flat=True) + + random_ids = random.sample(list(related_opps_ids), 3) - if random_sector: + related_opps = [] - three_random_opps = CapitalInvestOpportunityPage.objects.filter( - related_sectors__related_sector_id=random_sector.related_sector_id - ).order_by('?').exclude(id=instance.id)[:3] + for id in random_ids: + related_opps.append(CapitalInvestOpportunityPage.objects.get(pk=id)) serializer = RelatedCapitalInvestOpportunityPageSerializer( - three_random_opps, + related_opps, many=True, allow_null=True, context=self.context From 6fce8642b644663247de486e76cf7505a1cfff92 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 4 Feb 2020 17:00:59 +0000 Subject: [PATCH 2/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7d19dde..47130e02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - XOT-1275 - Add MADB link field to country guide ### Fixed bugs +No ticket - Fix error on opportunity page ## [2020.01.31](https://github.com/uktrade/directory-cms/releases/tag/2020.01.31) [Full Changelog](https://github.com/uktrade/directory-cms/compare/2020.01.29...2020.01.31) From e15791beab220bcb94137cc3d8773a5f0e4d915c Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 5 Feb 2020 10:56:11 +0000 Subject: [PATCH 3/5] Fix tests --- great_international/serializers.py | 41 +++++++++++++++++------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/great_international/serializers.py b/great_international/serializers.py index d2c631fd..cf941cab 100644 --- a/great_international/serializers.py +++ b/great_international/serializers.py @@ -1639,31 +1639,38 @@ def get_sub_sectors(self, instance): related_opportunities = serializers.SerializerMethodField() def get_related_opportunities(self, instance): - random_sector_id = random.choice( - instance.related_sectors.values_list('related_sector_id', flat=True)) + related_sectors_ids = instance.related_sectors.values_list('related_sector_id', flat=True) - if random_sector_id: + if not related_sectors_ids: + return [] + + random_sector_id = random.choice(related_sectors_ids) + + related_opps_ids = CapitalInvestOpportunityPage.objects.filter( + related_sectors__related_sector_id=random_sector_id + ).exclude(id=instance.id).values_list('pk', flat=True) - related_opps_ids = CapitalInvestOpportunityPage.objects.filter( - related_sectors__related_sector_id=random_sector_id - ).exclude(id=instance.id).values_list('pk', flat=True) + if not related_opps_ids: + return [] + elif len(related_opps_ids) > 3: random_ids = random.sample(list(related_opps_ids), 3) - related_opps = [] + else: + random_ids = related_opps_ids - for id in random_ids: - related_opps.append(CapitalInvestOpportunityPage.objects.get(pk=id)) + related_opps = [] - serializer = RelatedCapitalInvestOpportunityPageSerializer( - related_opps, - many=True, - allow_null=True, - context=self.context - ) - return serializer.data + for id in random_ids: + related_opps.append(CapitalInvestOpportunityPage.objects.get(pk=id)) - return [] + serializer = RelatedCapitalInvestOpportunityPageSerializer( + related_opps, + many=True, + allow_null=True, + context=self.context + ) + return serializer.data class MinimalPageSerializer(BasePageSerializer): From f4c873896482e4bf686e569e3f3391b3f6326506 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 5 Feb 2020 12:15:37 +0000 Subject: [PATCH 4/5] Fix tests --- great_international/models/capital_invest.py | 4 +- tests/great_international/factories.py | 8 ++ tests/great_international/test_serializers.py | 107 +++++++++++------- 3 files changed, 77 insertions(+), 42 deletions(-) diff --git a/great_international/models/capital_invest.py b/great_international/models/capital_invest.py index b04f4309..e8821253 100644 --- a/great_international/models/capital_invest.py +++ b/great_international/models/capital_invest.py @@ -544,9 +544,7 @@ class CapitalInvestRelatedSubSectors(Orderable, RelatedSubSector): ) -class CapitalInvestOpportunityPage( - panels.CapitalInvestOpportunityPagePanels, BaseInternationalPage -): +class CapitalInvestOpportunityPage(panels.CapitalInvestOpportunityPagePanels, BaseInternationalPage): parent_page_types = [ 'great_international.CapitalInvestOpportunityListingPage' diff --git a/tests/great_international/factories.py b/tests/great_international/factories.py index 6318ed58..c717d7ca 100644 --- a/tests/great_international/factories.py +++ b/tests/great_international/factories.py @@ -412,6 +412,14 @@ class Meta: parent = None +class CapitalInvestRelatedSectorsFactory(factory.django.DjangoModelFactory): + class Meta: + model = models.capital_invest.CapitalInvestRelatedSectors + + page = None + related_sector = None + + class CapitalInvestOpportunityListingPageFactory( wagtail_factories.PageFactory ): diff --git a/tests/great_international/test_serializers.py b/tests/great_international/test_serializers.py index 91e227f9..a16fa222 100644 --- a/tests/great_international/test_serializers.py +++ b/tests/great_international/test_serializers.py @@ -34,7 +34,8 @@ AboutUkLandingPageFactory, InvestInternationalHomePageFactory, CapitalInvestRegionPageFactory, AboutUkRegionListingPageFactory, InvestRegionPageFactory, - InternationalTradeHomePageFactory) + InternationalTradeHomePageFactory, CapitalInvestRelatedSectorsFactory +) from great_international.models.capital_invest import ( CapitalInvestRelatedRegions, @@ -511,8 +512,7 @@ def test_capital_invest_landing_page_has_how_we_help( @pytest.mark.django_db -def test_opportunity_page_can_add_sector_as_related(rf, - international_root_page): +def test_opportunity_page_can_add_sector_as_related(rf, international_root_page): guide_landing_page = InternationalGuideLandingPageFactory( parent=international_root_page, @@ -524,15 +524,13 @@ def test_opportunity_page_can_add_sector_as_related(rf, slug='sector' ) - related_sector = CapitalInvestRelatedSectors( - related_sector=sector - ) - opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='opp', - related_sectors=[related_sector] + related_sectors=[] ) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector, page=opportunity) + opportunity.related_sector = related_sector opportunity_serializer = CapitalInvestOpportunityPageSerializer( instance=opportunity, @@ -558,15 +556,13 @@ def test_international_sector_page_gets_opps_with_sector_as_related( slug='sector' ) - related_sector = CapitalInvestRelatedSectors( - related_sector=sector - ) - opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='opp', - related_sectors=[related_sector] + related_sectors=[] ) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector, page=opportunity) + opportunity.related_sector = related_sector opportunity_serializer = CapitalInvestOpportunityPageSerializer( instance=opportunity, @@ -588,13 +584,13 @@ def test_international_sector_page_gets_opps_with_sector_as_related( @pytest.mark.django_db def test_opp_page_null_case_related_sector(rf, international_root_page): - related_sector = CapitalInvestRelatedSectors() - opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='opp', - related_sectors=[related_sector] + related_sectors=[] ) + related_sector = CapitalInvestRelatedSectorsFactory(page=opportunity) + opportunity.related_sector = related_sector opportunity_serializer = CapitalInvestOpportunityPageSerializer( instance=opportunity, @@ -606,9 +602,46 @@ def test_opp_page_null_case_related_sector(rf, international_root_page): @pytest.mark.django_db -def test_opp_page_null_case_related_sector2( - rf, international_root_page -): +def test_opp_page_related_random_opps(rf, international_root_page): + + guide_landing_page = InternationalGuideLandingPageFactory( + parent=international_root_page, + slug='page-slug', + ) + + sector = InternationalSectorPageFactory( + parent=guide_landing_page, + slug='sector' + ) + + opportunity = CapitalInvestOpportunityPageFactory( + parent=international_root_page, + slug='opp', + related_sectors=[] + ) + + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector, page=opportunity) + opportunity.related_sectors = [related_sector] + + for i in range(3): + opp = CapitalInvestOpportunityPageFactory( + parent=international_root_page, + slug=f'opp{i}', + related_sectors=[related_sector] + ) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector, page=opp) + opp.related_sectors = [related_sector] + + serializer = CapitalInvestOpportunityPageSerializer( + instance=opportunity, + context={'request': rf.get('/')} + ) + + assert len(serializer.data['related_opportunities']) == 3 + + +@pytest.mark.django_db +def test_opp_page_null_case_related_sector2(rf, international_root_page): opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, @@ -644,15 +677,13 @@ def test_international_sector_opportunity_null_case( slug='sectorB' ) - related_sector = CapitalInvestRelatedSectors( - related_sector=sector_a - ) - opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='opp', - related_sectors=[related_sector] + related_sectors=[] ) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector_a, page=opportunity) + opportunity.related_sector = related_sector opportunity_serializer = CapitalInvestOpportunityPageSerializer( instance=opportunity, @@ -685,12 +716,10 @@ def test_international_sector_opportunity_null_case2( slug='sector' ) - related_sector = CapitalInvestRelatedSectors() - opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='opp', - related_sectors=[related_sector] + related_sectors=[] ) opportunity_serializer = CapitalInvestOpportunityPageSerializer( @@ -953,30 +982,30 @@ def test_opportunity_page_gets_opportunities_with_same_sector(rf, international_ title='sector_title' ) - CapitalInvestOpportunityPageFactory( + ashton_green = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='ashton-green', title_en_gb='Ashton Green', - related_sectors=[ - CapitalInvestRelatedSectors(related_sector=sector) - ] + related_sectors=[] ) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector, page=ashton_green) + ashton_green.related_sector = related_sector - birmingham_opportuntiy = CapitalInvestOpportunityPageFactory( + birmingham = CapitalInvestOpportunityPageFactory( parent=international_root_page, slug='birimingham-curzon', title_en_gb='Birmingham Curzon', - related_sectors=[ - CapitalInvestRelatedSectors(related_sector=sector) - ] + related_sectors=[] ) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector, page=birmingham) + ashton_green.related_sector = related_sector - birmingham_serializer = CapitalInvestOpportunityPageSerializer( - instance=birmingham_opportuntiy, + serializer = CapitalInvestOpportunityPageSerializer( + instance=birmingham, context={'request': rf.get('/')} ) - related_opps = birmingham_serializer.data['related_opportunities'] + related_opps = serializer.data['related_opportunities'] assert len(related_opps) == 1 @@ -996,7 +1025,7 @@ def test_opportunity_page_null_case_gets_opportunities_with_same_sector(rf, inte title='sector_title' ) - related_sector = CapitalInvestRelatedSectors(related_sector=sector) + related_sector = CapitalInvestRelatedSectorsFactory(related_sector=sector) opportunity = CapitalInvestOpportunityPageFactory( parent=international_root_page, From 7e3e278e9252133ecfcd347eeb628e75bec60bfb Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 5 Feb 2020 13:15:35 +0000 Subject: [PATCH 5/5] Fix flake8 --- tests/great_international/test_serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/great_international/test_serializers.py b/tests/great_international/test_serializers.py index a16fa222..fe9cf38c 100644 --- a/tests/great_international/test_serializers.py +++ b/tests/great_international/test_serializers.py @@ -41,7 +41,6 @@ CapitalInvestRelatedRegions, CapitalInvestHomesInEnglandCardFieldsSummary, CapitalInvestRegionCardFieldsSummary, - CapitalInvestRelatedSectors, CapitalInvestRelatedSubSectors ) from great_international.models.great_international import (