Add a way for sponsors to pay without asking for logos etc. #2503
Conversation
ericholscher
added
the
PR: work in progress
ericholscher
force-pushed
the
add-payment-form
branch
from
8c0d960
to
047bfa6
Compare
ericholscher
added
PR: ready for review
and removed
PR: work in progress
There was a problem hiding this comment.
I'm -1 on shoehorning this into donations, this is half-baked. Fine for now, as it doesn't seem like this is going to be used much yet.
What we should actually be shipping here is an invoice feature, with set amounts on the payments/etc. I don't want to see this implemented in RTD though.
| email = forms.CharField(required=True) | ||
|
|
||
| def validate_stripe(self): | ||
| """Call stripe for payment (not ideal here) and clean up logo < $200""" |
|
|
||
| <p> | ||
| We appreciate your contribution greatly, thank you for showing your support! | ||
| Your help will go a long ways towards making the service more sustainable. |
There was a problem hiding this comment.
This should instead mention buying an ad, marketing departments aren't donating to us to make us more sustainable.
| {% load i18n %} | ||
| {% load static %} | ||
|
|
||
| {% block title %}{% trans "Sustainability" %}{% endblock %} |
There was a problem hiding this comment.
Title should mention advertising
| <p> | ||
| This form can be used to pay for your sponsorship of Read the Docs. | ||
| Thanks for helping make Open Source more sustainable! | ||
| </p> |
There was a problem hiding this comment.
Same with the title and this paragraph block. Framing this as sponsorship is misleading.
| <option value="3000">$3,000</option> | ||
| <option value="5000" selected>$5,000</option> | ||
| <option value="10000">$10,000</option> | ||
| </select> |
There was a problem hiding this comment.
This select shouldn't exist at all.
There was a problem hiding this comment.
I left this to set the standard expectation that they should be paying us $5k test buys. Seemed simpler to have this select than rewrite how we were handling the actual data.
There was a problem hiding this comment.
It does seem a bit odd still though. Altering the form should be straight forward. You'd need to remove the <select>, drop the visible data bind parameter on <input>, and drop the style parameter on the <input> as well. This should give a permanent text input.
| from .mixins import DonateProgressMixin | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class PayAdsView(StripeMixin, CreateView): | ||
|
|
||
| """Create a donation locally and in Stripe""" |
There was a problem hiding this comment.
s/donation/advertising payment/
There was a problem hiding this comment.
Noted some HTML/CSS issues. These aren't horribly important. I pointed them out to ensure we're producing consistent templates. The current issue with our templates and UI is that it is not consistent. We can address these issues later if this is pressing, be sure to open a bug tracking this as cleanup if so.
| <option value="3000">$3,000</option> | ||
| <option value="5000" selected>$5,000</option> | ||
| <option value="10000">$10,000</option> | ||
| </select> |
There was a problem hiding this comment.
It does seem a bit odd still though. Altering the form should be straight forward. You'd need to remove the <select>, drop the visible data bind parameter on <input>, and drop the style parameter on the <input> as well. This should give a permanent text input.
|
|
||
| </div> | ||
|
|
||
| <br> |
There was a problem hiding this comment.
Whitespace shouldn't be added with html elements, this should be a css change.
| <th><strong>Day (UTC)</strong></th> | ||
| <th><strong>Views</strong></th> | ||
| <th><strong>Clicks</strong></th> | ||
| <th><strong>CTR</strong></th> |
There was a problem hiding this comment.
The <strong> here should also be replace be css rules. We shouldn't dictate styling in the HTML
| <td><strong>Total (over all time)</strong> </td> | ||
| <td><strong>{{ promo.total_views }}</strong></td> | ||
| <td><strong>{{ promo.total_clicks }}</strong></td> | ||
| <td><strong>{{ promo.total_click_ratio }}%</strong></td> |
There was a problem hiding this comment.
Same for styling here. This should perhaps be a rule for table > tr.table-totals or somesuch.
| def get_context_data(self, **kwargs): | ||
| promo_slug = kwargs['promo_slug'] | ||
| slugs = promo_slug.split(',') | ||
| days = int(self.request.GET.get('days', 90)) |
There was a problem hiding this comment.
Input isn't sanitized here, this should be wrapped for exception handling on TypeError
|
This has been live for quite a while -- can I just merge this @agjohnson, or do we care enough about the tidbits to fix it? |
|
My review still stands, but feel free to open issues to track the clean up efforts if you aren't going to handle them. I think this module makes more sense being pulled out of RTD eventually anyways |
|
Added #2691 |
This also adds the ability to see reports on specific promos.
Also gives a place to see how they display and test them before shipping them live.