diff --git a/sponsors/admin.py b/sponsors/admin.py index 5ca16f960..405ad9cbe 100644 --- a/sponsors/admin.py +++ b/sponsors/admin.py @@ -18,7 +18,7 @@ LegalClause, ) from sponsors import use_cases -from sponsors.forms import SponsorshipReviewAdminForm +from sponsors.forms import SponsorshipReviewAdminForm, SponsorBenefitAdminInlineForm from sponsors.exceptions import SponsorshipInvalidStatusException from cms.admin import ContentManageableModelAdmin @@ -94,10 +94,29 @@ class SponsorAdmin(ContentManageableModelAdmin): class SponsorBenefitInline(admin.TabularInline): model = SponsorBenefit - readonly_fields = ["name", "benefit_internal_value"] - fields = ["name", "benefit_internal_value"] + form = SponsorBenefitAdminInlineForm + fields = ["sponsorship_benefit", "benefit_internal_value"] extra = 0 - can_delete = False + + def has_add_permission(self, request): + # this work around is necessary because the `obj` parameter was added to + # InlineModelAdmin.has_add_permission only in Django 2.1.x and we're using 2.0.x + has_add_permission = super().has_add_permission(request) + match = request.resolver_match + if match.url_name == "sponsors_sponsorship_change": + sponsorship = self.parent_model.objects.get(pk=match.kwargs["object_id"]) + has_add_permission = has_add_permission and sponsorship.open_for_editing + return has_add_permission + + def get_readonly_fields(self, request, obj=None): + if obj and not obj.open_for_editing: + return ["sponsorship_benefit", "benefit_internal_value"] + return [] + + def has_delete_permission(self, request, obj=None): + if not obj: + return True + return obj.open_for_editing @admin.register(Sponsorship) @@ -265,17 +284,19 @@ def get_sponsor_primary_phone(self, obj): def get_sponsor_mailing_address(self, obj): sponsor = obj.sponsor - city_row = f'{sponsor.city} - {sponsor.get_country_display()} ({sponsor.country})' + city_row = ( + f"{sponsor.city} - {sponsor.get_country_display()} ({sponsor.country})" + ) if sponsor.state: - city_row = f'{sponsor.city} - {sponsor.state} - {sponsor.get_country_display()} ({sponsor.country})' + city_row = f"{sponsor.city} - {sponsor.state} - {sponsor.get_country_display()} ({sponsor.country})" mail_row = sponsor.mailing_address_line_1 if sponsor.mailing_address_line_2: - mail_row += f' - {sponsor.mailing_address_line_2}' + mail_row += f" - {sponsor.mailing_address_line_2}" - html = f'
{city_row}
' - html += f'{mail_row}
' - html += f'{sponsor.postal_code}
' + html = f"{city_row}
" + html += f"{mail_row}
" + html += f"{sponsor.postal_code}
" return mark_safe(html) get_sponsor_mailing_address.short_description = "Mailing/Billing Address" diff --git a/sponsors/forms.py b/sponsors/forms.py index 92478b242..6c0e8cb4e 100644 --- a/sponsors/forms.py +++ b/sponsors/forms.py @@ -13,6 +13,7 @@ Sponsor, SponsorContact, Sponsorship, + SponsorBenefit, ) @@ -331,3 +332,40 @@ def clean(self): raise forms.ValidationError("End date must be greater than start date") return cleaned_data + + +class SponsorBenefitAdminInlineForm(forms.ModelForm): + sponsorship_benefit = forms.ModelChoiceField( + queryset=SponsorshipBenefit.objects.select_related("program"), + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if "benefit_internal_value" in self.fields: + self.fields["benefit_internal_value"].required = True + + class Meta: + model = SponsorBenefit + fields = ["sponsorship_benefit", "sponsorship", "benefit_internal_value"] + + def save(self, commit=True): + sponsorship = self.cleaned_data["sponsorship"] + benefit = self.cleaned_data["sponsorship_benefit"] + value = self.cleaned_data["benefit_internal_value"] + + if not (self.instance and self.instance.pk): # new benefit + self.instance = SponsorBenefit(sponsorship=sponsorship) + else: + self.instance.refresh_from_db() + + self.instance.benefit_internal_value = value + if benefit.pk != self.instance.sponsorship_benefit_id: + self.instance.sponsorship_benefit = benefit + self.instance.name = benefit.name + self.instance.description = benefit.description + self.instance.program = benefit.program + + if commit: + self.instance.save() + + return self.instance diff --git a/sponsors/models.py b/sponsors/models.py index fd8d28856..d0a90cbe5 100644 --- a/sponsors/models.py +++ b/sponsors/models.py @@ -361,6 +361,10 @@ def package_benefits(self): def added_benefits(self): return self.benefits.filter(added_by_user=True) + @property + def open_for_editing(self): + return self.status == self.APPLIED + @property def next_status(self): states_map = { diff --git a/sponsors/tests/test_forms.py b/sponsors/tests/test_forms.py index d473bdfea..4edcbea3e 100644 --- a/sponsors/tests/test_forms.py +++ b/sponsors/tests/test_forms.py @@ -9,6 +9,9 @@ SponsorContact, Sponsor, SponsorContactFormSet, + SponsorBenefitAdminInlineForm, + SponsorBenefit, + Sponsorship, ) from sponsors.models import SponsorshipBenefit, SponsorContact from .utils import get_static_image_file_as_upload @@ -355,3 +358,85 @@ def test_invalidate_formset_if_no_form(self): self.data["contact-TOTAL_FORMS"] = 0 formset = SponsorContactFormSet(self.data, prefix="contact") self.assertFalse(formset.is_valid()) + + +class SponsorBenefitAdminInlineFormTests(TestCase): + def setUp(self): + self.benefit = baker.make(SponsorshipBenefit) + self.sponsorship = baker.make(Sponsorship) + self.data = { + "sponsorship_benefit": self.benefit.pk, + "sponsorship": self.sponsorship.pk, + "benefit_internal_value": 200, + } + + def test_required_fields_for_new_sponsor_benefit(self): + required_fields = [ + "sponsorship_benefit", + "sponsorship", + "benefit_internal_value", + ] + + form = SponsorBenefitAdminInlineForm({}) + self.assertFalse(form.is_valid()) + + for required in required_fields: + self.assertIn(required, form.errors) + self.assertEqual(len(required_fields), len(form.errors)) + + def test_create_new_sponsor_benefit_for_sponsorship(self): + form = SponsorBenefitAdminInlineForm(data=self.data) + self.assertTrue(form.is_valid(), form.errors) + + sponsor_benefit = form.save() + sponsor_benefit.refresh_from_db() + + self.assertEqual(sponsor_benefit.sponsorship, self.sponsorship) + self.assertEqual(sponsor_benefit.sponsorship_benefit, self.benefit) + self.assertEqual(sponsor_benefit.name, self.benefit.name) + self.assertEqual(sponsor_benefit.description, self.benefit.description) + self.assertEqual(sponsor_benefit.program, self.benefit.program) + self.assertEqual(sponsor_benefit.benefit_internal_value, 200) + + def test_update_existing_sponsor_benefit(self): + sponsor_benefit = baker.make( + SponsorBenefit, + sponsorship=self.sponsorship, + sponsorship_benefit=self.benefit, + ) + new_benefit = baker.make(SponsorshipBenefit) + self.data["sponsorship_benefit"] = new_benefit.pk + + form = SponsorBenefitAdminInlineForm(data=self.data, instance=sponsor_benefit) + self.assertTrue(form.is_valid(), form.errors) + form.save() + sponsor_benefit.refresh_from_db() + + self.assertEqual(1, SponsorBenefit.objects.count()) + self.assertEqual(sponsor_benefit.sponsorship, self.sponsorship) + self.assertEqual(sponsor_benefit.sponsorship_benefit, new_benefit) + self.assertEqual(sponsor_benefit.name, new_benefit.name) + self.assertEqual(sponsor_benefit.description, new_benefit.description) + self.assertEqual(sponsor_benefit.program, new_benefit.program) + self.assertEqual(sponsor_benefit.benefit_internal_value, 200) + + def test_do_not_update_sponsorship_if_it_doesn_change(self): + sponsor_benefit = baker.make( + SponsorBenefit, + sponsorship=self.sponsorship, + sponsorship_benefit=self.benefit, + ) + new_benefit = baker.make(SponsorshipBenefit) + + form = SponsorBenefitAdminInlineForm(data=self.data, instance=sponsor_benefit) + self.assertTrue(form.is_valid(), form.errors) + form.save() + sponsor_benefit.refresh_from_db() + self.benefit.name = "new name" + self.benefit.save() + + self.assertEqual(1, SponsorBenefit.objects.count()) + self.assertEqual(sponsor_benefit.sponsorship, self.sponsorship) + self.assertEqual(sponsor_benefit.sponsorship_benefit, self.benefit) + self.assertNotEqual(sponsor_benefit.name, "new name") + self.assertEqual(sponsor_benefit.benefit_internal_value, 200)