Sponsorship (maybe) final UX bugfixes (#1682)

* Separate package benefits in application form

* Flag sponsors benefits added by user

* Split package and added benefits in full sponsorship detail

* Filter by package only benefits

Author: berinhard

sponsors/admin.py

@@ -40,7 +40,7 @@ class SponsorshipBenefitAdmin(OrderedModelAdmin):
         "internal_value",
         "move_up_down_links",
     ]
-    list_filter = ["program"]
+    list_filter = ["program", "package_only"]
     search_fields = ["name"]
 
     fieldsets = [

sponsors/migrations/0017_sponsorbenefit_added_by_user.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.0.13 on 2020-11-20 14:43
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("sponsors", "0016_auto_20201119_1448"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="sponsorbenefit",
+            name="added_by_user",
+            field=models.BooleanField(default=False, verbose_name="Added by user?"),
+        ),
+    ]

sponsors/models.py

@@ -4,6 +4,7 @@
 from django.db.models import Sum
 from django.template.defaultfilters import truncatechars
 from django.utils import timezone
+from django.utils.functional import cached_property
 from django.urls import reverse
 from markupfield.fields import MarkupField
 from ordered_model.models import OrderedModel, OrderedModelManager
@@ -266,7 +267,9 @@ def new(cls, sponsor, benefits, package=None, submited_by=None):
         This will create SponsorBenefit copies from the benefits
         """
         for_modified_package = False
+        package_benefits = []
         if package and package.has_user_customization(benefits):
+            package_benefits = package.benefits.all()
             for_modified_package = True
         elif not package:
             for_modified_package = True
@@ -283,13 +286,16 @@ def new(cls, sponsor, benefits, package=None, submited_by=None):
         )
 
         for benefit in benefits:
+            added_by_user = for_modified_package and benefit not in package_benefits
+
             SponsorBenefit.objects.create(
                 sponsorship=sponsorship,
                 sponsorship_benefit=benefit,
                 name=benefit.name,
                 description=benefit.description,
                 program=benefit.program,
                 benefit_internal_value=benefit.internal_value,
+                added_by_user=added_by_user,
             )
 
         return sponsorship
@@ -322,6 +328,14 @@ def verified_emails(self):
     def admin_url(self):
         return reverse("admin:sponsors_sponsorship_change", args=[self.pk])
 
+    @cached_property
+    def package_benefits(self):
+        return self.benefits.filter(added_by_user=False)
+
+    @cached_property
+    def added_benefits(self):
+        return self.benefits.filter(added_by_user=True)
+
 
 class SponsorBenefit(models.Model):
     sponsorship = models.ForeignKey(
@@ -359,6 +373,9 @@ class SponsorBenefit(models.Model):
         verbose_name="Benefit Internal Value",
         help_text=("Benefit's internal value from when the Sponsorship gets created"),
     )
+    added_by_user = models.BooleanField(
+        blank=True, default=False, verbose_name="Added by user?"
+    )
 
 
 class Sponsor(ContentManageable):

sponsors/tests/test_models.py

@@ -64,6 +64,7 @@ def test_create_new_sponsorship(self):
         self.assertEqual(sponsorship.benefits.count(), len(self.benefits))
         for benefit in self.benefits:
             sponsor_benefit = sponsorship.benefits.get(sponsorship_benefit=benefit)
+            self.assertTrue(sponsor_benefit.added_by_user)
             self.assertEqual(sponsor_benefit.name, benefit.name)
             self.assertEqual(sponsor_benefit.description, benefit.description)
             self.assertEqual(sponsor_benefit.program, benefit.program)
@@ -79,6 +80,8 @@ def test_create_new_sponsorship_with_package(self):
         self.assertEqual(sponsorship.level_name, "PSF Sponsorship Program")
         self.assertEqual(sponsorship.sponsorship_fee, 100)
         self.assertFalse(sponsorship.for_modified_package)
+        for benefit in sponsorship.benefits.all():
+            self.assertFalse(benefit.added_by_user)
 
     def test_create_new_sponsorship_with_package_modifications(self):
         benefits = self.benefits[:2]
@@ -88,6 +91,24 @@ def test_create_new_sponsorship_with_package_modifications(self):
 
         self.assertTrue(sponsorship.for_modified_package)
         self.assertEqual(sponsorship.benefits.count(), 2)
+        for benefit in sponsorship.benefits.all():
+            self.assertFalse(benefit.added_by_user)
+
+    def test_create_new_sponsorship_with_package_added_benefit(self):
+        extra_benefit = baker.make(SponsorshipBenefit)
+        benefits = self.benefits + [extra_benefit]
+        sponsorship = Sponsorship.new(self.sponsor, benefits, package=self.package)
+        sponsorship.refresh_from_db()
+
+        self.assertTrue(sponsorship.for_modified_package)
+        self.assertEqual(sponsorship.benefits.count(), 6)
+        for benefit in self.benefits:
+            sponsor_benefit = sponsorship.benefits.get(sponsorship_benefit=benefit)
+            self.assertFalse(sponsor_benefit.added_by_user)
+            self.assertIn(sponsor_benefit, sponsorship.package_benefits)
+        sponsor_benefit = sponsorship.benefits.get(sponsorship_benefit=extra_benefit)
+        self.assertTrue(sponsor_benefit.added_by_user)
+        self.assertEqual([sponsor_benefit], list(sponsorship.added_benefits))
 
     def test_estimated_cost_property(self):
         sponsorship = Sponsorship.new(self.sponsor, self.benefits)

sponsors/tests/test_views.py

@@ -191,6 +191,7 @@ def test_display_template_with_form_and_context(self):
         self.assertEqual(
             len(r.context["sponsorship_benefits"]), len(self.program_1_benefits)
         )
+        self.assertEqual(len(r.context["added_benefits"]), 0)
         self.assertEqual(
             r.context["sponsorship_price"], self.package.sponsorship_amount
         )
@@ -206,6 +207,8 @@ def test_return_package_as_none_if_not_previously_selected(self):
         r = self.client.get(self.url)
         self.assertIsNone(r.context["sponsorship_package"])
         self.assertIsNone(r.context["sponsorship_price"])
+        self.assertEqual(len(r.context["added_benefits"]), len(self.program_1_benefits))
+        self.assertEqual(len(r.context["sponsorship_benefits"]), 0)
 
     def test_no_sponsorship_price_if_customized_benefits(self):
         extra_benefit = baker.make(SponsorshipBenefit)
@@ -221,8 +224,9 @@ def test_no_sponsorship_price_if_customized_benefits(self):
 
         self.assertEqual(r.context["sponsorship_package"], self.package)
         self.assertIsNone(r.context["sponsorship_price"])
-        for benefit in benefits:
+        for benefit in self.program_1_benefits:
             self.assertIn(benefit, r.context["sponsorship_benefits"])
+        self.assertIn(extra_benefit, r.context["added_benefits"])
 
     def test_display_form_with_errors_if_invalid_post(self):
         r = self.client.post(self.url, {})

sponsors/views.py

@@ -122,15 +122,29 @@ def get_context_data(self, *args, **kwargs):
             *[self.benefits_data[k] for k in self.benefits_data if k != "package"]
         )
         benefits = SponsorshipBenefit.objects.filter(id__in=benefits_ids)
-        price = None
 
+        # sponsorship benefits holds selected package's benefits
+        # added benefits holds holds extra benefits added by users
+        added_benefits, sponsorship_benefits = [], benefits
+        price = None
         if package and not package.has_user_customization(benefits):
             price = package.sponsorship_amount
+        elif package:
+            sponsorship_benefits = []
+            package_benefits = package.benefits.all()
+            for benefit in benefits:
+                if benefit in package_benefits:
+                    sponsorship_benefits.append(benefit)
+                else:
+                    added_benefits.append(benefit)
+        else:
+            added_benefits, sponsorship_benefits = sponsorship_benefits, []
 
         kwargs.update(
             {
                 "sponsorship_package": package,
-                "sponsorship_benefits": benefits,
+                "sponsorship_benefits": sponsorship_benefits,
+                "added_benefits": added_benefits,
                 "sponsorship_price": price,
             }
         )

templates/sponsors/new_sponsorship_application_form.html

@@ -10,18 +10,25 @@
     <h1>Submit Sponsorship Information</h1>
 
     <div id="sponsorship-info-container" class="user-feedback level-general">
-      <p>
       {% if sponsorship_package %}
+      <p>
         You selected the <b>{{ sponsorship_package.name }}</b> package {% if sponsorship_price %}costing ${{ sponsorship_price|intcomma }} USD {% endif %}and the following benefits:
-      {% else %}
-        You selected the following benefits:
-      {% endif %}
       </p>
       <ul>
         {% for benefit in sponsorship_benefits %}
           <li>{{ benefit.name }}</li>
         {% endfor %}
       </ul>
+      {% endif %}
+
+      {% if added_benefits %}
+      <p>Added benefits:</p>
+      <ul>
+        {% for benefit in added_benefits %}
+          <li>{{ benefit.name }}</li>
+        {% endfor %}
+      </ul>
+      {% endif %}
       <p><i>Please complete the form below.</i></p>
       <span class="remove"><label id="close-info-container">Close</label> | <a href="{% url 'select_sponsorship_application_benefits' %}">Back to select benefits</a></span>
     </div>

templates/sponsors/partials/full_sponsorship.txt

@@ -11,7 +11,9 @@
 # Sponsorship Information
   * Status		{{ sponsorship.status }}
 {% if sponsorship.level_name %}  * Level		{{ sponsorship.level_name }}{% else %}  * Level			Custom Package{% endif %}
-{% if for_modified_package %}  * Sponsorship Fee	To be determined{% else %}  * Sponsorship Fee	${{ sponsorship.sponsorship_fee|intcomma }} USD{% endif %}
-  * Benefits
-{% for benefit in  benefits %}    - {{ benefit.program.name }} - {{ benefit.name }}
-{% endfor %}
+{% if sponsorship.for_modified_package %}  * Sponsorship Fee	To be determined{% else %}  * Sponsorship Fee	${{ sponsorship.sponsorship_fee|intcomma }} USD{% endif %}
+  {% if sponsorship.package_benefits %}* Package Benefits
+{% for benefit in  sponsorship.package_benefits %}    - {{ benefit.program.name }} - {{ benefit.name }}
+{% endfor %}{% endif %}  {% if sponsorship.added_benefits %}* Added Benefits
+{% for benefit in  sponsorship.added_benefits %}    - {{ benefit.program.name }} - {{ benefit.name }}
+{% endfor %}{% endif %}