Enable admin users to edit benefits from sponsorship application via …

…django admin (#1710)

* New admin form to manage sponsorship's benefits

* Can only edit benefits if sponsorship wasn't approved or rejected

* black

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'<p>{city_row}</p>'
-        html += f'<p>{mail_row}</p>'
-        html += f'<p>{sponsor.postal_code}</p>'
+        html = f"<p>{city_row}</p>"
+        html += f"<p>{mail_row}</p>"
+        html += f"<p>{sponsor.postal_code}</p>"
         return mark_safe(html)
 
     get_sponsor_mailing_address.short_description = "Mailing/Billing Address"

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

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 = {

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)